From 8cdae161afd0f231e73b3a9e7f3b803e75f3bd52 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Tue, 13 Oct 2026 13:00:36 -0400 Subject: [PATCH 01/13] feat(agents): keep agent state in a folder the agent host can read The app kept each agent's settings, the owner's teams, unfinished setups, the Claude and Codex sessions and their default settings in the WebView's localStorage, which no program outside the app can read. Keep them in agents/state/ beside identities.json instead, one file per key, through two native commands. The app reads the folder once before it starts, keeps a copy in memory, and saves each write to its key's file in order. On first start it moves the agent keys it had in localStorage into the folder. Each agent's record and its Claude sessions now have their own key, so two programs that run different agents never write over each other. Agent types get the storage from the agents service (agents.storage). Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson --- src-tauri/build.rs | 2 + src-tauri/capabilities/default.json | 2 + src-tauri/src/agent_state.rs | 181 ++++++++++++++++++++ src-tauri/src/browser_permissions_tests.rs | 2 + src-tauri/src/lib.rs | 9 + src/bundled/agents/AgentsPage.test.tsx | 1 + src/bundled/claude-code/index.ts | 4 +- src/bundled/claude-code/runtime.test.ts | 3 +- src/bundled/claude-code/sessions.test.ts | 32 +++- src/bundled/claude-code/sessions.ts | 47 ++++-- src/bundled/codex/index.ts | 8 +- src/features/agents/service.test.ts | 53 +++++- src/features/agents/service.ts | 31 ++-- src/features/agents/state.test.ts | 88 ++++++++++ src/features/agents/state.ts | 139 ++++++++++++++++ src/features/agents/store.test.ts | 72 ++++++-- src/features/agents/store.ts | 184 +++++++++++++-------- src/main.tsx | 40 +++-- tests/browser/plugin-import.spec.mjs | 3 + tests/browser/updates.spec.mjs | 3 + 20 files changed, 762 insertions(+), 142 deletions(-) create mode 100644 src-tauri/src/agent_state.rs create mode 100644 src/features/agents/state.test.ts create mode 100644 src/features/agents/state.ts diff --git a/src-tauri/build.rs b/src-tauri/build.rs index e0ce0d3ce2..57b15203de 100644 --- a/src-tauri/build.rs +++ b/src-tauri/build.rs @@ -155,6 +155,8 @@ fn main() { "jev_classifier_set_key", "jev_classifier_clear_key", "jev_classify", + "app_agent_state_load", + "app_agent_state_set", "title_bar_double_click", "notification_show", "notification_permission_state", diff --git a/src-tauri/capabilities/default.json b/src-tauri/capabilities/default.json index cba381a270..a15ba65747 100644 --- a/src-tauri/capabilities/default.json +++ b/src-tauri/capabilities/default.json @@ -117,6 +117,8 @@ "allow-jev-classifier-set-key", "allow-jev-classifier-clear-key", "allow-jev-classify", + "allow-app-agent-state-load", + "allow-app-agent-state-set", "allow-title-bar-double-click", "allow-notification-show", diff --git a/src-tauri/src/agent_state.rs b/src-tauri/src/agent_state.rs new file mode 100644 index 0000000000..0cd7c7d7c0 --- /dev/null +++ b/src-tauri/src/agent_state.rs @@ -0,0 +1,181 @@ +//! Agent state shared by every program that runs this machine's agents: each +//! agent's settings, the owner's teams, unfinished setups, and the agent types' +//! saved sessions. It lives in `agents/state/`, next to `identities.json`, so +//! the app and the standalone `buzz-agent-host` read and write the same state. +//! +//! One file per key, each replaced whole through a rename, so a crash leaves +//! the last complete copy, and two programs that write different keys never +//! overwrite each other. The `hosts/` locks decide which program runs an agent. +use std::collections::HashMap; +use std::fs; +use std::io::ErrorKind; +use std::path::{Path, PathBuf}; + +/// The largest value kept, as an upper bound on what `localStorage` held. +const VALUE_LIMIT: usize = 4 * 1024 * 1024; +const KEY_LIMIT: usize = 200; +const TEMPORARY: &str = ".tmp"; + +pub(crate) struct AgentState { + dir: Result, +} + +impl AgentState { + /// `dir` is `agents/state`. + pub(crate) fn new(dir: Result) -> Self { + Self { dir } + } + + fn dir(&self) -> Result<&Path, String> { + self.dir.as_deref().map_err(Clone::clone) + } + + /// Every key and its value. Unreadable or foreign files are skipped. + pub(crate) fn load(&self) -> Result, String> { + let dir = self.dir()?; + let entries = match fs::read_dir(dir) { + Ok(entries) => entries, + Err(error) if error.kind() == ErrorKind::NotFound => return Ok(HashMap::new()), + Err(_) => return Err("Could not read agent state".into()), + }; + let mut items = HashMap::new(); + for entry in entries.flatten() { + let name = entry.file_name(); + let Some(key) = name.to_str().and_then(decode) else { + continue; + }; + if let Ok(value) = fs::read_to_string(entry.path()) { + items.insert(key, value); + } + } + Ok(items) + } + + /// Saves `value` under `key`, or removes the key when `value` is `None`. + pub(crate) fn set(&self, key: &str, value: Option<&str>) -> Result<(), String> { + if key.is_empty() || key.len() > KEY_LIMIT { + return Err("Invalid agent state key".into()); + } + let dir = self.dir()?; + let path = dir.join(encode(key)); + let Some(value) = value else { + return match fs::remove_file(&path) { + Err(error) if error.kind() != ErrorKind::NotFound => { + Err("Could not save agent state".into()) + } + _ => Ok(()), + }; + }; + if value.len() > VALUE_LIMIT { + return Err("Agent state is too large".into()); + } + let saved = (|| { + fs::create_dir_all(dir)?; + let mut file = tempfile::Builder::new() + .suffix(TEMPORARY) + .tempfile_in(dir)?; + std::io::Write::write_all(&mut file, value.as_bytes())?; + file.as_file().sync_all()?; + file.persist(&path).map_err(|error| error.error)?; + Ok::<_, std::io::Error>(()) + })(); + saved.map_err(|_| "Could not save agent state".into()) + } +} + +/// A file name for `key`: bytes other than `A-Z a-z 0-9 . _ -` become `%XX`. +pub(crate) fn encode(key: &str) -> String { + let mut name = String::with_capacity(key.len()); + for byte in key.bytes() { + if byte.is_ascii_alphanumeric() || matches!(byte, b'.' | b'_' | b'-') { + name.push(byte as char); + } else { + name.push_str(&format!("%{byte:02X}")); + } + } + name +} + +/// The key a file name holds, or `None` for a file that is not one, such as a +/// write in progress. +pub(crate) fn decode(name: &str) -> Option { + if name.is_empty() || name.ends_with(TEMPORARY) || name.starts_with('.') { + return None; + } + let bytes = name.as_bytes(); + let mut key = Vec::with_capacity(bytes.len()); + let mut index = 0; + while index < bytes.len() { + match bytes[index] { + b'%' => { + let hex = name.get(index + 1..index + 3)?; + key.push(u8::from_str_radix(hex, 16).ok()?); + index += 3; + } + byte if byte.is_ascii_alphanumeric() || matches!(byte, b'.' | b'_' | b'-') => { + key.push(byte); + index += 1; + } + _ => return None, + } + } + String::from_utf8(key).ok() +} + +#[tauri::command] +pub(crate) async fn app_agent_state_load( + state: tauri::State<'_, AgentState>, +) -> Result, String> { + state.load() +} + +#[tauri::command] +pub(crate) async fn app_agent_state_set( + state: tauri::State<'_, AgentState>, + key: String, + value: Option, +) -> Result<(), String> { + state.set(&key, value.as_deref()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn names_round_trip() { + for key in [ + "buzz.agents.v1", + "buzz.codex.sessions.v4:wss://relay.example/a:ab", + "a/b c%d", + ] { + let name = encode(key); + assert!(!name.contains('/') && !name.contains(':'), "{name}"); + assert_eq!(decode(&name).as_deref(), Some(key)); + } + assert_eq!(decode("x.tmp"), None); + assert_eq!(decode(".hidden"), None); + assert_eq!(decode("bad%Z"), None); + assert_eq!(decode("has space"), None); + } + + #[test] + fn saves_loads_and_removes() { + let root = tempfile::tempdir().unwrap(); + let state = AgentState::new(Ok(root.path().join("state"))); + assert!(state.load().unwrap().is_empty()); + state.set("one:a", Some("1")).unwrap(); + state.set("two", Some("2")).unwrap(); + state.set("two", Some("22")).unwrap(); + fs::write(root.path().join("state").join("partial.tmp"), "x").unwrap(); + let items = state.load().unwrap(); + assert_eq!(items.len(), 2); + assert_eq!(items["one:a"], "1"); + assert_eq!(items["two"], "22"); + state.set("one:a", None).unwrap(); + state.set("missing", None).unwrap(); + assert_eq!(state.load().unwrap().keys().collect::>(), ["two"]); + assert!(state.set("", Some("x")).is_err()); + assert!(state.set(&"k".repeat(KEY_LIMIT + 1), Some("x")).is_err()); + } +} diff --git a/src-tauri/src/browser_permissions_tests.rs b/src-tauri/src/browser_permissions_tests.rs index eacf115558..a808f99f36 100644 --- a/src-tauri/src/browser_permissions_tests.rs +++ b/src-tauri/src/browser_permissions_tests.rs @@ -157,6 +157,8 @@ fn native_command_permissions_isolate_webviews() { "jev_classifier_set_key", "jev_classifier_clear_key", "jev_classify", + "app_agent_state_load", + "app_agent_state_set", "title_bar_double_click", "notification_show", #[cfg(target_os = "macos")] diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index bafccceea3..d33a5c5679 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -14,6 +14,7 @@ use browser::{ browser_action, browser_attach, browser_detach, browser_navigate, browser_set_bounds, browser_status, }; +mod agent_state; mod app_agents; mod deep_links; mod dock; @@ -556,6 +557,8 @@ fn commands() -> impl Fn(tauri::ipc::Invoke) -> bool + Sen jev::jev_classifier_set_key, jev::jev_classifier_clear_key, jev::jev_classify, + agent_state::app_agent_state_load, + agent_state::app_agent_state_set, title_bar_double_click, notification_show, #[cfg(target_os = "macos")] @@ -685,6 +688,12 @@ pub fn run() { .map_err(|_| "Could not resolve legacy library directory")?; Ok((root, legacy)) })(); + app.manage(agent_state::AgentState::new( + paths + .as_ref() + .map(|(root, _)| root.with_file_name(sign_out::AGENTS).join("state")) + .map_err(Clone::clone), + )); app.manage(app_agents::AppAgentHost::new( paths .as_ref() diff --git a/src/bundled/agents/AgentsPage.test.tsx b/src/bundled/agents/AgentsPage.test.tsx index 75d2ceea34..d0c0c50d05 100644 --- a/src/bundled/agents/AgentsPage.test.tsx +++ b/src/bundled/agents/AgentsPage.test.tsx @@ -347,6 +347,7 @@ function setup( }; for (const listener of listeners) listener(); }), + storage: localStorage, } satisfies Agents; const disconnected = { status: "disconnected", generation: 0 }; const relay = { diff --git a/src/bundled/claude-code/index.ts b/src/bundled/claude-code/index.ts index 0808755c0d..1850a6ff55 100644 --- a/src/bundled/claude-code/index.ts +++ b/src/bundled/claude-code/index.ts @@ -25,12 +25,12 @@ export const apply: PluginModule["apply"] = (ctx) => { const typeKey = `${ctx.pluginOwner?.id ?? "buzz.claude-code"}/${TYPE_ID}`; const spawn = host.spawn && withScript(host.spawn.bind(host)); const setup = new ClaudeSetup(spawn, (input) => host.request(input)); - const providerDefaults = new ClaudeDefaults(); + const providerDefaults = new ClaudeDefaults(agents.storage); const runtime = new ClaudeRuntime( spawn ? { spawn } : {}, (pubkey) => ownerReads(agents.relay(pubkey), pubkey), (pubkey, signal) => agents.core(pubkey, signal), - localStorage, + agents.storage, (raw) => providerDefaults.resolve(raw), () => agents.tools(), ); diff --git a/src/bundled/claude-code/runtime.test.ts b/src/bundled/claude-code/runtime.test.ts index ba97188ac8..d9dbf5e096 100644 --- a/src/bundled/claude-code/runtime.test.ts +++ b/src/bundled/claude-code/runtime.test.ts @@ -132,6 +132,7 @@ function setup( const storage = { getItem: (key: string) => data.get(key) ?? null, setItem: (key: string, value: string) => void data.set(key, value), + removeItem: (key: string) => void data.delete(key), } as Storage; // The host's copy of `core` memory, which needs no relay read. const core = vi.fn( @@ -1043,7 +1044,7 @@ it("runs a timer as a one-off turn that is never saved", async () => { expect(prompt).toMatch(/\nHandle the work that caused this turn/); // Its process stops once the turn is done, and no session is kept for it. expect(claude?.killed).toBe(true); - expect(data.get("buzz.claude-code.sessions.v1") ?? "{}").toBe("{}"); + expect(data.get(`buzz.claude-code.sessions.v1:${self}`) ?? "{}").toBe("{}"); expect(runtime.sessions(self)).toEqual([]); }); diff --git a/src/bundled/claude-code/sessions.test.ts b/src/bundled/claude-code/sessions.test.ts index 9d16e95dd4..4d12557318 100644 --- a/src/bundled/claude-code/sessions.test.ts +++ b/src/bundled/claude-code/sessions.test.ts @@ -603,6 +603,7 @@ it("keeps saved sessions per agent in storage", () => { const storage = { getItem: (key: string) => data.get(key) ?? null, setItem: (key: string, value: string) => void data.set(key, value), + removeItem: (key: string) => void data.delete(key), } as Storage; const one = localSessions(storage, "one"); const two = localSessions(storage, "two"); @@ -613,8 +614,37 @@ it("keeps saved sessions per agent in storage", () => { one.delete("c/root"); expect(one.get("c/root")).toBeUndefined(); expect(two.get("c/root")?.id).toBe("s2"); - data.set("buzz.claude-code.sessions.v1", "not json"); + expect([...data.keys()].sort()).toEqual([ + "buzz.claude-code.sessions.v1:one", + "buzz.claude-code.sessions.v1:two", + ]); + data.set("buzz.claude-code.sessions.v1:one", "not json"); + expect(one.get("c/root")).toBeUndefined(); +}); + +it("reads an agent's sessions from the shared value until it saves its own", () => { + const data = new Map([ + [ + "buzz.claude-code.sessions.v1", + JSON.stringify({ + one: { "c/root": { id: "s1", seen: 3, at: 1 } }, + two: { "c/root": { id: "s2", seen: 4, at: 2 } }, + }), + ], + ]); + const storage = { + getItem: (key: string) => data.get(key) ?? null, + setItem: (key: string, value: string) => void data.set(key, value), + removeItem: (key: string) => void data.delete(key), + } as Storage; + const one = localSessions(storage, "one"); + expect(one.get("c/root")?.id).toBe("s1"); + one.delete("c/root"); expect(one.get("c/root")).toBeUndefined(); + expect(localSessions(storage, "two").get("c/root")?.id).toBe("s2"); + localSessions(storage, "two").set("c/other", { id: "s3", seen: 1, at: 3 }); + expect(data.has("buzz.claude-code.sessions.v1")).toBe(false); + expect(localSessions(storage, "two").get("c/root")?.id).toBe("s2"); }); it("shares a temporary metadata lookup, releases its process, and refreshes after settings change", async () => { diff --git a/src/bundled/claude-code/sessions.ts b/src/bundled/claude-code/sessions.ts index f4f34c8353..9a764d1a21 100644 --- a/src/bundled/claude-code/sessions.ts +++ b/src/bundled/claude-code/sessions.ts @@ -517,53 +517,70 @@ export class Spare { } } -/** Sessions saved in local storage, by agent and then conversation. */ +/** Where sessions were kept for every agent in one value, before each agent's + * had its own key. */ +export const LEGACY_SESSIONS = "buzz.claude-code.sessions.v1"; + +/** Sessions saved in agent storage, one key per agent, by conversation. Each + * agent's own key lets programs that run different agents share the storage. */ export function localSessions( storage: Storage, agent: string, - name = "buzz.claude-code.sessions.v1", + name = LEGACY_SESSIONS, ): SessionStore & { /** Saved sessions, most recently used first. */ list(): [string, SavedSession][]; } { - const readAll = (): Record> => { + const key = `${name}:${agent}`; + const object = (text: string | null) => { try { - const value = JSON.parse(storage.getItem(name) ?? "{}"); + const value = JSON.parse(text ?? "{}"); return value && typeof value === "object" ? value : {}; } catch { return {}; } }; + const read = (): Record => { + const own = storage.getItem(key); + return own === null + ? (object(storage.getItem(name))[agent] ?? {}) + : object(own); + }; const update = (change: (sessions: Record) => void) => { - const all = readAll(); - const sessions = { ...(all[agent] ?? {}) }; + const sessions = { ...read() }; change(sessions); const kept = Object.entries(sessions) .sort(([, a], [, b]) => b.at - a.at) .slice(0, SAVED_LIMIT); - all[agent] = Object.fromEntries(kept); try { - storage.setItem(name, JSON.stringify(all)); + storage.setItem(key, JSON.stringify(Object.fromEntries(kept))); + const legacy = object(storage.getItem(name)); + if (agent in legacy) { + delete legacy[agent]; + if (Object.keys(legacy).length) + storage.setItem(name, JSON.stringify(legacy)); + else storage.removeItem(name); + } } catch (error) { console.warn("Claude sessions were not saved", error); } }; return { list: () => - Object.entries(readAll()[agent] ?? {}) + Object.entries(read()) .filter(([, saved]) => typeof saved?.id === "string") .sort(([, a], [, b]) => b.at - a.at), - get: (key) => { - const saved = readAll()[agent]?.[key]; + get: (conversation) => { + const saved = read()[conversation]; return saved && typeof saved.id === "string" ? saved : undefined; }, - set: (key, session) => + set: (conversation, session) => update((sessions) => { - sessions[key] = session; + sessions[conversation] = session; }), - delete: (key) => + delete: (conversation) => update((sessions) => { - delete sessions[key]; + delete sessions[conversation]; }), }; } diff --git a/src/bundled/codex/index.ts b/src/bundled/codex/index.ts index d212c8b9e2..928d948ebe 100644 --- a/src/bundled/codex/index.ts +++ b/src/bundled/codex/index.ts @@ -22,12 +22,16 @@ export const apply: PluginModule["apply"] = (ctx) => { (async () => { throw new Error("Codex agents run only in the desktop app"); }); - const providerController = new CodexProvider(start, localStorage, setup); + const providerController = new CodexProvider( + start, + ctx.agents.storage, + setup, + ); const runtime = new CodexRuntime( start, (pubkey) => ownerReads(ctx.agents.relay(pubkey), pubkey), (pubkey, signal) => ctx.agents.core(pubkey, signal), - localStorage, + ctx.agents.storage, (raw) => providerController.resolve(raw), () => ctx.agents.tools(), ); diff --git a/src/features/agents/service.test.ts b/src/features/agents/service.test.ts index 8304afce1b..0dc2443c09 100644 --- a/src/features/agents/service.test.ts +++ b/src/features/agents/service.test.ts @@ -2245,7 +2245,7 @@ it("shows an agent whose settings could not be saved with its type's defaults", const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); await service.create({ type: "example/echo", name: "Echo" }); warn.mockRestore(); - expect(storage.getItem("buzz.agents.v1") ?? "").not.toContain(bot); + expect(storage.getItem(`buzz.agent.v1:${bot}`)).toBeNull(); expect(service.find(bot)).toMatchObject({ config: { reply: "ok" } }); expect(Object.keys(service.find(bot)?.attention ?? {})).toEqual([ "interest/default", @@ -2253,7 +2253,7 @@ it("shows an agent whose settings could not be saved with its type's defaults", ]); // The next save stores them. await service.save(bot, { config: { reply: "saved" } }); - expect(storage.getItem("buzz.agents.v1")).toContain(bot); + expect(storage.getItem(`buzz.agent.v1:${bot}`)).toContain(bot); }); it("saves nothing when native create fails", async () => { @@ -2295,7 +2295,7 @@ it("creates an agent with the config it was given, not its type's defaults, and "interest/default", "watch/channel", ]); - expect(storage.getItem("buzz.agents.v1")).toContain("chosen"); + expect(storage.getItem(`buzz.agent.v1:${bot}`)).toContain("chosen"); }); it("keeps a new agent whose picture could not be set", async () => { @@ -2382,7 +2382,41 @@ it("forgets records whose key is gone when identities load", async () => { expect(service.snapshot().agents.map((agent) => agent.name)).toEqual([ "Echo", ]); - expect(storage.getItem("buzz.agents.v1")).not.toContain(sibling); + expect(storage.getItem("buzz.agents.v1")).toBeNull(); + expect(storage.getItem(`buzz.agent.v1:${sibling}`)).toBeNull(); + expect(storage.getItem(`buzz.agent.v1:${bot}`)).toContain(bot); +}); + +it("saving one agent leaves another program's changes to other agents", async () => { + const storage = memoryStorage(); + for (const pubkey of [bot, sibling]) + storage.setItem( + `buzz.agent.v1:${pubkey}`, + JSON.stringify({ pubkey, attention: {}, config: { reply: "start" } }), + ); + const identity = (pubkey: string, name: string): AgentIdentity => ({ + pubkey, + relay: "wss://relay.example.test", + owner: viewer, + type: "example/echo", + name, + deleted: false, + }); + const { service } = await setup({ + storage, + identities: [identity(bot, "Echo"), identity(sibling, "Other")], + }); + await vi.waitFor(() => expect(service.find(sibling)).toBeDefined()); + // The standalone host runs the sibling and saves its progress. + const hosted = JSON.stringify({ + pubkey: sibling, + attention: {}, + config: { reply: "from the host" }, + }); + storage.setItem(`buzz.agent.v1:${sibling}`, hosted); + await service.save(bot, { config: { reply: "edited" } }); + expect(storage.getItem(`buzz.agent.v1:${bot}`)).toContain("edited"); + expect(storage.getItem(`buzz.agent.v1:${sibling}`)).toBe(hosted); }); it("keeps an agent created while identities are still loading", async () => { @@ -2415,7 +2449,7 @@ it("keeps an agent created while identities are still loading", async () => { release([]); await vi.waitFor(() => expect(service.snapshot().status).toBe("ready")); expect(service.find(bot)?.name).toBe("Echo"); - expect(storage.getItem("buzz.agents.v1")).toContain(bot); + expect(storage.getItem(`buzz.agent.v1:${bot}`)).toContain(bot); }); it("reads the list again on refresh and clears the error a failed read left", async () => { @@ -2520,7 +2554,7 @@ it("fires a due timer once, then again an interval after it ran", async () => { expect(run).toHaveBeenCalledTimes(2); // Its run state lives in the agent's record and goes when the timer does. const stored = () => - JSON.parse(storage.getItem("buzz.agents.v1") ?? "{}").agents[bot]; + JSON.parse(storage.getItem(`buzz.agent.v1:${bot}`) ?? "{}"); expect(stored().timers["watch/tick"]).toMatchObject({ used: 2 }); expect(stored().timers["watch/stale"]).toMatchObject({ used: 16 }); await service.save(bot, { attention: { "watch/tick": null } }); @@ -2744,7 +2778,7 @@ it("stops it first, and deletes its key only after it left its channels and was "forget", ]); expect(service.snapshot().agents).toEqual([]); - expect(storage.getItem("buzz.agents.v1")).not.toContain(bot); + expect(storage.getItem(`buzz.agent.v1:${bot}`)).toBeNull(); expect(leaveChannels).toHaveBeenCalledWith( expect.objectContaining({ archives }), bot, @@ -4490,9 +4524,10 @@ it("wakes nothing from the time its watch was off, also when it is turned on aft await settle(); first.connect(false); // Turned on again elsewhere while this copy was closed. - const saved = storage.getItem("buzz.agents.v1") ?? ""; + const saved = storage.getItem(`buzz.agent.v1:${bot}`) ?? ""; + expect(saved).toContain('"enabled":false'); storage.setItem( - "buzz.agents.v1", + `buzz.agent.v1:${bot}`, saved.replace('"enabled":false', '"enabled":true'), ); vi.setSystemTime((start + 60) * 1000); diff --git a/src/features/agents/service.ts b/src/features/agents/service.ts index b2791836d6..98ed062873 100644 --- a/src/features/agents/service.ts +++ b/src/features/agents/service.ts @@ -23,6 +23,7 @@ import { ownerReads } from "./reads"; // can show it: as here, hosted, or on another device. import { migrateAgentStorage } from "./migrate-storage"; import { migrateWatchFilters } from "./migrate-filter"; +import { agentStorage } from "./state"; import { Service, type Context } from "@deepseek-ai/cordis"; import { npubEncode } from "nostr-tools/nip19"; import { bytesToHex } from "nostr-tools/utils"; @@ -99,7 +100,9 @@ import { readUnfinished, setAttention, writeArchiveUnchecked, - writeRecords, + removeRecord, + splitLegacyRecords, + writeRecord, writeTeams, writeUnfinished, type AgentRecord, @@ -734,6 +737,10 @@ export type Agents = { pubkey: string, signal?: AbortSignal, ): Promise; + /** Where agent types keep what they save for an agent, such as its + * sessions. Every program that runs this machine's agents shares it, so + * keep each agent's state under its own key. */ + readonly storage: Storage; /** Makes an agent of `type` in the selected community: here, or with a * hosted type at its provider. A hosted one whose setup then fails is kept * as `unfinished`. */ @@ -1038,7 +1045,7 @@ export class AgentsService extends Service implements Agents { ctx: Context, private readonly communities: AgentCommunities, private readonly native: AgentsNative | undefined = nativeAgents(), - private readonly storage: Storage = globalThis.localStorage, + readonly storage: Storage = agentStorage(), private readonly authorize: ( pubkey: string, signal: AbortSignal, @@ -1055,6 +1062,7 @@ export class AgentsService extends Service implements Agents { migrateAgentStorage(storage); migrateWatchFilters(storage); this.records = readRecords(storage); + splitLegacyRecords(storage, this.records); this.unfinished = Object.freeze(readUnfinished(storage)); this.unarchiving = readArchiveUnchecked(storage); this.teamList = Object.freeze(readTeams(storage)); @@ -1957,10 +1965,10 @@ export class AgentsService extends Service implements Agents { ...this.recordOf(identity), config: change ? change.config : adopted, }; - const records = { ...this.records, [pubkey]: nextRecord }; - writeRecords(this.storage, records); + // Only this agent's key: others may be another program's to change. + writeRecord(this.storage, nextRecord); // Publish the in-memory pair only after its config is durable. - this.records = records; + this.records = { ...this.records, [pubkey]: nextRecord }; this.identities = this.identities.map((saved) => saved.pubkey === pubkey ? { ...saved, type } : saved, ); @@ -2373,9 +2381,9 @@ export class AgentsService extends Service implements Agents { /** Saves one record; memory changes only once storage has taken it. */ private write(record: AgentRecord) { - const records = { ...this.records, [record.pubkey]: record }; - writeRecords(this.storage, records); - this.records = records; + // Only this agent's key: others may be another program's to change. + writeRecord(this.storage, record); + this.records = { ...this.records, [record.pubkey]: record }; this.update(); } private live(pubkey: string) { @@ -2513,11 +2521,14 @@ export class AgentsService extends Service implements Agents { .filter((saved) => !saved.deleted) .map((saved) => saved.pubkey), ); - if (Object.keys(this.records).some((pubkey) => !live.has(pubkey))) { + const gone = Object.keys(this.records).filter( + (pubkey) => !live.has(pubkey), + ); + if (gone.length) { + for (const pubkey of gone) removeRecord(this.storage, pubkey); this.records = Object.fromEntries( Object.entries(this.records).filter(([pubkey]) => live.has(pubkey)), ); - writeRecords(this.storage, this.records); } // A deleted agent leaves its teams; the teams stay. if ( diff --git a/src/features/agents/state.test.ts b/src/features/agents/state.test.ts new file mode 100644 index 0000000000..bb74c20d15 --- /dev/null +++ b/src/features/agents/state.test.ts @@ -0,0 +1,88 @@ +import { expect, it, vi } from "vitest"; +import { fileBacked, loadAgentStorage } from "./state"; +import { memoryStorage } from "./test-fakes"; + +const files = (items: Record = {}) => { + const saved = new Map(Object.entries(items)); + return { + saved, + load: vi.fn(async () => Object.fromEntries(saved)), + set: vi.fn(async (key: string, value: string | null) => { + if (value === null) saved.delete(key); + else saved.set(key, value); + }), + }; +}; + +it("moves agent state from localStorage, keeping what agent storage has", async () => { + const local = memoryStorage(); + local.setItem("buzz.agents.v1", "records"); + local.setItem("buzz.agents.teams.v1", "old teams"); + local.setItem("buzz.claude-code.sessions.v1", "sessions"); + local.setItem("buzz.codex.sessions.v4:scope:ab", "codex"); + local.setItem("buzz.codex.provider-defaults.v1", "codex defaults"); + local.setItem("buzz.claude-code.provider-defaults.v1", "claude defaults"); + local.setItem("buzz.agents.archive-unchecked.v1", "unchecked"); + local.setItem("buzz.agents.catch-up.v1", "checkpoints"); + local.setItem("buzz.theme", "dark"); + const native = files({ "buzz.agents.teams.v1": "shared teams" }); + const storage = await loadAgentStorage(local, native); + expect(Object.fromEntries(native.saved)).toEqual({ + "buzz.agents.v1": "records", + "buzz.agents.teams.v1": "shared teams", + "buzz.claude-code.sessions.v1": "sessions", + "buzz.codex.sessions.v4:scope:ab": "codex", + "buzz.codex.provider-defaults.v1": "codex defaults", + "buzz.claude-code.provider-defaults.v1": "claude defaults", + "buzz.agents.archive-unchecked.v1": "unchecked", + "buzz.agents.catch-up.v1": "checkpoints", + }); + expect(storage.getItem("buzz.agents.teams.v1")).toBe("shared teams"); + // Moved keys leave localStorage; other keys stay. + expect(local.length).toBe(1); + expect(local.getItem("buzz.theme")).toBe("dark"); +}); + +it("keeps a key in localStorage when it could not be moved", async () => { + const local = memoryStorage(); + local.setItem("buzz.agents.v1", "records"); + const native = files(); + native.set.mockRejectedValueOnce(new Error("disk full")); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + await loadAgentStorage(local, native); + error.mockRestore(); + expect(local.getItem("buzz.agents.v1")).toBe("records"); +}); + +it("uses localStorage when agent storage cannot be read", async () => { + const local = memoryStorage(); + const native = files(); + native.load.mockRejectedValueOnce(new Error("denied")); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + expect(await loadAgentStorage(local, native)).toBe(local); + expect(await loadAgentStorage(local, undefined)).toBe(local); + error.mockRestore(); +}); + +it("saves each write in order, and a failed write does not stop later ones", async () => { + const native = files(); + const storage = fileBacked(new Map(), native.set); + native.set.mockRejectedValueOnce(new Error("busy")); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + storage.setItem("a", "1"); + storage.setItem("a", "2"); + storage.setItem("b", "3"); + storage.removeItem("b"); + storage.removeItem("missing"); + await storage.saved(); + error.mockRestore(); + expect(native.set.mock.calls).toEqual([ + ["a", "1"], + ["a", "2"], + ["b", "3"], + ["b", null], + ]); + expect(Object.fromEntries(native.saved)).toEqual({ a: "2" }); + expect(storage.getItem("a")).toBe("2"); + expect(storage.length).toBe(1); +}); diff --git a/src/features/agents/state.ts b/src/features/agents/state.ts new file mode 100644 index 0000000000..7f23f5db1b --- /dev/null +++ b/src/features/agents/state.ts @@ -0,0 +1,139 @@ +// Agent storage: each agent's settings, the owner's teams, unfinished setups, +// and the sessions agent types save. In the desktop app it lives in the +// `agents/state` folder beside the agent identities, one file per key, so the +// standalone agent host (`buzz-agent-host`) reads and writes the same state. +// Elsewhere, as in a browser, it is this page's localStorage. +// +// Storage reads are synchronous, so the app reads every file once before it +// starts, and then keeps a copy in memory. Each write changes the copy at once +// and is then saved, in order, to its key's file. Another program's later +// writes reach this copy only when the app starts again. +import { invoke, isTauri } from "@tauri-apps/api/core"; +import { CATCH_UP_KEY } from "./catch-up"; +import { + ARCHIVE_UNCHECKED_KEY, + RECORD_PREFIX, + STORAGE_KEY, + TEAMS_KEY, + UNFINISHED_KEY, +} from "./store"; + +/** The localStorage keys of agent state, moved into agent storage once. */ +export function isAgentKey(key: string) { + return ( + key === STORAGE_KEY || + key === TEAMS_KEY || + key === UNFINISHED_KEY || + key === ARCHIVE_UNCHECKED_KEY || + key === CATCH_UP_KEY || + key.startsWith(RECORD_PREFIX) || + // The sessions the bundled agent types save. + key.startsWith("buzz.claude-code.sessions.") || + key.startsWith("buzz.codex.sessions.") || + // The device-wide defaults that blank agent settings inherit. + key === "buzz.claude-code.provider-defaults.v1" || + key === "buzz.codex.provider-defaults.v1" || + // TEMPORARY agents rename: see migrate-storage.ts. + key.startsWith("buzz.agents2.") + ); +} + +let loaded: Storage | undefined; + +/** Agent storage, once `loadAgentStorage` has read it; localStorage before + * that, and outside the desktop app. */ +export function agentStorage(): Storage { + return loaded ?? globalThis.localStorage; +} + +type Native = Readonly<{ + load(): Promise>; + set(key: string, value: string | null): Promise; +}>; + +const native: Native = { + load: () => invoke("app_agent_state_load"), + set: (key, value) => invoke("app_agent_state_set", { key, value }), +}; + +/** Reads agent storage before the app starts. It moves agent state the app + * kept in localStorage into it: each key it does not have yet. If agent storage + * cannot be read, the app keeps using localStorage. */ +export async function loadAgentStorage( + local: Storage = globalThis.localStorage, + files: Native | undefined = isTauri() ? native : undefined, +): Promise { + if (!files) return local; + let items: Record; + try { + items = await files.load(); + } catch (error) { + console.error( + "Agent storage could not be read; using this window's", + error, + ); + return local; + } + const storage = fileBacked(new Map(Object.entries(items)), files.set); + const moved: string[] = []; + for (let index = 0; index < local.length; index++) { + const key = local.key(index); + if (key && isAgentKey(key)) moved.push(key); + } + for (const key of moved) { + const value = local.getItem(key); + if (value === null) continue; + try { + if (storage.getItem(key) === null) { + storage.setItem(key, value); + await (storage as FileBacked).saved(); + } + local.removeItem(key); + } catch (error) { + // Kept in localStorage, so the next start tries again. + console.error(`Agent state ${key} could not be moved`, error); + } + } + loaded = storage; + return storage; +} + +type FileBacked = Storage & { + /** Settles when every write so far is saved; rejects if the last failed. */ + saved(): Promise; +}; + +/** A Storage held in memory and saved through `save`, one key at a time. */ +export function fileBacked( + items: Map, + save: Native["set"], +): FileBacked { + let queue: Promise = Promise.resolve(); + const write = (key: string, value: string | null) => { + const next = queue.then(() => save(key, value)); + queue = next.catch((error) => { + console.error(`Agent state ${key} was not saved`, error); + }); + return next; + }; + let last: Promise = Promise.resolve(); + return { + get length() { + return items.size; + }, + key: (index) => [...items.keys()][index] ?? null, + getItem: (key) => items.get(key) ?? null, + setItem(key, value) { + items.set(key, String(value)); + last = write(key, String(value)); + }, + removeItem(key) { + if (!items.delete(key)) return; + last = write(key, null); + }, + clear() { + for (const key of [...items.keys()]) this.removeItem(key); + }, + saved: () => last, + }; +} diff --git a/src/features/agents/store.test.ts b/src/features/agents/store.test.ts index 334446ea45..c455031c76 100644 --- a/src/features/agents/store.test.ts +++ b/src/features/agents/store.test.ts @@ -2,10 +2,14 @@ import { expect, it } from "vitest"; import { STORAGE_KEY, TEAMS_KEY, + readOne, readRecords, + recordKey, readTeams, setAttention, - writeRecords, + removeRecord, + splitLegacyRecords, + writeRecord, writeTeams, type AgentRecord, } from "./store"; @@ -26,16 +30,16 @@ it("round-trips records and drops attention objects a reader would ignore", () = { type: "interest", instructions: "Be brief." }, 7, ); - writeRecords(storage, { [pubkey]: saved }); + writeRecord(storage, saved); expect(readRecords(storage)).toEqual({ [pubkey]: saved }); - const stored = JSON.parse(storage.getItem(STORAGE_KEY) as string); - stored.agents[pubkey].attention["watch/bad"] = { + const stored = JSON.parse(storage.getItem(recordKey(pubkey)) as string); + stored.attention["watch/bad"] = { slug: "watch/bad", value: { type: "event" }, modifiedAt: 1, }; - stored.agents.nonsense = record; - storage.setItem(STORAGE_KEY, JSON.stringify(stored)); + storage.setItem(recordKey(pubkey), JSON.stringify(stored)); + storage.setItem(recordKey("nonsense"), JSON.stringify(record)); const read = readRecords(storage); expect(read[pubkey]?.attention).toEqual(saved.attention); expect(read).not.toHaveProperty("nonsense"); @@ -44,9 +48,9 @@ it("round-trips records and drops attention objects a reader would ignore", () = value: { type: "event" }, problem: expect.stringMatching(/Missing/), }); - writeRecords(storage, read); + writeRecord(storage, read[pubkey] as AgentRecord); expect( - JSON.parse(storage.getItem(STORAGE_KEY) as string).agents[pubkey].attention[ + JSON.parse(storage.getItem(recordKey(pubkey)) as string).attention[ "watch/bad" ], ).toEqual({ slug: "watch/bad", value: { type: "event" }, modifiedAt: 1 }); @@ -55,17 +59,51 @@ it("round-trips records and drops attention objects a reader would ignore", () = expect(fixed.skipped).toEqual({}); }); -it("saves attention only as on; absent or anything else reads as off", () => { +it("keeps each agent's record under its own key and moves the old shared value", () => { const storage = memoryStorage(); - writeRecords(storage, { - [pubkey]: { ...record, attentionEnabled: true }, - ["b".repeat(64)]: { ...record, pubkey: "b".repeat(64) }, - ["c".repeat(64)]: { - ...record, - pubkey: "c".repeat(64), - attentionEnabled: false, + const other = "b".repeat(64); + storage.setItem( + STORAGE_KEY, + JSON.stringify({ + version: 1, + agents: { [pubkey]: record, [other]: { ...record, pubkey: other } }, + }), + ); + expect(Object.keys(readRecords(storage)).sort()).toEqual([pubkey, other]); + expect(readOne(storage, other)?.pubkey).toBe(other); + // Saved records win over the old shared value. + writeRecord(storage, { ...record, config: { word: "new" } }); + expect(readRecords(storage)[pubkey]?.config).toEqual({ word: "new" }); + splitLegacyRecords(storage, readRecords(storage)); + expect(storage.getItem(STORAGE_KEY)).toBeNull(); + expect(readOne(storage, other)?.pubkey).toBe(other); + // Writing one record leaves another program's record alone. + storage.setItem(recordKey(other), "sentinel"); + writeRecord(storage, { ...record, config: { word: "newer" } }); + expect(storage.getItem(recordKey(other))).toBe("sentinel"); + // A record that did not change is not written again. + const writes: string[] = []; + const watched = Object.assign(Object.create(storage), { + setItem: (key: string, value: string) => { + writes.push(key); + storage.setItem(key, value); }, }); + writeRecord(watched, { ...record, config: { word: "newer" } }); + expect(writes).toEqual([]); + // A removed record is gone. + removeRecord(storage, other); + expect(storage.getItem(recordKey(other))).toBeNull(); +}); + +it("saves attention only as on; absent or anything else reads as off", () => { + const storage = memoryStorage(); + for (const saved of [ + { ...record, attentionEnabled: true }, + { ...record, pubkey: "b".repeat(64) }, + { ...record, pubkey: "c".repeat(64), attentionEnabled: false }, + ] as AgentRecord[]) + writeRecord(storage, saved); const read = readRecords(storage); expect(read[pubkey]?.attentionEnabled).toBe(true); expect(read["b".repeat(64)]).not.toHaveProperty("attentionEnabled"); @@ -206,7 +244,9 @@ it("reads nothing from missing or malformed storage", () => { const storage = memoryStorage(); expect(readRecords(storage)).toEqual({}); storage.setItem(STORAGE_KEY, "{"); + storage.setItem(recordKey(pubkey), "{"); expect(readRecords(storage)).toEqual({}); + expect(readOne(storage, pubkey)).toBeUndefined(); }); it("round-trips teams and skips any that is not one", () => { diff --git a/src/features/agents/store.ts b/src/features/agents/store.ts index 08d316270a..f92a58a786 100644 --- a/src/features/agents/store.ts +++ b/src/features/agents/store.ts @@ -40,7 +40,12 @@ export type AgentRecord = Readonly<{ }>; type Stored = { version: 1; agents: Record }; +/** Where records were kept, all in one value, before each had its own key. */ export const STORAGE_KEY = "buzz.agents.v1"; +/** Each agent's record has its own key, so programs that run different + * agents and share this storage never write over each other's records. */ +export const RECORD_PREFIX = "buzz.agent.v1:"; +export const recordKey = (pubkey: string) => `${RECORD_PREFIX}${pubkey}`; const PUBKEY = /^[0-9a-f]{64}$/; const group = (value: AttentionValue) => value.type === "interest" ? "Interests" : "watches and timers"; @@ -48,12 +53,42 @@ const group = (value: AttentionValue) => /** Reads every record. An attention object a reader would not apply is skipped * and reported, never dropped. */ export function readRecords(storage: Storage): Record { - let stored: unknown; + const stored: Record = { ...legacyRecords(storage) }; + for (let index = 0; index < storage.length; index++) { + const key = storage.key(index); + if (!key?.startsWith(RECORD_PREFIX)) continue; + const record = parsed(storage.getItem(key)); + if (record) stored[key.slice(RECORD_PREFIX.length)] = record as AgentRecord; + } + const records: Record = {}; + for (const [pubkey, record] of Object.entries(stored)) { + const read = readRecord(pubkey, record); + if (read) records[pubkey] = read; + } + return records; +} + +/** The agent's record as now stored, or undefined when it has none. */ +export function readOne( + storage: Storage, + pubkey: string, +): AgentRecord | undefined { + const record = + parsed(storage.getItem(recordKey(pubkey))) ?? + legacyRecords(storage)[pubkey]; + return record ? readRecord(pubkey, record as AgentRecord) : undefined; +} + +function parsed(text: string | null): unknown { try { - stored = JSON.parse(storage.getItem(STORAGE_KEY) ?? "null"); + return JSON.parse(text ?? "null"); } catch { - return {}; + return undefined; } +} + +function legacyRecords(storage: Storage): Record { + const stored = parsed(storage.getItem(STORAGE_KEY)); if ( !stored || typeof stored !== "object" || @@ -61,80 +96,93 @@ export function readRecords(storage: Storage): Record { typeof (stored as Stored).agents !== "object" ) return {}; - const records: Record = {}; - for (const [pubkey, record] of Object.entries((stored as Stored).agents)) { - if (!PUBKEY.test(pubkey) || record?.pubkey !== pubkey) continue; - const attention: Record = {}; - const skipped: Record = {}; - const counts = { Interests: 0, "watches and timers": 0 }; - const entries = Object.entries(record.attention ?? {}).sort(([a], [b]) => - a < b ? -1 : a > b ? 1 : 0, - ); - for (const [slug, object] of entries) { - const modifiedAt = Number.isSafeInteger(object?.modifiedAt) - ? object.modifiedAt - : 0; - let problem = - object?.slug === slug - ? validateObject(slug, object.value) - : "Stored under another slug"; - if (!problem) { - const kind = group(object.value); - if (++counts[kind] > OBJECT_LIMIT) - problem = `Over the limit of ${OBJECT_LIMIT} ${kind}`; - } - if (problem) - skipped[slug] = { slug, value: object?.value, modifiedAt, problem }; - else attention[slug] = { slug, value: object.value, modifiedAt }; + return (stored as Stored).agents; +} + +function readRecord( + pubkey: string, + record: AgentRecord | undefined, +): AgentRecord | undefined { + if (!PUBKEY.test(pubkey) || record?.pubkey !== pubkey) return undefined; + const attention: Record = {}; + const skipped: Record = {}; + const counts = { Interests: 0, "watches and timers": 0 }; + const entries = Object.entries(record.attention ?? {}).sort(([a], [b]) => + a < b ? -1 : a > b ? 1 : 0, + ); + for (const [slug, object] of entries) { + const modifiedAt = Number.isSafeInteger(object?.modifiedAt) + ? object.modifiedAt + : 0; + let problem = + object?.slug === slug + ? validateObject(slug, object.value) + : "Stored under another slug"; + if (!problem) { + const kind = group(object.value); + if (++counts[kind] > OBJECT_LIMIT) + problem = `Over the limit of ${OBJECT_LIMIT} ${kind}`; } - const timers: Record = {}; - for (const [slug, state] of Object.entries(record.timers ?? {})) - if ( - attention[slug]?.value.type === "timer" && - [state?.armedAt, state?.nextDue, state?.used].every( - Number.isSafeInteger, - ) - ) - timers[slug] = state; - records[pubkey] = { - pubkey, - attention, - skipped, - config: record.config, - timers, - ...(record.attentionEnabled === true ? { attentionEnabled: true } : {}), - }; + if (problem) + skipped[slug] = { slug, value: object?.value, modifiedAt, problem }; + else attention[slug] = { slug, value: object.value, modifiedAt }; } - return records; + const timers: Record = {}; + for (const [slug, state] of Object.entries(record.timers ?? {})) + if ( + attention[slug]?.value.type === "timer" && + [state?.armedAt, state?.nextDue, state?.used].every(Number.isSafeInteger) + ) + timers[slug] = state; + return { + pubkey, + attention, + skipped, + config: record.config, + timers, + ...(record.attentionEnabled === true ? { attentionEnabled: true } : {}), + }; } -export function writeRecords( +/** Moves records from the old value that held every agent into one key per + * agent, then removes it. Only that move writes many records at once: after + * it, each save writes only its own agent's key, so a program that runs + * other agents from the same storage keeps its changes. */ +export function splitLegacyRecords( storage: Storage, records: Readonly>, +) { + if (storage.getItem(STORAGE_KEY) === null) return; + for (const record of Object.values(records)) writeRecord(storage, record); + storage.removeItem(STORAGE_KEY); +} + +/** Removes an agent's record. */ +export function removeRecord(storage: Storage, pubkey: string) { + storage.removeItem(recordKey(pubkey)); +} + +/** Saves one record, when it changed. */ +export function writeRecord( + storage: Storage, + { skipped, ...record }: AgentRecord, ) { // Skipped objects go back where they were read from, so a later reader that // can apply them still finds them. - const agents = Object.fromEntries( - Object.entries(records).map(([pubkey, { skipped, ...record }]) => [ - pubkey, - { - ...record, - attention: { - ...Object.fromEntries( - Object.values(skipped ?? {}).map(({ problem: _, ...object }) => [ - object.slug, - object, - ]), - ), - ...record.attention, - }, - }, - ]), - ); - storage.setItem( - STORAGE_KEY, - JSON.stringify({ version: 1, agents } as Stored), - ); + const text = JSON.stringify({ + ...record, + attention: { + ...Object.fromEntries( + Object.values(skipped ?? {}).map(({ problem: _, ...object }) => [ + object.slug, + object, + ]), + ), + ...record.attention, + }, + }); + const key = recordKey(record.pubkey); + if (storage.getItem(key) !== text) storage.setItem(key, text); } /** Replaces or deletes (`value: null`) one attention object, after validating it diff --git a/src/main.tsx b/src/main.tsx index 0b280b27a6..f4a099e49e 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -4,6 +4,7 @@ import "./shared/styles/globals.css"; import { StrictMode } from "react"; import { createRoot } from "react-dom/client"; import { createServices } from "./app/services"; +import { loadAgentStorage } from "./features/agents/state"; import { App } from "./app/App"; import "@fontsource-variable/inter/wght.css"; import "@fontsource-variable/inter/wght-italic.css"; @@ -17,22 +18,25 @@ if (!container) { throw new Error('Missing
in index.html'); } -const services = createServices(); -const root = createRoot(container); -function HostApp() { - useKeyboardFocusVisibility(); - useRejectUnclaimedDrops(); - return ; -} -root.render( - - - , -); +// Agent storage is read before any service, since its reads are synchronous. +void loadAgentStorage().then(() => { + const services = createServices(); + const root = createRoot(container); + function HostApp() { + useKeyboardFocusVisibility(); + useRejectUnclaimedDrops(); + return ; + } + root.render( + + + , + ); -if (import.meta.hot) { - import.meta.hot.dispose(() => { - root.unmount(); - void services.dispose().catch(console.error); - }); -} + if (import.meta.hot) { + import.meta.hot.dispose(() => { + root.unmount(); + void services.dispose().catch(console.error); + }); + } +}); diff --git a/tests/browser/plugin-import.spec.mjs b/tests/browser/plugin-import.spec.mjs index 0993be5466..0f78cce659 100644 --- a/tests/browser/plugin-import.spec.mjs +++ b/tests/browser/plugin-import.spec.mjs @@ -106,6 +106,9 @@ async function nativeImports(page, samples = [], previewOptions = {}) { return preview; } if (command === "plugin_import_discard") return; + // Agent storage starts empty; the app saves agent state there. + if (command === "app_agent_state_load") return {}; + if (command === "app_agent_state_set") return; if (command === "plugin_import_install") { if (window.importInstallError) { await new Promise((resolve) => { diff --git a/tests/browser/updates.spec.mjs b/tests/browser/updates.spec.mjs index 1b13379e98..032093f5ee 100644 --- a/tests/browser/updates.spec.mjs +++ b/tests/browser/updates.spec.mjs @@ -54,6 +54,9 @@ async function nativeUpdater( }, }; if (command === "plugin_module") return "export function apply() {}"; + // Agent storage starts empty; the app saves agent state there. + if (command === "app_agent_state_load") return {}; + if (command === "app_agent_state_set") return; const plugin = /^plugin:(updater|resources)\|(\w+)$/.exec(command); if (!plugin && command !== "update_restart") throw new Error(`Unexpected command ${command}`); From 0b6b7eec612818da58e2e80e4061611623b7d740 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Tue, 13 Oct 2026 18:21:13 -0400 Subject: [PATCH 02/13] fix(agents): remove moved agent state only once its new copy is saved A failed file write during the one-time moves (the shared record value, the shared Claude sessions value, the agents2 rename) still removed the source, so the next start lost that agent's settings or sessions. Each move now removes its source only after every new copy is saved. A failed write also no longer passes as saved: agent storage puts back the key's saved value, saves of settings, providers and teams reject, and the same value can be saved again. A failed timer-progress save still keeps the run's memory, so a timer does not run again at once. Agent state keys are limited by their file name (255 bytes), not by a 200-byte key, so Codex sessions on long community hosts are saved. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson --- src-tauri/src/agent_state.rs | 25 +++++++++++-- src/bundled/claude-code/sessions.test.ts | 26 +++++++++++++ src/bundled/claude-code/sessions.ts | 29 +++++++++++---- src/features/agents/migrate-storage.ts | 17 ++++++++- src/features/agents/service.test.ts | 23 ++++++++++++ src/features/agents/service.ts | 33 ++++++++++++----- src/features/agents/state.test.ts | 19 ++++++++++ src/features/agents/state.ts | 47 ++++++++++++++++++------ src/features/agents/store.test.ts | 35 +++++++++++++++++- src/features/agents/store.ts | 37 ++++++++++++++++++- 10 files changed, 256 insertions(+), 35 deletions(-) diff --git a/src-tauri/src/agent_state.rs b/src-tauri/src/agent_state.rs index 0cd7c7d7c0..74d8a91a78 100644 --- a/src-tauri/src/agent_state.rs +++ b/src-tauri/src/agent_state.rs @@ -13,7 +13,8 @@ use std::path::{Path, PathBuf}; /// The largest value kept, as an upper bound on what `localStorage` held. const VALUE_LIMIT: usize = 4 * 1024 * 1024; -const KEY_LIMIT: usize = 200; +/// The longest file name macOS, Linux and Windows all allow, in bytes. +const NAME_LIMIT: usize = 255; const TEMPORARY: &str = ".tmp"; pub(crate) struct AgentState { @@ -53,7 +54,8 @@ impl AgentState { /// Saves `value` under `key`, or removes the key when `value` is `None`. pub(crate) fn set(&self, key: &str, value: Option<&str>) -> Result<(), String> { - if key.is_empty() || key.len() > KEY_LIMIT { + // The limit is on the file name, after `encode` lengthens the key. + if key.is_empty() || encode(key).len() > NAME_LIMIT { return Err("Invalid agent state key".into()); } let dir = self.dir()?; @@ -176,6 +178,23 @@ mod tests { state.set("missing", None).unwrap(); assert_eq!(state.load().unwrap().keys().collect::>(), ["two"]); assert!(state.set("", Some("x")).is_err()); - assert!(state.set(&"k".repeat(KEY_LIMIT + 1), Some("x")).is_err()); + assert!(state.set(&"k".repeat(NAME_LIMIT + 1), Some("x")).is_err()); + assert!(state.set(&":".repeat(86), Some("x")).is_err()); + state.set(&"k".repeat(NAME_LIMIT), Some("x")).unwrap(); + } + + #[test] + fn saves_codex_session_keys_of_long_community_hosts() { + let root = tempfile::tempdir().unwrap(); + let state = AgentState::new(Ok(root.path().join("state"))); + // 202 bytes, as Codex names it for a community on a 47-character host. + let key = format!( + "buzz.codex.sessions.v4:https://agents.production.engineering.example.com:{}:{}", + "a".repeat(64), + "b".repeat(64), + ); + assert_eq!(key.len(), 202); + state.set(&key, Some("{}")).unwrap(); + assert_eq!(state.load().unwrap()[&key], "{}"); } } diff --git a/src/bundled/claude-code/sessions.test.ts b/src/bundled/claude-code/sessions.test.ts index 4d12557318..d4b4c0c564 100644 --- a/src/bundled/claude-code/sessions.test.ts +++ b/src/bundled/claude-code/sessions.test.ts @@ -1,4 +1,5 @@ import { afterEach, expect, it, vi } from "vitest"; +import { fileBacked } from "../../features/agents/state"; import { backgroundTasks, fakeSpawn, @@ -647,6 +648,31 @@ it("reads an agent's sessions from the shared value until it saves its own", () expect(localSessions(storage, "two").get("c/root")?.id).toBe("s2"); }); +it("keeps an agent in the shared sessions value until its own is saved", async () => { + const name = "buzz.claude-code.sessions.v1"; + const shared = JSON.stringify({ + one: { "c/root": { id: "s1", seen: 3, at: 1 } }, + }); + const disk = new Map([[name, shared]]); + const save = vi.fn(async (key: string, value: string | null) => { + if (key !== name) throw new Error("disk full"); + if (value === null) disk.delete(key); + else disk.set(key, value); + }); + const storage = fileBacked(new Map(disk), save); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + localSessions(storage, "one").set("c/other", { id: "s2", seen: 1, at: 2 }); + await storage.saved().catch(() => {}); + await new Promise((resolve) => setTimeout(resolve)); + error.mockRestore(); + warn.mockRestore(); + expect(disk.get(name)).toBe(shared); + // The next start still resumes the saved session. + const restarted = fileBacked(new Map(disk), save); + expect(localSessions(restarted, "one").get("c/root")?.id).toBe("s1"); +}); + it("shares a temporary metadata lookup, releases its process, and refreshes after settings change", async () => { const { pool, claudes, live, setModel } = sessions(); const [first, second] = await Promise.all([pool.metadata(), pool.metadata()]); diff --git a/src/bundled/claude-code/sessions.ts b/src/bundled/claude-code/sessions.ts index 9a764d1a21..870f5dd186 100644 --- a/src/bundled/claude-code/sessions.ts +++ b/src/bundled/claude-code/sessions.ts @@ -16,6 +16,7 @@ import { type TurnPick, } from "./claude"; import { STANDALONE_DEADLINE_MS } from "../../features/agent-harness/standalone"; +import { afterSaved } from "../../features/agents/store"; /** A conversation's process stops after this long without a message or its * own activity, such as background work. */ @@ -554,16 +555,30 @@ export function localSessions( .slice(0, SAVED_LIMIT); try { storage.setItem(key, JSON.stringify(Object.fromEntries(kept))); - const legacy = object(storage.getItem(name)); - if (agent in legacy) { - delete legacy[agent]; - if (Object.keys(legacy).length) - storage.setItem(name, JSON.stringify(legacy)); - else storage.removeItem(name); - } } catch (error) { console.warn("Claude sessions were not saved", error); + return; } + // The old shared value drops this agent once its own key is saved: until + // then it holds the only saved copy. + if (!(agent in object(storage.getItem(name)))) return; + afterSaved( + storage, + [key], + () => { + const legacy = object(storage.getItem(name)); + if (!(agent in legacy)) return; + delete legacy[agent]; + try { + if (Object.keys(legacy).length) + storage.setItem(name, JSON.stringify(legacy)); + else storage.removeItem(name); + } catch (error) { + console.warn("Claude sessions were not saved", error); + } + }, + (error) => console.warn("Claude sessions were not saved", error), + ); }; return { list: () => diff --git a/src/features/agents/migrate-storage.ts b/src/features/agents/migrate-storage.ts index 056a1c7ccf..e3e08a33e0 100644 --- a/src/features/agents/migrate-storage.ts +++ b/src/features/agents/migrate-storage.ts @@ -1,6 +1,6 @@ // TEMPORARY agents rename: remove this module and its startup call after // 2026-10-18, once the development users have upgraded. No runtime aliases. -import { STORAGE_KEY, TEAMS_KEY } from "./store"; +import { afterSaved, STORAGE_KEY, TEAMS_KEY } from "./store"; export function migrateAgentStorage(storage: Storage) { for (const [oldKey, key] of [ @@ -22,9 +22,22 @@ export function migrateAgentStorage(storage: Storage) { } try { if (current === null) storage.setItem(key, old); - storage.removeItem(oldKey); } catch (error) { console.error(`Cannot migrate ${oldKey}`, error); + continue; } + // The source goes once the copy is saved; until then it is the only one. + afterSaved( + storage, + [key], + () => { + try { + storage.removeItem(oldKey); + } catch (error) { + console.error(`Cannot migrate ${oldKey}`, error); + } + }, + (error) => console.error(`Cannot migrate ${oldKey}`, error), + ); } } diff --git a/src/features/agents/service.test.ts b/src/features/agents/service.test.ts index 0dc2443c09..5ac74ca95d 100644 --- a/src/features/agents/service.test.ts +++ b/src/features/agents/service.test.ts @@ -26,6 +26,7 @@ import { } from "./service"; import type { ClassifierNative } from "./classifier"; import { CATCH_UP_KEY, readCheckpoints } from "./catch-up"; +import { fileBacked } from "./state"; import { memoryStorage } from "./test-fakes"; import { STANDALONE_DEADLINE_MS } from "../agent-harness/standalone"; import { readTeams, writeTeams } from "./store"; @@ -2256,6 +2257,28 @@ it("shows an agent whose settings could not be saved with its type's defaults", expect(storage.getItem(`buzz.agent.v1:${bot}`)).toContain(bot); }); +it("rejects a save that agent storage could not write, and saves it on retry", async () => { + const disk = new Map(); + let failing = false; + const storage = fileBacked(new Map(), async (key, value) => { + if (failing) throw new Error("disk full"); + if (value === null) disk.delete(key); + else disk.set(key, value); + }); + const { service } = await setup({ storage }); + await service.create({ type: "example/echo", name: "Echo" }); + failing = true; + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + await expect( + service.save(bot, { config: { reply: "unsaved" } }), + ).rejects.toThrow("disk full"); + error.mockRestore(); + expect(disk.get(`buzz.agent.v1:${bot}`)).not.toContain("unsaved"); + failing = false; + await service.save(bot, { config: { reply: "unsaved" } }); + expect(disk.get(`buzz.agent.v1:${bot}`)).toContain("unsaved"); +}); + it("saves nothing when native create fails", async () => { const storage = memoryStorage(); const { service, native } = await setup({ storage }); diff --git a/src/features/agents/service.ts b/src/features/agents/service.ts index 98ed062873..a955831654 100644 --- a/src/features/agents/service.ts +++ b/src/features/agents/service.ts @@ -95,8 +95,11 @@ import { } from "./native"; import { readArchiveUnchecked, + persisted, readRecords, + recordKey, readTeams, + TEAMS_KEY, readUnfinished, setAttention, writeArchiveUnchecked, @@ -1096,7 +1099,12 @@ export class AgentsService extends Service implements Agents { if (!identity) throw new Error("No such agent on this device"); return this.recordOf(identity); }, - write: (record) => this.write(record), + // A failed save logs; the run's memory keeps it from running again. + write: (record) => { + void this.write(record).catch((error) => + console.warn(`Agent ${record.pubkey} state was not saved`, error), + ); + }, start: (agent) => { const { settings, instructions } = this.configBinding?.(agent) ?? {}; const merge = this.types().find( @@ -1770,7 +1778,7 @@ export class AgentsService extends Service implements Agents { record = { ...record, config: made.config }; if (carry && kind.adopt) record = { ...record, config: kind.adopt(carry, record.config) }; - this.write(record); + await this.write(record); } catch (error) { // The agent exists either way; unsaved settings start from the defaults. console.warn(`Agent ${identity.name} settings were not saved`, error); @@ -1850,7 +1858,7 @@ export class AgentsService extends Service implements Agents { change.attention || change.attentionEnabled !== undefined ) - this.write(record); + await this.write(record); const picture = change.picture === undefined ? undefined : change.picture || null; const renamed = name !== undefined && name !== identity.name; @@ -1967,6 +1975,7 @@ export class AgentsService extends Service implements Agents { }; // Only this agent's key: others may be another program's to change. writeRecord(this.storage, nextRecord); + await persisted(this.storage, recordKey(pubkey)); // Publish the in-memory pair only after its config is durable. this.records = { ...this.records, [pubkey]: nextRecord }; this.identities = this.identities.map((saved) => @@ -2113,7 +2122,7 @@ export class AgentsService extends Service implements Agents { instructions, members: Object.freeze([...new Set(members)]), }); - this.writeTeams( + await this.writeTeams( id ? this.teamList.map((saved) => (saved.id === id ? team : saved)) : [...this.teamList, team], @@ -2145,15 +2154,17 @@ export class AgentsService extends Service implements Agents { } async removeTeam(id: string) { - this.writeTeams(this.teamList.filter((team) => team.id !== id)); + await this.writeTeams(this.teamList.filter((team) => team.id !== id)); } - /** Saves the teams; memory changes only once storage has taken them. */ + /** Saves the teams; memory changes once storage has taken them, and the + * promise settles once they are saved. */ private writeTeams(teams: readonly Team[]) { writeTeams(this.storage, teams); this.teamList = Object.freeze(teams); this.state = Object.freeze({ ...this.state, teams: this.teamList }); this.notify(); + return persisted(this.storage, TEAMS_KEY); } /** Saves which imports need an archive check, then keeps it in memory. */ @@ -2165,6 +2176,7 @@ export class AgentsService extends Service implements Agents { writeArchiveUnchecked(this.storage, next); this.unarchiving = next; } + /** Saves the unfinished setups; memory changes only once storage has them. */ private writeUnfinished(unfinished: readonly Unfinished[]) { writeUnfinished(this.storage, unfinished); @@ -2379,12 +2391,15 @@ export class AgentsService extends Service implements Agents { return agent; } - /** Saves one record; memory changes only once storage has taken it. */ + /** Saves one record; memory changes once storage has taken it, and the + * promise settles once it is saved. A failed save keeps memory as it is: + * an agent's timers must not run again because their progress was lost. */ private write(record: AgentRecord) { // Only this agent's key: others may be another program's to change. writeRecord(this.storage, record); this.records = { ...this.records, [record.pubkey]: record }; this.update(); + return persisted(this.storage, recordKey(record.pubkey)); } private live(pubkey: string) { return this.identities.find( @@ -2536,12 +2551,12 @@ export class AgentsService extends Service implements Agents { team.members.some((pubkey) => !live.has(pubkey)), ) ) - this.writeTeams( + void this.writeTeams( this.teamList.map((team) => ({ ...team, members: team.members.filter((pubkey) => live.has(pubkey)), })), - ); + ).catch((error) => console.warn("Agent teams were not saved", error)); } catch (error) { if (turn !== this.listed) return; this.state = Object.freeze({ diff --git a/src/features/agents/state.test.ts b/src/features/agents/state.test.ts index bb74c20d15..00f8ef8411 100644 --- a/src/features/agents/state.test.ts +++ b/src/features/agents/state.test.ts @@ -86,3 +86,22 @@ it("saves each write in order, and a failed write does not stop later ones", asy expect(storage.getItem("a")).toBe("2"); expect(storage.length).toBe(1); }); + +it("puts back a key whose write failed, so it is not taken as saved and can be set again", async () => { + const native = files({ a: "old" }); + const storage = fileBacked(new Map([["a", "old"]]), native.set); + native.set.mockRejectedValueOnce(new Error("disk full")); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + storage.setItem("a", "new"); + storage.setItem("b", "kept"); + await expect(storage.saved("a")).rejects.toThrow("disk full"); + await expect(storage.saved()).rejects.toThrow("disk full"); + await expect(storage.saved("b")).resolves.toBeUndefined(); + error.mockRestore(); + expect(storage.getItem("a")).toBe("old"); + // The same value again is a new write, not a no-op. + storage.setItem("a", "new"); + await storage.saved(); + expect(native.saved.get("a")).toBe("new"); + expect(storage.getItem("a")).toBe("new"); +}); diff --git a/src/features/agents/state.ts b/src/features/agents/state.ts index 7f23f5db1b..e7b45e4199 100644 --- a/src/features/agents/state.ts +++ b/src/features/agents/state.ts @@ -86,7 +86,7 @@ export async function loadAgentStorage( try { if (storage.getItem(key) === null) { storage.setItem(key, value); - await (storage as FileBacked).saved(); + await storage.saved(key); } local.removeItem(key); } catch (error) { @@ -99,24 +99,41 @@ export async function loadAgentStorage( } type FileBacked = Storage & { - /** Settles when every write so far is saved; rejects if the last failed. */ - saved(): Promise; + /** Settles once `key`'s value, or without a key every value, is saved; + * rejects while the last write of one failed. */ + saved(key?: string): Promise; }; -/** A Storage held in memory and saved through `save`, one key at a time. */ +/** A Storage held in memory and saved through `save`, one key at a time. A + * failed write puts back the key's saved value, so the copy never holds what + * is not saved and the same value can be set again. */ export function fileBacked( items: Map, save: Native["set"], ): FileBacked { + /** What each key's file holds, as far as this program saved it. */ + const durable = new Map(items); + /** Each key's last write, until it is saved. */ + const pending = new Map>(); let queue: Promise = Promise.resolve(); const write = (key: string, value: string | null) => { const next = queue.then(() => save(key, value)); - queue = next.catch((error) => { - console.error(`Agent state ${key} was not saved`, error); - }); - return next; + pending.set(key, next); + queue = next.then( + () => { + if (value === null) durable.delete(key); + else durable.set(key, value); + if (pending.get(key) === next) pending.delete(key); + }, + (error) => { + console.error(`Agent state ${key} was not saved`, error); + if (pending.get(key) !== next) return; + const kept = durable.get(key); + if (kept === undefined) items.delete(key); + else items.set(key, kept); + }, + ); }; - let last: Promise = Promise.resolve(); return { get length() { return items.size; @@ -125,15 +142,21 @@ export function fileBacked( getItem: (key) => items.get(key) ?? null, setItem(key, value) { items.set(key, String(value)); - last = write(key, String(value)); + write(key, String(value)); }, removeItem(key) { if (!items.delete(key)) return; - last = write(key, null); + write(key, null); }, clear() { for (const key of [...items.keys()]) this.removeItem(key); }, - saved: () => last, + async saved(key) { + // Writes settle in order, so after the queue each pending one is done. + await queue; + const failed = + key === undefined ? [...pending.values()] : [pending.get(key)]; + await Promise.all(failed); + }, }; } diff --git a/src/features/agents/store.test.ts b/src/features/agents/store.test.ts index c455031c76..d3c45b2c1e 100644 --- a/src/features/agents/store.test.ts +++ b/src/features/agents/store.test.ts @@ -1,4 +1,5 @@ -import { expect, it } from "vitest"; +import { expect, it, vi } from "vitest"; +import { fileBacked } from "./state"; import { STORAGE_KEY, TEAMS_KEY, @@ -266,3 +267,35 @@ it("round-trips teams and skips any that is not one", () => { storage.setItem(TEAMS_KEY, "{"); expect(readTeams(storage)).toEqual([]); }); + +it("keeps the old shared value until every record from it is saved", async () => { + const shared = JSON.stringify({ version: 1, agents: { [pubkey]: record } }); + const disk = new Map([[STORAGE_KEY, shared]]); + const save = vi.fn(async (key: string, value: string | null) => { + if (key === recordKey(pubkey)) throw new Error("disk full"); + if (value === null) disk.delete(key); + else disk.set(key, value); + }); + const storage = fileBacked(new Map(disk), save); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + splitLegacyRecords(storage, readRecords(storage)); + await storage.saved().catch(() => {}); + await new Promise((resolve) => setTimeout(resolve)); + error.mockRestore(); + expect(save).not.toHaveBeenCalledWith(STORAGE_KEY, null); + // The next start still has the agent. + const restarted = fileBacked(new Map(disk), save); + expect(readRecords(restarted)[pubkey]?.config).toEqual(record.config); + // Once its record saves, the split finishes. + save.mockImplementation(async (key, value) => { + if (value === null) disk.delete(key); + else disk.set(key, value); + }); + splitLegacyRecords(restarted, readRecords(restarted)); + await restarted.saved(); + await new Promise((resolve) => setTimeout(resolve)); + expect(disk.has(STORAGE_KEY)).toBe(false); + expect(readRecords(fileBacked(new Map(disk), save))[pubkey]?.config).toEqual( + record.config, + ); +}); diff --git a/src/features/agents/store.ts b/src/features/agents/store.ts index f92a58a786..d88e795b7d 100644 --- a/src/features/agents/store.ts +++ b/src/features/agents/store.ts @@ -144,6 +144,34 @@ function readRecord( }; } +type Saving = { saved?: (key?: string) => Promise }; + +/** Settles once `key`'s value in `storage`, or without a key every value it + * holds, is saved: agent storage files (state.ts) save after they are set, + * other storage as it is set. */ +export function persisted(storage: Storage, key?: string): Promise { + const { saved } = storage as Saving; + return typeof saved === "function" + ? saved.call(storage, key) + : Promise.resolve(); +} + +/** Runs `then` once these keys' values are saved: at once for storage that + * saves as it is set, else later, and not at all if one failed. A move that + * removes its source this way never leaves no saved copy. */ +export function afterSaved( + storage: Storage, + keys: readonly string[], + then: () => void, + failed: (error: unknown) => void, +) { + if (typeof (storage as Saving).saved !== "function") return then(); + void Promise.all(keys.map((key) => persisted(storage, key))).then( + then, + failed, + ); +} + /** Moves records from the old value that held every agent into one key per * agent, then removes it. Only that move writes many records at once: after * it, each save writes only its own agent's key, so a program that runs @@ -154,7 +182,14 @@ export function splitLegacyRecords( ) { if (storage.getItem(STORAGE_KEY) === null) return; for (const record of Object.values(records)) writeRecord(storage, record); - storage.removeItem(STORAGE_KEY); + // The shared value goes only once every record is saved: until then it is + // the only saved copy, and a record of an agent's own wins when both are. + afterSaved( + storage, + Object.keys(records).map(recordKey), + () => storage.removeItem(STORAGE_KEY), + (error) => console.error("Agent records were not split", error), + ); } /** Removes an agent's record. */ From f0909eb8ff7958382985903fcb0b062c9db423c5 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Tue, 13 Oct 2026 19:01:59 -0400 Subject: [PATCH 03/13] fix(agents): show only what agent storage saved, and save long keys A settings, team or attention change was shown before its save finished and stayed shown when the save failed. The settings form then saw no change and turned off Save, and an agent's watch or interest_set edit reported success. Each now goes back to what storage still has when its save fails, and an attention write passes the error to the tool. Only a timer's progress stays, so the timer does not run again at once. The first move out of localStorage deleted a key whose copy failed from the storage the app reads, so the app started with no teams and default settings, and a later save replaced them. Now every key is copied before any leaves localStorage, and if a copy fails, or loading takes longer than 15 seconds, the app uses localStorage until the next start. The agent keys still in localStorage then replace the copies in the folder, since the app wrote them last. A key whose encoded file name is over 255 bytes, such as a Codex session key on a long community host, could not be saved. Its file is now named by the key's SHA-256 and holds the key with the value. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson --- src-tauri/src/agent_state.rs | 108 +++++++++++++++++++++++++--- src/features/agents/runner.ts | 11 +-- src/features/agents/service.test.ts | 55 ++++++++++++++ src/features/agents/service.ts | 42 ++++++++--- src/features/agents/state.test.ts | 62 +++++++++++++--- src/features/agents/state.ts | 69 +++++++++++------- src/main.tsx | 3 +- 7 files changed, 291 insertions(+), 59 deletions(-) diff --git a/src-tauri/src/agent_state.rs b/src-tauri/src/agent_state.rs index 74d8a91a78..7fb1d0485c 100644 --- a/src-tauri/src/agent_state.rs +++ b/src-tauri/src/agent_state.rs @@ -6,16 +6,32 @@ //! One file per key, each replaced whole through a rename, so a crash leaves //! the last complete copy, and two programs that write different keys never //! overwrite each other. The `hosts/` locks decide which program runs an agent. +//! +//! A file's name is its key, encoded. A key whose encoded name is too long for +//! the file system gets a fixed-length name instead, `~` and the SHA-256 of the +//! key, and its file holds `{"key": …, "value": …}` so `load` can recover it. use std::collections::HashMap; use std::fs; use std::io::ErrorKind; use std::path::{Path, PathBuf}; +use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; + /// The largest value kept, as an upper bound on what `localStorage` held. const VALUE_LIMIT: usize = 4 * 1024 * 1024; /// The longest file name macOS, Linux and Windows all allow, in bytes. const NAME_LIMIT: usize = 255; const TEMPORARY: &str = ".tmp"; +/// Starts the name of a file that holds its key. `encode` never writes `~`. +const HASHED: char = '~'; + +/// The contents of a file whose name is a hash of its key. +#[derive(Serialize, Deserialize)] +struct Hashed<'a> { + key: std::borrow::Cow<'a, str>, + value: std::borrow::Cow<'a, str>, +} pub(crate) struct AgentState { dir: Result, @@ -42,11 +58,26 @@ impl AgentState { let mut items = HashMap::new(); for entry in entries.flatten() { let name = entry.file_name(); - let Some(key) = name.to_str().and_then(decode) else { + let Some(name) = name.to_str() else { continue }; + let key = decode(name); + if key.is_none() && !name.starts_with(HASHED) { + continue; + } + let Ok(text) = fs::read_to_string(entry.path()) else { continue; }; - if let Ok(value) = fs::read_to_string(entry.path()) { - items.insert(key, value); + match key { + Some(key) => { + items.insert(key, text); + } + None => { + // Skip a file whose key does not give its name. + if let Ok(saved) = serde_json::from_str::(&text) { + if file_name(&saved.key) == name { + items.insert(saved.key.into_owned(), saved.value.into_owned()); + } + } + } } } Ok(items) @@ -54,12 +85,12 @@ impl AgentState { /// Saves `value` under `key`, or removes the key when `value` is `None`. pub(crate) fn set(&self, key: &str, value: Option<&str>) -> Result<(), String> { - // The limit is on the file name, after `encode` lengthens the key. - if key.is_empty() || encode(key).len() > NAME_LIMIT { + if key.is_empty() { return Err("Invalid agent state key".into()); } let dir = self.dir()?; - let path = dir.join(encode(key)); + let name = file_name(key); + let path = dir.join(&name); let Some(value) = value else { return match fs::remove_file(&path) { Err(error) if error.kind() != ErrorKind::NotFound => { @@ -71,12 +102,23 @@ impl AgentState { if value.len() > VALUE_LIMIT { return Err("Agent state is too large".into()); } + let hashed; + let contents = if name.starts_with(HASHED) { + hashed = serde_json::to_string(&Hashed { + key: key.into(), + value: value.into(), + }) + .map_err(|_| "Could not save agent state")?; + hashed.as_str() + } else { + value + }; let saved = (|| { fs::create_dir_all(dir)?; let mut file = tempfile::Builder::new() .suffix(TEMPORARY) .tempfile_in(dir)?; - std::io::Write::write_all(&mut file, value.as_bytes())?; + std::io::Write::write_all(&mut file, contents.as_bytes())?; file.as_file().sync_all()?; file.persist(&path).map_err(|error| error.error)?; Ok::<_, std::io::Error>(()) @@ -85,6 +127,16 @@ impl AgentState { } } +/// The file `key` is saved in: its encoded name, or `~` and the key's SHA-256 +/// when that name is too long. +pub(crate) fn file_name(key: &str) -> String { + let name = encode(key); + if name.len() <= NAME_LIMIT { + return name; + } + format!("{HASHED}{:x}", Sha256::digest(key.as_bytes())) +} + /// A file name for `key`: bytes other than `A-Z a-z 0-9 . _ -` become `%XX`. pub(crate) fn encode(key: &str) -> String { let mut name = String::with_capacity(key.len()); @@ -178,9 +230,47 @@ mod tests { state.set("missing", None).unwrap(); assert_eq!(state.load().unwrap().keys().collect::>(), ["two"]); assert!(state.set("", Some("x")).is_err()); - assert!(state.set(&"k".repeat(NAME_LIMIT + 1), Some("x")).is_err()); - assert!(state.set(&":".repeat(86), Some("x")).is_err()); state.set(&"k".repeat(NAME_LIMIT), Some("x")).unwrap(); + assert!(root + .path() + .join("state") + .join("k".repeat(NAME_LIMIT)) + .exists()); + } + + #[test] + fn saves_keys_whose_names_are_too_long_under_a_hash() { + let root = tempfile::tempdir().unwrap(); + let dir = root.path().join("state"); + let state = AgentState::new(Ok(dir.clone())); + // 251 bytes, as Codex names it for a community on a 95-character host; + // `encode` makes it 263. + let key = format!( + "buzz.codex.sessions.v4:https://agents.production.us-west-2.buzz-workspace.platform.engineering.internal.tools.example.com:{}:{}", + "a".repeat(64), + "b".repeat(64), + ); + assert_eq!(key.len(), 251); + assert!(encode(&key).len() > NAME_LIMIT); + for key in [key.as_str(), &":".repeat(86), &"k".repeat(NAME_LIMIT + 1)] { + state.set(key, Some("{\"a\":1}")).unwrap(); + let name = file_name(key); + assert!(name.starts_with(HASHED) && name.len() == 65, "{name}"); + assert!(dir.join(&name).exists()); + assert_eq!(state.load().unwrap()[key], "{\"a\":1}"); + state.set(key, Some("2")).unwrap(); + assert_eq!(state.load().unwrap()[key], "2"); + state.set(key, None).unwrap(); + assert!(!state.load().unwrap().contains_key(key)); + } + // A hashed file whose key does not give its name is not loaded. + fs::write( + dir.join(format!("{HASHED}{}", "0".repeat(64))), + r#"{"key":"other","value":"x"}"#, + ) + .unwrap(); + fs::write(dir.join(format!("{HASHED}bad")), "not json").unwrap(); + assert!(state.load().unwrap().is_empty()); } #[test] diff --git a/src/features/agents/runner.ts b/src/features/agents/runner.ts index 13e30a869c..9745b9d9ab 100644 --- a/src/features/agents/runner.ts +++ b/src/features/agents/runner.ts @@ -333,8 +333,11 @@ export type RunnerHost = { saved(pubkey: string): AgentRecord | undefined; /** Its record as its attention handle reads it; throws once it is gone. */ record(pubkey: string): AgentRecord; - /** Saves one record; throws when storage refuses it. */ - write(record: AgentRecord): void; + /** Saves one record; rejects when it was not saved. */ + write(record: AgentRecord): Promise; + /** Saves timers' progress. It stays in memory even if the save later fails, + * so a timer does not run again at once; throws when storage refuses it. */ + progress(record: AgentRecord): void; /** The team sections of its prompt; empty without a team. */ team(pubkey: string): string; /** What its next run starts with, from this device's storage only. */ @@ -1589,7 +1592,7 @@ export class AgentRunners { if (next && ran) timers = { ...timers, [next.slug]: ran }; if (timers !== prior) try { - this.host.write({ ...record, timers }); + this.host.progress({ ...record, timers }); } catch (error) { // Unsaved, it would run again at once; it waits for storage instead. console.warn(`Agent ${agent.name} timer state was not saved`, error); @@ -2058,7 +2061,7 @@ export class AgentRunners { `${slug} is changing; show it again`, ); } - this.host.write(after); + await this.host.write(after); return show(after, slug, seconds()); } }, diff --git a/src/features/agents/service.test.ts b/src/features/agents/service.test.ts index 5ac74ca95d..399fc35dc0 100644 --- a/src/features/agents/service.test.ts +++ b/src/features/agents/service.test.ts @@ -2274,9 +2274,30 @@ it("rejects a save that agent storage could not write, and saves it on retry", a ).rejects.toThrow("disk full"); error.mockRestore(); expect(disk.get(`buzz.agent.v1:${bot}`)).not.toContain("unsaved"); + // The agent shows what is saved, so a settings form's draft differs from it + // again and its Save is enabled. + expect(service.find(bot)?.config).toEqual({ reply: "ok" }); failing = false; await service.save(bot, { config: { reply: "unsaved" } }); expect(disk.get(`buzz.agent.v1:${bot}`)).toContain("unsaved"); + expect(service.find(bot)?.config).toEqual({ reply: "unsaved" }); +}); + +it("shows the saved teams after a team save agent storage could not write", async () => { + let failing = false; + const storage = fileBacked(new Map(), async () => { + if (failing) throw new Error("disk full"); + }); + const { service } = await setup({ storage }); + await service.create({ type: "example/echo", name: "Echo" }); + await service.saveTeam({ name: "Crew", instructions: "", members: [bot] }); + failing = true; + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + await expect( + service.saveTeam({ name: "Solo", instructions: "", members: [bot] }), + ).rejects.toThrow("disk full"); + error.mockRestore(); + expect(service.snapshot().teams.map((team) => team.name)).toEqual(["Crew"]); }); it("saves nothing when native create fails", async () => { @@ -5857,6 +5878,40 @@ it("lets a run read and write its attention with expected-state tokens", async ( ).rejects.toThrow(/^conflict:/); }); +it("fails an agent's attention write that agent storage could not save, and keeps the saved value", async () => { + let failing = false; + const storage = fileBacked(new Map(), async () => { + if (failing) throw new Error("disk full"); + }); + const { service, run, emit } = await setup({ storage }); + await service.create({ type: "example/echo", name: "Echo" }); + await service.save(bot, { attentionEnabled: true }); + const { attention } = await handleOf(run, emit); + const shown = await attention.show("interest/default"); + failing = true; + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + await expect( + attention.write( + "interest/default", + { type: "interest", instructions: "Unsaved." }, + { expected: shown.expected_state }, + ), + ).rejects.toThrow("disk full"); + error.mockRestore(); + expect(await attention.show("interest/default")).toEqual(shown); + expect(service.find(bot)?.attention["interest/default"]?.value).toEqual({ + type: "interest", + instructions: "Be brief.", + }); + failing = false; + const written = await attention.write( + "interest/default", + { type: "interest", instructions: "Saved." }, + { expected: shown.expected_state }, + ); + expect(written.object).toMatchObject({ instructions: "Saved." }); +}); + it("keeps attention off across a reload", async () => { const { service, storage, native } = await setup(); await service.create({ type: "example/echo", name: "Echo" }); diff --git a/src/features/agents/service.ts b/src/features/agents/service.ts index a955831654..9f8c8d1588 100644 --- a/src/features/agents/service.ts +++ b/src/features/agents/service.ts @@ -96,6 +96,7 @@ import { import { readArchiveUnchecked, persisted, + readOne, readRecords, recordKey, readTeams, @@ -1099,9 +1100,10 @@ export class AgentsService extends Service implements Agents { if (!identity) throw new Error("No such agent on this device"); return this.recordOf(identity); }, - // A failed save logs; the run's memory keeps it from running again. - write: (record) => { - void this.write(record).catch((error) => + write: (record) => this.write(record), + // A failed save logs; memory keeps it, so the timer does not run again. + progress: (record) => { + void this.write(record, true).catch((error) => console.warn(`Agent ${record.pubkey} state was not saved`, error), ); }, @@ -2158,13 +2160,21 @@ export class AgentsService extends Service implements Agents { } /** Saves the teams; memory changes once storage has taken them, and the - * promise settles once they are saved. */ + * promise settles once they are saved. When the save fails, memory goes + * back to the teams storage still has. */ private writeTeams(teams: readonly Team[]) { writeTeams(this.storage, teams); + const shown = this.showTeams(teams); + return persisted(this.storage, TEAMS_KEY).catch((error) => { + if (this.teamList === shown) this.showTeams(readTeams(this.storage)); + throw error; + }); + } + private showTeams(teams: readonly Team[]) { this.teamList = Object.freeze(teams); this.state = Object.freeze({ ...this.state, teams: this.teamList }); this.notify(); - return persisted(this.storage, TEAMS_KEY); + return this.teamList; } /** Saves which imports need an archive check, then keeps it in memory. */ @@ -2392,14 +2402,26 @@ export class AgentsService extends Service implements Agents { } /** Saves one record; memory changes once storage has taken it, and the - * promise settles once it is saved. A failed save keeps memory as it is: - * an agent's timers must not run again because their progress was lost. */ - private write(record: AgentRecord) { + * promise settles once it is saved. When the save fails, memory goes back + * to what storage still has, so the owner and the agent see the change was + * not made, unless `keep`: timer progress stays, so a timer whose progress + * was lost does not run again at once. */ + private write(record: AgentRecord, keep = false) { + const { pubkey } = record; // Only this agent's key: others may be another program's to change. writeRecord(this.storage, record); - this.records = { ...this.records, [record.pubkey]: record }; + this.records = { ...this.records, [pubkey]: record }; this.update(); - return persisted(this.storage, recordKey(record.pubkey)); + return persisted(this.storage, recordKey(pubkey)).catch((error) => { + // Unless a later write replaced it, which settles on its own. + if (!keep && this.records[pubkey] === record) { + const { [pubkey]: _, ...others } = this.records; + const saved = readOne(this.storage, pubkey); + this.records = saved ? { ...others, [pubkey]: saved } : others; + this.update(); + } + throw error; + }); } private live(pubkey: string) { return this.identities.find( diff --git a/src/features/agents/state.test.ts b/src/features/agents/state.test.ts index 00f8ef8411..3748ae8af3 100644 --- a/src/features/agents/state.test.ts +++ b/src/features/agents/state.test.ts @@ -14,10 +14,10 @@ const files = (items: Record = {}) => { }; }; -it("moves agent state from localStorage, keeping what agent storage has", async () => { +it("moves agent state from localStorage, which replaces what agent storage has", async () => { const local = memoryStorage(); local.setItem("buzz.agents.v1", "records"); - local.setItem("buzz.agents.teams.v1", "old teams"); + local.setItem("buzz.agents.teams.v1", "new teams"); local.setItem("buzz.claude-code.sessions.v1", "sessions"); local.setItem("buzz.codex.sessions.v4:scope:ab", "codex"); local.setItem("buzz.codex.provider-defaults.v1", "codex defaults"); @@ -25,11 +25,15 @@ it("moves agent state from localStorage, keeping what agent storage has", async local.setItem("buzz.agents.archive-unchecked.v1", "unchecked"); local.setItem("buzz.agents.catch-up.v1", "checkpoints"); local.setItem("buzz.theme", "dark"); - const native = files({ "buzz.agents.teams.v1": "shared teams" }); + const native = files({ + "buzz.agents.teams.v1": "stale teams", + "buzz.agent.v1:aa": "kept", + }); const storage = await loadAgentStorage(local, native); expect(Object.fromEntries(native.saved)).toEqual({ "buzz.agents.v1": "records", - "buzz.agents.teams.v1": "shared teams", + "buzz.agents.teams.v1": "new teams", + "buzz.agent.v1:aa": "kept", "buzz.claude-code.sessions.v1": "sessions", "buzz.codex.sessions.v4:scope:ab": "codex", "buzz.codex.provider-defaults.v1": "codex defaults", @@ -37,21 +41,42 @@ it("moves agent state from localStorage, keeping what agent storage has", async "buzz.agents.archive-unchecked.v1": "unchecked", "buzz.agents.catch-up.v1": "checkpoints", }); - expect(storage.getItem("buzz.agents.teams.v1")).toBe("shared teams"); + expect(storage.getItem("buzz.agents.teams.v1")).toBe("new teams"); // Moved keys leave localStorage; other keys stay. expect(local.length).toBe(1); expect(local.getItem("buzz.theme")).toBe("dark"); }); -it("keeps a key in localStorage when it could not be moved", async () => { +it("uses localStorage for a start where a copy fails, and keeps its state through a later move", async () => { const local = memoryStorage(); local.setItem("buzz.agents.v1", "records"); + local.setItem("buzz.agents.teams.v1", "teams"); const native = files(); + // The records copy is saved; the teams copy fails. + native.set.mockResolvedValueOnce(undefined); native.set.mockRejectedValueOnce(new Error("disk full")); const error = vi.spyOn(console, "error").mockImplementation(() => {}); - await loadAgentStorage(local, native); + const first = await loadAgentStorage(local, native); error.mockRestore(); - expect(local.getItem("buzz.agents.v1")).toBe("records"); + expect(first).toBe(local); + expect(first.getItem("buzz.agents.v1")).toBe("records"); + expect(first.getItem("buzz.agents.teams.v1")).toBe("teams"); + // The owner saves while the app uses localStorage. + first.setItem("buzz.agents.v1", "saved records"); + + // The disk recovers: the next start moves everything, the newer records too. + const second = await loadAgentStorage(local, native); + expect(second).not.toBe(local); + expect(local.getItem("buzz.agents.v1")).toBeNull(); + second.setItem("buzz.agent.v1:aa", "saved again"); + await vi.waitFor(() => + expect(native.saved.get("buzz.agent.v1:aa")).toBe("saved again"), + ); + + const third = await loadAgentStorage(local, native); + expect(third.getItem("buzz.agents.teams.v1")).toBe("teams"); + expect(third.getItem("buzz.agents.v1")).toBe("saved records"); + expect(third.getItem("buzz.agent.v1:aa")).toBe("saved again"); }); it("uses localStorage when agent storage cannot be read", async () => { @@ -64,6 +89,27 @@ it("uses localStorage when agent storage cannot be read", async () => { error.mockRestore(); }); +it("uses localStorage when agent storage does not answer, and leaves it as it was", async () => { + vi.useFakeTimers(); + const local = memoryStorage(); + local.setItem("buzz.agents.v1", "records"); + const native = files(); + let answer = () => {}; + native.load.mockImplementationOnce( + () => new Promise((resolve) => (answer = () => resolve({}))), + ); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + const started = loadAgentStorage(local, native, 1_000); + await vi.advanceTimersByTimeAsync(1_000); + expect(await started).toBe(local); + // A late answer moves nothing: the app is using localStorage. + answer(); + await vi.runAllTimersAsync(); + error.mockRestore(); + vi.useRealTimers(); + expect(local.getItem("buzz.agents.v1")).toBe("records"); +}); + it("saves each write in order, and a failed write does not stop later ones", async () => { const native = files(); const storage = fileBacked(new Map(), native.set); diff --git a/src/features/agents/state.ts b/src/features/agents/state.ts index e7b45e4199..7a57028fab 100644 --- a/src/features/agents/state.ts +++ b/src/features/agents/state.ts @@ -57,45 +57,60 @@ const native: Native = { }; /** Reads agent storage before the app starts. It moves agent state the app - * kept in localStorage into it: each key it does not have yet. If agent storage - * cannot be read, the app keeps using localStorage. */ + * kept in localStorage into it. Agent keys are in localStorage only before + * the first move, or when an earlier start could not use agent storage, so + * they are the newer copy and replace what agent storage has. They leave + * localStorage only once every one is saved; if agent storage cannot be read, + * a copy fails, or this takes longer than `timeoutMs`, the app uses + * localStorage until the next start. */ export async function loadAgentStorage( local: Storage = globalThis.localStorage, files: Native | undefined = isTauri() ? native : undefined, + timeoutMs = 15_000, ): Promise { if (!files) return local; - let items: Record; + let late = false; + const move = async () => { + const storage = fileBacked( + new Map(Object.entries(await files.load())), + files.set, + ); + const moved: string[] = []; + for (let index = 0; index < local.length; index++) { + const key = local.key(index); + if (key && isAgentKey(key)) moved.push(key); + } + for (const key of moved) { + const value = local.getItem(key); + if (value !== null && storage.getItem(key) !== value) { + storage.setItem(key, value); + } + } + await storage.saved(); + // Too late: the app already started with localStorage, which keeps them. + if (late) throw new Error("Agent storage took too long"); + for (const key of moved) local.removeItem(key); + loaded = storage; + return storage; + }; + let timer: ReturnType | undefined; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => { + late = true; + reject(new Error("Agent storage took too long")); + }, timeoutMs); + }); try { - items = await files.load(); + return await Promise.race([move(), timeout]); } catch (error) { console.error( - "Agent storage could not be read; using this window's", + "Agent storage could not be used; using this window's", error, ); return local; + } finally { + clearTimeout(timer); } - const storage = fileBacked(new Map(Object.entries(items)), files.set); - const moved: string[] = []; - for (let index = 0; index < local.length; index++) { - const key = local.key(index); - if (key && isAgentKey(key)) moved.push(key); - } - for (const key of moved) { - const value = local.getItem(key); - if (value === null) continue; - try { - if (storage.getItem(key) === null) { - storage.setItem(key, value); - await storage.saved(key); - } - local.removeItem(key); - } catch (error) { - // Kept in localStorage, so the next start tries again. - console.error(`Agent state ${key} could not be moved`, error); - } - } - loaded = storage; - return storage; } type FileBacked = Storage & { diff --git a/src/main.tsx b/src/main.tsx index f4a099e49e..930b41ad1e 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -19,7 +19,8 @@ if (!container) { } // Agent storage is read before any service, since its reads are synchronous. -void loadAgentStorage().then(() => { +// It settles, with localStorage at worst, so the window always starts. +void loadAgentStorage().finally(() => { const services = createServices(); const root = createRoot(container); function HostApp() { From 060191a4c25cfdad8f565d6c1db81f8239216549 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Wed, 14 Oct 2026 16:16:18 -0400 Subject: [PATCH 04/13] fix(agents): keep the moved agent state, and show settings only once saved Each start let agent keys in localStorage replace the agent storage files. After the first move, localStorage has no agent state, so a start that could not use agent storage began with none, and what it saved then replaced the full files at the next start: for example, a session list with one conversation replaced one with three. Agent storage now keeps a marker once every key was moved. After that, the files win, and keys from such a start only fill in keys the files do not have. A settings save was shown before it was saved. The Codex settings form resets its draft when the shown settings change, so when the save failed, the owner's edits were replaced with the saved settings and Save was turned off. Settings are now shown only once they are saved. Later changes still build on a write that is being saved. Timer progress is still shown at once. The Claude Code and Codex provider defaults dialogs closed before the defaults were saved, and a failed save was lost at the next start. They now wait for the save, close only when it succeeds, and otherwise keep the draft and show the error, so the owner can try again. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson --- src/bundled/claude-code/defaults.ts | 6 ++- src/bundled/claude-code/tabs.test.tsx | 52 +++++++++++++++++++++++-- src/bundled/claude-code/tabs.tsx | 20 ++++------ src/bundled/codex/tabs.test.tsx | 56 ++++++++++++++++++++++----- src/bundled/codex/tabs.tsx | 53 +++++++++++++------------ src/features/agents/service.test.ts | 8 +++- src/features/agents/service.ts | 52 ++++++++++++++++++------- src/features/agents/state.test.ts | 33 +++++++++++++++- src/features/agents/state.ts | 38 +++++++++++++----- 9 files changed, 239 insertions(+), 79 deletions(-) diff --git a/src/bundled/claude-code/defaults.ts b/src/bundled/claude-code/defaults.ts index 095e44698f..5b85f70fd5 100644 --- a/src/bundled/claude-code/defaults.ts +++ b/src/bundled/claude-code/defaults.ts @@ -1,3 +1,4 @@ +import { persisted } from "../../features/agents/store"; import { config, type Config } from "./runtime"; export type ClaudeProviderDefaults = Readonly<{ @@ -34,12 +35,15 @@ export class ClaudeDefaults { : ""), }; } - save(next: ClaudeProviderDefaults) { + /** Settles once the defaults are saved, and only then uses them; rejects, + * keeping the saved ones, when they could not be saved. */ + async save(next: ClaudeProviderDefaults) { const { model, effort = "" } = next; this.storage.setItem( STORAGE_KEY, JSON.stringify({ version: 1, model, effort }), ); + await persisted(this.storage, STORAGE_KEY); this.state = Object.freeze({ model, effort }); for (const listener of this.listeners) listener(); } diff --git a/src/bundled/claude-code/tabs.test.tsx b/src/bundled/claude-code/tabs.test.tsx index 07e22376fd..8aa6bdb2f3 100644 --- a/src/bundled/claude-code/tabs.test.tsx +++ b/src/bundled/claude-code/tabs.test.tsx @@ -18,6 +18,7 @@ import { StrictMode } from "react"; import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, expect, it, vi } from "vitest"; import type { Agent, AgentType } from "../../features/agents/service"; +import { fileBacked } from "../../features/agents/state"; import type { HostProcessOptions } from "../../features/host/service"; import { ClaudeLogoIcon } from "../../shared/design-system/icons"; import { apply, inject } from "./index"; @@ -108,7 +109,10 @@ function applied({ latest = version, files = {}, catalog = { listed: true }, + storage, }: { + /** Agent storage; this page's localStorage without it. */ + storage?: Storage; loggedIn?: boolean; available?: boolean; version?: string; @@ -236,6 +240,7 @@ function applied({ })), }, agents: { + storage, register: (registered: AgentType) => { type = registered; }, @@ -511,11 +516,48 @@ it("edits the model through the Settings dropdown and preserves agent inheritanc expect(save).not.toHaveBeenCalled(); }); -it("preserves native behavior for invalid defaults and keeps explicit agent models", () => { +it("keeps the saved default and shows the error while a default could not be saved", async () => { + let failing = true; + const storage = fileBacked(new Map(), async () => { + if (failing) throw new Error("disk full"); + }); + const Control = installed({ storage }).type.provider?.DefaultControl; + if (!Control) throw new Error("Missing default control"); + render(); + const control = await screen.findByRole("button", { + name: /^Claude default model:/, + }); + await waitFor(() => expect(control).toHaveTextContent("Opus 5.5")); + const pick = async () => { + fireEvent.click(control); + fireEvent.click(await screen.findByRole("menuitem", { name: "Claude" })); + fireEvent.click( + await screen.findByRole("menuitemradio", { name: "Fable 5.1" }), + ); + }; + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + await pick(); + expect(await screen.findByRole("alert")).toHaveTextContent("disk full"); + error.mockRestore(); + expect(control).toHaveTextContent("Opus 5.5"); + expect(storage.getItem("buzz.claude-code.provider-defaults.v1")).toBeNull(); + // The same choice can be saved again. + failing = false; + await pick(); + await waitFor(() => + expect(screen.queryByRole("alert")).not.toBeInTheDocument(), + ); + expect(control).toHaveTextContent("Fable 5.1"); + expect(storage.getItem("buzz.claude-code.provider-defaults.v1")).toContain( + "fable", + ); +}); + +it("preserves native behavior for invalid defaults and keeps explicit agent models", async () => { localStorage.setItem("buzz.claude-code.provider-defaults.v1", "not json"); const defaults = new ClaudeDefaults(localStorage); expect(defaults.resolve(DEFAULT_CONFIG)).toEqual(DEFAULT_CONFIG); - defaults.save({ model: "sonnet" }); + await defaults.save({ model: "sonnet" }); const inherited = { ...DEFAULT_CONFIG }; expect(defaults.resolve(inherited).model).toBe("sonnet"); expect(inherited).toEqual(DEFAULT_CONFIG); @@ -1074,8 +1116,10 @@ it("saves model-specific default effort from the nested menu and inherits it", a await screen.findByRole("menuitemradio", { name: "Sonnet 5.5" }), ); fireEvent.click(await screen.findByRole("menuitemradio", { name: "High" })); - expect(control).toHaveAccessibleName( - "Claude default model: Sonnet 5.5, High", + await waitFor(() => + expect(control).toHaveAccessibleName( + "Claude default model: Sonnet 5.5, High", + ), ); const reopened = new ClaudeDefaults(localStorage); diff --git a/src/bundled/claude-code/tabs.tsx b/src/bundled/claude-code/tabs.tsx index 1cdc34280e..d4d7d463da 100644 --- a/src/bundled/claude-code/tabs.tsx +++ b/src/bundled/claude-code/tabs.tsx @@ -435,12 +435,10 @@ export function createTabs({ effort={saved.effort ?? ""} effortLabel={saved.effort ? effortLabel(saved.effort) : "Automatic"} onEffortChange={(model, effort) => { - try { - defaults.save({ model, effort }); - setError(""); - } catch (reason) { - setError(String(reason)); - } + defaults.save({ model, effort }).then( + () => setError(""), + (reason) => setError(String(reason)), + ); }} value={ saved.model @@ -452,12 +450,10 @@ export function createTabs({ defaultLabel={providerDefaultLabel("", state.models)} onConfigure={onConfigure} onChange={(model) => { - try { - defaults.save({ model }); - setError(""); - } catch (reason) { - setError(String(reason)); - } + defaults.save({ model }).then( + () => setError(""), + (reason) => setError(String(reason)), + ); }} /> diff --git a/src/bundled/codex/tabs.test.tsx b/src/bundled/codex/tabs.test.tsx index ada3eea018..4223eae6eb 100644 --- a/src/bundled/codex/tabs.test.tsx +++ b/src/bundled/codex/tabs.test.tsx @@ -18,6 +18,7 @@ import { StrictMode } from "react"; import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, expect, it, vi } from "vitest"; import { ownerReads } from "../../features/agents/reads"; +import { fileBacked } from "../../features/agents/state"; import type { AgentViewProps } from "../../features/agents/service"; import type { HostProcessOptions } from "../../features/host/service"; import type { RelayData } from "../../features/relay/service"; @@ -58,7 +59,10 @@ function fixture({ turns = () => ({ result: { data: [] } }), threads = [], configured = {}, + storage = localStorage, }: { + /** Agent storage, which keeps the provider defaults. */ + storage?: Storage; signedIn?: boolean; installed?: boolean; saved?: Record; @@ -165,7 +169,7 @@ function fixture({ return process; }); const setup = new CodexSetup(spawn); - const provider = new CodexProvider(spawn, localStorage, setup); + const provider = new CodexProvider(spawn, storage, setup); const relay = { snapshot: () => ({ status: "ready", @@ -444,6 +448,7 @@ it("keeps provider readiness quiet and shows recovery when needed", async () => await user.click(screen.getByLabelText("Default thinking")); await user.click(await screen.findByRole("option", { name: "High" })); fireEvent.click(screen.getByRole("button", { name: "Save defaults" })); + await waitFor(() => expect(onClose).toHaveBeenCalledOnce()); expect(f.provider.resolve(defaults)).toMatchObject({ model: "", effort: "high", @@ -475,10 +480,39 @@ it("keeps provider readiness quiet and shows recovery when needed", async () => expect(screen.getByRole("button", { name: "Sign in" })).toBeVisible(); expect(screen.queryByText("Connection")).not.toBeInTheDocument(); }); +it("keeps the provider dialog and its draft open while defaults could not be saved", async () => { + let failing = true; + const storage = fileBacked(new Map(), async () => { + if (failing) throw new Error("disk full"); + }); + const f = fixture({ storage }); + const onClose = vi.fn(); + render(); + await waitFor(() => + expect(screen.getByLabelText("Default thinking")).toBeEnabled(), + ); + const user = userEvent.setup(); + await user.click(screen.getByLabelText("Default thinking")); + await user.click(await screen.findByRole("option", { name: "High" })); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + fireEvent.click(screen.getByRole("button", { name: "Save defaults" })); + expect(await screen.findByRole("alert")).toHaveTextContent("disk full"); + error.mockRestore(); + expect(onClose).not.toHaveBeenCalled(); + expect(screen.getByLabelText("Default thinking")).toHaveTextContent("High"); + // Nothing inherits defaults that were not saved. + expect(f.provider.snapshot().defaults.effort).toBe(""); + failing = false; + fireEvent.click(screen.getByRole("button", { name: "Save defaults" })); + await waitFor(() => expect(onClose).toHaveBeenCalledOnce()); + expect(f.provider.resolve(defaults).effort).toBe("high"); + expect(storage.getItem("buzz.codex.provider-defaults.v1")).toContain("high"); +}); + it("applies provider precedence without rewriting records and falls back for incompatible thinking", async () => { const f = fixture(); await f.provider.check(); - f.provider.saveDefaults({ model: "model-a", effort: "high" }); + await f.provider.saveDefaults({ model: "model-a", effort: "high" }); const inherited = { ...defaults }; expect(f.provider.resolve(inherited)).toMatchObject({ model: "model-a", @@ -493,7 +527,7 @@ it("applies provider precedence without rewriting records and falls back for inc f.provider.resolve({ ...defaults, model: "model-b", effort: "high" }), ).toMatchObject({ model: "model-b", effort: "high" }); - f.provider.saveDefaults({ model: "model-b", effort: "medium" }); + await f.provider.saveDefaults({ model: "model-b", effort: "medium" }); render(); await waitFor(() => expect(screen.getByRole("combobox", { name: "Model" })).toHaveTextContent( @@ -529,12 +563,12 @@ it("selects a model in the Settings dropdown without changing local Codex prefer expect( new CodexProvider(f.spawn, localStorage).resolve(defaults), ).toMatchObject({ model: "model-b", effort: "medium" }); - expect(control).toHaveTextContent("Medium"); + await waitFor(() => expect(control).toHaveTextContent("Medium")); fireEvent.click(control); fireEvent.click( await screen.findByRole("menuitem", { name: "Use Codex recommended" }), ); - expect(f.provider.snapshot().defaults.model).toBe(""); + await waitFor(() => expect(f.provider.snapshot().defaults.model).toBe("")); expect(configured).toEqual({ model: "model-b", @@ -633,7 +667,7 @@ it("shows a provider model recheck inside a focusable inactive picker", async () it("keeps model-default provider thinking dynamic across save and reopen", async () => { const f = fixture(); await f.provider.check(); - f.provider.saveDefaults({ model: "", effort: "high" }); + await f.provider.saveDefaults({ model: "", effort: "high" }); const onClose = vi.fn(); const view = render(); const user = userEvent.setup(); @@ -1304,8 +1338,10 @@ it("keeps the recommended model inherited when only effort changes", async () => await screen.findByRole("menuitemradio", { name: "Model A" }), ); fireEvent.click(await screen.findByRole("menuitemradio", { name: "High" })); - expect(f.provider.snapshot().defaults).toMatchObject({ - model: "", - effort: "high", - }); + await waitFor(() => + expect(f.provider.snapshot().defaults).toMatchObject({ + model: "", + effort: "high", + }), + ); }); diff --git a/src/bundled/codex/tabs.tsx b/src/bundled/codex/tabs.tsx index 0a87980c6f..f9023c6008 100644 --- a/src/bundled/codex/tabs.tsx +++ b/src/bundled/codex/tabs.tsx @@ -24,6 +24,7 @@ import type { AgentProviderSnapshot, AgentViewProps, } from "../../features/agents/service"; +import { persisted } from "../../features/agents/store"; import { PanelSubview } from "../../features/panels/PanelSubview"; import { Button } from "../../shared/design-system/ui/Button"; import { Field } from "../../shared/design-system/ui/Field"; @@ -187,7 +188,9 @@ export class CodexProvider { if (this.abort === abort) this.abort = undefined; } } - saveDefaults(next: ProviderDefaults) { + /** Settles once the defaults are saved, and only then uses them; rejects, + * keeping the saved ones, when they could not be saved. */ + async saveDefaults(next: ProviderDefaults) { const selected = this.model(next.model); const effort = !next.effort || @@ -197,7 +200,11 @@ export class CodexProvider { ? next.effort : (selected?.defaultReasoningEffort ?? ""); const defaults = Object.freeze({ model: next.model, effort }); - this.writeDefaults(defaults); + this.storage.setItem( + STORAGE_KEY, + JSON.stringify({ version: 1, ...defaults }), + ); + await persisted(this.storage, STORAGE_KEY); this.set({ defaults }); } resolve(raw: unknown): Config { @@ -230,12 +237,6 @@ export class CodexProvider { private model(model: string) { return catalogModel(model, this.state.data); } - private writeDefaults(defaults: ProviderDefaults) { - this.storage.setItem( - STORAGE_KEY, - JSON.stringify({ version: 1, ...defaults }), - ); - } private readDefaults(): ProviderDefaults { try { const value: unknown = JSON.parse( @@ -441,24 +442,20 @@ export function createTabs( : "Automatic" } onEffortChange={(model, effort) => { - try { - provider.saveDefaults({ model, effort }); - setError(""); - } catch (reason) { - setError(String(reason)); - } + provider.saveDefaults({ model, effort }).then( + () => setError(""), + (reason) => setError(String(reason)), + ); }} value={state.defaults.model} defaultModel={catalogModel("", state.data)?.model} defaultLabel={providerDefaultLabel("", state.data)} onConfigure={onConfigure} onChange={(model) => { - try { - provider.saveDefaults({ model, effort: "" }); - setError(""); - } catch (reason) { - setError(String(reason)); - } + provider.saveDefaults({ model, effort: "" }).then( + () => setError(""), + (reason) => setError(String(reason)), + ); }} /> {error && {error}} @@ -477,6 +474,7 @@ export function createTabs( ); const [draft, setDraft] = useState(state.defaults); const [saveError, setSaveError] = useState(""); + const [savingDefaults, setSavingDefaults] = useState(false); const action = error === NOT_INSTALLED ? ({ step: "install", label: "Install Codex" } as const) @@ -555,14 +553,14 @@ export function createTabs( hidden={state.readiness !== "ready"} onSubmit={(event) => { event.preventDefault(); + if (savingDefaults) return; setSaveError(""); - try { - provider.saveDefaults(draft); - } catch (reason) { - setSaveError(message(reason)); - return; - } - onClose(); + setSavingDefaults(true); + // Closes only once saved; else the draft stays to try again. + provider + .saveDefaults(draft) + .then(onClose, (reason) => setSaveError(message(reason))) + .finally(() => setSavingDefaults(false)); }} >