diff --git a/src-tauri/src/identity.rs b/src-tauri/src/identity.rs index 1f4f74d6de..c569e35b62 100644 --- a/src-tauri/src/identity.rs +++ b/src-tauri/src/identity.rs @@ -532,12 +532,14 @@ fn validate_sidebar_payload(coordinate: &str, value: &serde_json::Value) -> Resu "main-agent" => { let agent = data.get("agent").ok_or_else(invalid)?; + // A null agent saves none: the owner removed their Bestie. if data.len() != 2 - || !agent.as_str().is_some_and(|v| { - v.len() == 64 - && v.bytes() - .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) - }) + || !(agent.is_null() + || agent.as_str().is_some_and(|v| { + v.len() == 64 + && v.bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) + })) { return Err(invalid()); } diff --git a/src-tauri/src/identity/tests.rs b/src-tauri/src/identity/tests.rs index 2318eca334..0f9c27c6c3 100644 --- a/src-tauri/src/identity/tests.rs +++ b/src-tauri/src/identity/tests.rs @@ -733,7 +733,7 @@ pub(crate) fn write_denied_fixture() -> (IdentityHost, impl Fn()) { } #[tokio::test] -async fn main_agent_signer_admits_only_one_agent() { +async fn main_agent_signer_admits_one_agent_or_none() { use serde_json::json; let host = super::IdentityHost::fixture(); let payload = json!({"version": 1, "agent": "ab".repeat(32)}); @@ -746,9 +746,18 @@ async fn main_agent_signer_admits_only_one_agent() { host.decode_sidebar(vec![event]).await.unwrap()["main-agent"], payload ); + let none = json!({"version": 1, "agent": null}); + let event = host + .sign_sidebar("main-agent".into(), none.clone(), 101) + .await + .unwrap(); + host.admit_sidebar(event.clone()).await.unwrap(); + assert_eq!( + host.decode_sidebar(vec![event]).await.unwrap()["main-agent"], + none + ); for invalid in [ json!({"version": 1}), - json!({"version": 1, "agent": null}), json!({"version": 2, "agent": "ab".repeat(32)}), json!({"version": 1, "agent": "AB".repeat(32)}), json!({"version": 1, "agent": "ab".repeat(31)}), diff --git a/src/bundled/agents/AgentsPage.test.tsx b/src/bundled/agents/AgentsPage.test.tsx index c530b1c9ce..bddf843292 100644 --- a/src/bundled/agents/AgentsPage.test.tsx +++ b/src/bundled/agents/AgentsPage.test.tsx @@ -3657,6 +3657,52 @@ it("offers Make my bestie from the card's actions button", async () => { expect(screen.getByRole("gridcell", { name: "Bea" })).toBeVisible(); }); +it("offers Make my bestie only for an agent on this device, and says why", async () => { + const user = userEvent.setup(); + const ada = agent("Ada", 1); + const sky: Agent = { ...agent("Sky", 2), place: "hosted" }; + const view = setup([ada, sky]); + const rows = [ada, sky].map((each) => ({ + pubkey: each.pubkey, + name: each.name, + managed: true, + })); + const choices = { + status: "ready", + complete: true, + pending: false, + identities: rows, + selectable: rows, + archives: { status: "unavailable", archived: [] }, + }; + const connection = { + status: "ready", + scope: "local-only-bestie", + session: { + agentChoices: { + snapshot: () => choices, + subscribe: () => () => {}, + retain: () => () => {}, + }, + }, + }; + const relay = { + snapshot: () => connection, + subscribe: () => () => {}, + } as unknown as RelayData; + view.rerender(); + const card = await screen.findByRole("gridcell", { name: /^Sky/ }); + await user.click( + within(card).getByRole("button", { name: "Actions for Sky" }), + ); + expect( + await screen.findByRole("menuitem", { name: "Make my bestie" }), + ).toHaveAttribute("aria-disabled", "true"); + expect( + screen.getByText("Only agents on this device can be your Bestie for now."), + ).toBeVisible(); +}); + it("keeps a failed Bestie replacement visible and retries the account save", async () => { const user = userEvent.setup(); const ada = agent("Ada", 1); diff --git a/src/bundled/agents/AgentsPage.tsx b/src/bundled/agents/AgentsPage.tsx index 3ca0db630a..104f3f7605 100644 --- a/src/bundled/agents/AgentsPage.tsx +++ b/src/bundled/agents/AgentsPage.tsx @@ -61,7 +61,7 @@ import { type SharedDefinition, } from "../../features/agents/agent-share"; import { useDefaultAgent } from "../../features/solo/default-agent"; -import { useBestieChoice } from "./BestieChoice"; +import { BESTIE_LOCAL_ONLY, useBestieChoice } from "./BestieChoice"; import type { RelayData } from "../../features/relay/service"; import type { Navigation } from "../../features/navigation/controller"; import { targetKey } from "../../features/navigation/targets"; @@ -91,6 +91,7 @@ import { Input } from "../../shared/design-system/ui/Input"; import { ContextMenuRoot, ContextMenuTrigger, + MenuNote, MenuSeparator, MenuPopup, MenuItem, @@ -993,9 +994,14 @@ function InventoryCard({ <> {bestie && !bestie.selected && ( <> - + {/* A remote agent isn't set up to be Bestie yet. */} + Make my bestie + {agent.place !== "here" && {BESTIE_LOCAL_ONLY}} )} diff --git a/src/bundled/agents/BestieChoice.tsx b/src/bundled/agents/BestieChoice.tsx index 0ccbe85c6f..00079b53d5 100644 --- a/src/bundled/agents/BestieChoice.tsx +++ b/src/bundled/agents/BestieChoice.tsx @@ -20,6 +20,11 @@ export function onBestieMade(listener: (pubkey: string) => void) { return () => void made.delete(listener); } +/** Why only some agents can be made Bestie: a remote one isn't set up for + * it yet. */ +export const BESTIE_LOCAL_ONLY = + "Only agents on this device can be your Bestie for now."; + /** Making an agent Bestie, wherever it is offered: `make` asks before * replacing an explicitly saved Bestie, in the dialog `view` renders. */ export function useBestieChoice(session: RelaySession, scope: string) { diff --git a/src/bundled/bestie/BestieTab.test.tsx b/src/bundled/bestie/BestieTab.test.tsx index 14359248a6..99a1b2648f 100644 --- a/src/bundled/bestie/BestieTab.test.tsx +++ b/src/bundled/bestie/BestieTab.test.tsx @@ -6,6 +6,7 @@ import { afterEach, expect, it, vi } from "vitest"; import type { Agent, AgentChange, Agents } from "../../features/agents/service"; import type { RelayData } from "../../features/relay/service"; import { writeView } from "../../shared/view-state"; +import { setDefaultAgent } from "../../features/solo/default-agent"; import { onBestieMade } from "../agents/BestieChoice"; import { bestieTab, formerBestieTab } from "./BestieTab"; import { validateObject } from "../../features/agents/attention"; @@ -34,13 +35,17 @@ const member = (name: string, digit: string): Agent => const ada = member("Ada", "a"); const bea = member("Bea", "b"); -/** Agents that save as a synced editor would: the agent's next config. */ -function host(types = ["test/echo"]) { +/** Agents that save as a synced editor would: the agent's next config. The + * agents on this device are Ada and Bea. */ +function host(types = ["test/echo"], here: Agent[] = [ada, bea]) { const save = vi.fn(async () => {}); + const snapshot = { agents: here, running: here }; const agents = { save, types: () => types.map((key) => ({ key, adopt: () => ({}) })), - } as unknown as Pick; + snapshot: () => snapshot, + subscribe: () => () => {}, + } as unknown as Pick; return { agents, save }; } @@ -61,8 +66,9 @@ function community(saved: string | undefined, agents = [ada, bea]) { }; const mainAgent = { read: vi.fn(async () => record), - write: vi.fn(async (agent: string) => { - record = agent; + // None reads back as no record, as the account's reader does. + write: vi.fn(async (agent: string | null) => { + record = agent ?? undefined; }), }; const connection = { @@ -143,6 +149,22 @@ it("shows on the saved Bestie only, and moves when Change picks another", async stop?.(); }); +it("offers only agents on this device in Change, and says why", async () => { + const user = userEvent.setup(); + const cara = { ...member("Cara", "c"), place: "hosted" } as Agent; + const { relay } = community(ada.pubkey, [ada, bea, cara]); + const Tab = bestieTab(relay, host([], [ada, bea, cara]).agents).component; + await act(async () => { + render(); + }); + await user.click(screen.getByRole("button", { name: "Change" })); + expect(await screen.findByRole("menuitem", { name: "Bea" })).toBeVisible(); + expect(screen.queryByRole("menuitem", { name: "Cara" })).toBeNull(); + expect( + screen.getByText("Only agents on this device can be your Bestie for now."), + ).toBeVisible(); +}); + it("keeps Change unavailable while there is no other agent to choose", async () => { const { relay } = community(ada.pubkey, [ada]); const Tab = bestieTab(relay, host().agents).component; @@ -150,6 +172,31 @@ it("keeps Change unavailable while there is no other agent to choose", async () render(); }); expect(screen.getByRole("button", { name: "Change" })).toBeDisabled(); + // The menu's note can't be opened, so the row says why instead. + expect( + screen.getByText( + "It's pinned at the top of Buzz, with your DM as its home. Only agents on this device can be your Bestie for now.", + ), + ).toBeVisible(); +}); + +it("offers no way back to a former Bestie that isn't on this device, and says why", async () => { + const { relay } = community(bea.pubkey); + const remote = { + ...ada, + place: "hosted", + config: { instructions: withFeature("Be brief.", memory, true) }, + } as Agent; + const Tab = formerBestieTab(relay, host([], [bea, remote]).agents).component; + await act(async () => { + render(); + }); + expect( + screen.getByRole("button", { name: "Make my Bestie again" }), + ).toBeDisabled(); + expect( + screen.getByText("Only agents on this device can be your Bestie for now."), + ).toBeVisible(); }); it("tells the host only when the Bestie or its community changes", async () => { @@ -448,6 +495,84 @@ it("shows a Former Bestie tab while a former Bestie has a setting on, with a way stop?.(); }); +it("removes the Bestie after asking, and shows the agent as a Former Bestie", async () => { + const user = userEvent.setup(); + const { relay, mainAgent } = community(ada.pubkey); + const agents = host().agents; + const tab = bestieTab(relay, agents); + const former = formerBestieTab(relay, agents); + const remembering = { + ...ada, + config: { instructions: withFeature("Be brief.", memory, true) }, + } as Agent; + const stop = tab.subscribe?.(() => {}); + await vi.waitFor(() => expect(tab.when?.(ada)).toBe(true)); + expect(former.when?.(remembering)).toBe(false); + const Tab = tab.component; + render(); + // A failed save says so and keeps the Bestie. + mainAgent.write.mockRejectedValueOnce(new Error("offline")); + await user.click(screen.getByRole("button", { name: "Remove" })); + let ask = await screen.findByRole("alertdialog", { + name: "Remove Ada as your Bestie?", + }); + expect(ask).toHaveTextContent("Ada stays in Agents"); + expect(ask).toHaveTextContent("Its check-ins stop."); + expect(ask).toHaveTextContent("memory rules and private channels stay"); + await user.click( + within(ask).getByRole("button", { name: "Remove as Bestie" }), + ); + expect(await within(ask).findByRole("alert")).toHaveTextContent( + "Removing your bestie couldn't be saved to your account. Try again.", + ); + expect(tab.when?.(ada)).toBe(true); + await user.click(within(ask).getByRole("button", { name: "Cancel" })); + expect(screen.queryByRole("alertdialog")).toBeNull(); + + await user.click(screen.getByRole("button", { name: "Remove" })); + ask = await screen.findByRole("alertdialog", { + name: "Remove Ada as your Bestie?", + }); + await user.click( + within(ask).getByRole("button", { name: "Remove as Bestie" }), + ); + expect(mainAgent.write).toHaveBeenLastCalledWith(null); + await vi.waitFor(() => expect(tab.when?.(ada)).toBe(false)); + expect(screen.queryByRole("alertdialog")).toBeNull(); + // With no Bestie known, the agent with memory on is a Former Bestie. + expect(former.when?.(remembering)).toBe(true); + expect(former.when?.(bea)).toBe(false); + stop?.(); +}); + +it("shows no Former Bestie tab while the account's choice is unread or unreadable", async () => { + const { relay, mainAgent } = community(undefined); + mainAgent.read.mockRejectedValueOnce(new Error("offline")); + const former = formerBestieTab(relay, host().agents); + const remembering = { + ...ada, + config: { instructions: withFeature("Be brief.", memory, true) }, + } as Agent; + const stop = former.subscribe?.(() => {}); + await act(async () => {}); + expect(former.when?.(remembering)).toBe(false); + stop?.(); +}); + +it("offers no Remove without an account to save none to", async () => { + const { relay } = community(undefined); + // A device-only session would pick another agent at once. + delete (relay.snapshot() as { session: { mainAgent?: unknown } }).session + .mainAgent; + setDefaultAgent(SCOPE, ada.pubkey); + const Tab = bestieTab(relay, host().agents).component; + await act(async () => { + render(); + }); + expect(screen.getByRole("button", { name: "Change" })).toBeVisible(); + expect(screen.queryByRole("button", { name: "Remove" })).toBeNull(); +}); + it("lets a former Bestie's leftover check-in be turned off", async () => { const user = userEvent.setup(); const { relay } = community(bea.pubkey); diff --git a/src/bundled/bestie/BestieTab.tsx b/src/bundled/bestie/BestieTab.tsx index 72ee085939..c97adc14d4 100644 --- a/src/bundled/bestie/BestieTab.tsx +++ b/src/bundled/bestie/BestieTab.tsx @@ -1,15 +1,22 @@ -import { useState } from "react"; -import type { Agent, AgentAddedTab } from "../../features/agents/service"; +import { useRef, useState, useSyncExternalStore } from "react"; +import type { + Agent, + AgentAddedTab, + Agents, +} from "../../features/agents/service"; import { useRelayConnection } from "../../features/relay/react"; import type { RelayData } from "../../features/relay/service"; import type { RelaySession } from "../../features/relay/session"; +import { clearDefaultAgent } from "../../features/solo/default-agent"; import { BestieIcon } from "../../shared/design-system/icons"; +import { AlertDialog } from "../../shared/design-system/ui/AlertDialog"; import { Button } from "../../shared/design-system/ui/Button"; import { InlineHeader } from "../../shared/design-system/ui/Header"; import { MenuGroup, MenuGroupLabel, MenuItem, + MenuNote, MenuPopup, MenuRoot, MenuTrigger, @@ -17,7 +24,11 @@ import { import { PreferenceRow } from "../../shared/design-system/ui/PreferenceRow"; import { SettingsGroup } from "../../shared/design-system/ui/SettingsGroup"; import { SwitchPreferenceRow } from "../../shared/design-system/ui/SwitchPreferenceRow"; -import { bestieName, useBestieChoice } from "../agents/BestieChoice"; +import { + BESTIE_LOCAL_ONLY, + bestieName, + useBestieChoice, +} from "../agents/BestieChoice"; import { FEATURES, type Feature, @@ -36,11 +47,11 @@ import { } from "./save"; import { inCommunity, savedBestie } from "./saved"; +/** The agents a tab saves to, and reads to know which are on this device. */ +type TabAgents = FeatureAgents & Pick; + /** The Bestie tab, on the agent saved as Bestie in its community. */ -export function bestieTab( - relay: RelayData, - agents: FeatureAgents, -): AgentAddedTab { +export function bestieTab(relay: RelayData, agents: TabAgents): AgentAddedTab { const bestie = savedBestie(relay); return { id: "bestie", @@ -57,10 +68,11 @@ export function bestieTab( /** The Former Bestie tab, on an agent that was Bestie and still has * something on. It goes once the agent has nothing left on, and shows only - * against a Bestie saved to the account and read, as check-ins stop. */ + * against the account's choice once read, another Bestie or none, as + * check-ins stop. */ export function formerBestieTab( relay: RelayData, - agents: FeatureAgents, + agents: TabAgents, ): AgentAddedTab { const bestie = savedBestie(relay); return { @@ -70,7 +82,7 @@ export function formerBestieTab( const current = bestie.snapshot(); const snapshot = relay.snapshot(); return ( - !!current && + (!!current || bestie.known()) && !!snapshot.session.mainAgent && agent.pubkey !== current && inCommunity(agent, snapshot) && @@ -98,7 +110,7 @@ function BestieTab({ former = false, }: { relay: RelayData; - agents: FeatureAgents; + agents: TabAgents; agent: Agent; former?: boolean; }) { @@ -106,8 +118,17 @@ function BestieTab({ const bestie = useBestieChoice(session, scope ?? ""); // One save at a time: each switch saves from the instructions it shows. const [saving, setSaving] = useState(false); + const [removing, setRemoving] = useState(false); + const { agents: saved } = useSyncExternalStore( + agents.subscribe, + agents.snapshot, + ); + // Only an agent on this device can be Bestie for now: a remote one isn't + // set up for it yet. + const here = (pubkey: string) => + saved.some((each) => each.pubkey === pubkey && each.place === "here"); const others = bestie.choices.selectable.filter( - (choice) => choice.pubkey !== agent.pubkey, + (choice) => choice.pubkey !== agent.pubkey && here(choice.pubkey), ); const name = agent.name || "This agent"; const switches = (features: readonly Feature[]) => @@ -132,10 +153,12 @@ function BestieTab({ } title={`${name} is no longer your Bestie. These are still on.`} + subtitle={here(agent.pubkey) ? undefined : BESTIE_LOCAL_ONLY} trailing={ + )} + /> + )} @@ -189,10 +235,68 @@ function BestieTab({ {...(former && { former: name })} /> {bestie.view} + {removing && ( + clearDefaultAgent(session, scope ?? "")} + onClose={() => setRemoving(false)} + /> + )} ); } +/** Asks before the agent stops being Bestie, then saves no Bestie. The tab + * goes with it; the agent then shows Former Bestie while a setting is on. */ +function RemoveBestie({ + name, + remove, + onClose, +}: { + name: string; + remove(): Promise; + onClose(): void; +}) { + const [pending, setPending] = useState(false); + const [error, setError] = useState(""); + const keep = useRef(null); + return ( + {error}

})} + actions={ + <> + + + + } + /> + ); +} + /** A feature's switch. Off also turns off the features that require this * one. On a former Bestie, check-ins can only be turned off. A section edited * by hand where it starts or ends shows that instead, with Restore when what diff --git a/src/bundled/bestie/README.md b/src/bundled/bestie/README.md index aca9cb2a47..0c5bec67ec 100644 --- a/src/bundled/bestie/README.md +++ b/src/bundled/bestie/README.md @@ -14,7 +14,12 @@ channel and sends a first message on your behalf so Bestie introduces itself. The agent you made your Bestie (with **Make my bestie** in Agents) gets a **Bestie** tab on its agent page while it runs on this device. **Change** picks -another agent and asks before replacing your Bestie. +another agent and asks before replacing your Bestie. Only agents on this device +can be your Bestie for now, in Change, in **Make my bestie** and in **Make my +Bestie again**: a remote one isn't set up for it yet. **Remove as Bestie** asks, +then saves no Bestie to your account: the agent stays, its check-ins stop, and +its memory section and private channels stay, which its Former Bestie tab can +turn off. **Remembers you** adds memory rules to the agent's instructions, in a section marked `` … ``, and @@ -51,12 +56,15 @@ check-in is a feature in `features.ts`, so another kind is one more entry. **Changing your Bestie.** Only your Bestie checks in: whenever the saved Bestie is known, on this device or synced from another, any other agent here has its -check-ins removed. Its memory section and channels stay. An agent this device -makes Bestie starts with Remembers you on; a choice synced from elsewhere -changes nothing, so memory turned off stays off. A former Bestie keeps a -**Former Bestie** tab while it has a Bestie section, or, if this device saw it -as Bestie, while it is in one of your private channels. The tab has the same -switches, **Remove from all** for its channels, and **Make my Bestie again**. +check-ins removed. Once your account is read to have no Bestie, saved as none +or never saved, every agent here has them removed; while it is being read, or +can't be, nothing is. Its memory section and channels stay. An agent this +device makes Bestie starts with Remembers you on; a choice synced from +elsewhere changes nothing, so memory turned off stays off. A former Bestie +keeps a **Former Bestie** tab while it has a Bestie section, or, if this device +saw it as Bestie, while it is in one of your private channels. The tab has the +same switches, **Remove from all** for its channels, and **Make my Bestie +again**. --- diff --git a/src/bundled/bestie/former.test.ts b/src/bundled/bestie/former.test.ts index d74d07d74a..3c67c13643 100644 --- a/src/bundled/bestie/former.test.ts +++ b/src/bundled/bestie/former.test.ts @@ -1,8 +1,10 @@ // @vitest-environment jsdom import { afterEach, expect, it, vi } from "vitest"; -import type { Agent, Agents } from "../../features/agents/service"; +import type { Agent, AgentChange, Agents } from "../../features/agents/service"; import type { ChannelSummary } from "../../features/relay/contracts"; import type { RelayData, RelaySnapshot } from "../../features/relay/service"; +import type { RelaySession } from "../../features/relay/session"; +import { chooseDefaultAgent } from "../../features/solo/default-agent"; import { checkinHourly, memory, withFeature } from "./features"; import { formerWithSettings, @@ -39,17 +41,19 @@ const agent = (digit: string, instructions = "", attention = {}): Agent => const ada = agent("a", checkingIn, TIMER); const bea = agent("b"); -/** A community whose account read of the saved Bestie waits until `settle`. */ +/** A community whose account read of the saved Bestie waits until `settle`, + * or `fail`. */ function community(channels: readonly ChannelSummary[] = [], account = true) { let settle: (pubkey: string | undefined) => void = () => {}; - const read = new Promise((resolve) => { + let fail: (error: Error) => void = () => {}; + const read = new Promise((resolve, reject) => { settle = resolve; + fail = reject; }); + const write = vi.fn(async (_agent: string | null) => {}); const session = { viewer: OWNER, - ...(account && { - mainAgent: { read: () => read, write: vi.fn(async () => {}) }, - }), + ...(account && { mainAgent: { read: () => read, write } }), channels: { list: () => ({ channels }), subscribeList: () => () => {}, @@ -63,12 +67,18 @@ function community(channels: readonly ChannelSummary[] = [], account = true) { } as unknown as RelayData; return { relay, + write, snapshot: snapshot as unknown as RelaySnapshot, settle: async (pubkey: string | undefined) => { settle(pubkey); await read; await Promise.resolve(); }, + fail: async () => { + fail(new Error("offline")); + await read.catch(() => {}); + await Promise.resolve(); + }, }; } @@ -144,12 +154,83 @@ it("stops a former Bestie's check-ins once the Bestie is known, and leaves its m stop(); }); +it("stops every agent's check-ins once the account is read to have no Bestie", async () => { + const none = community(); + const cara = agent("c", checkingIn, TIMER); + const { agents, save } = host([ada, bea, cara]); + const stop = stopFormerCheckIns(none.relay, agents); + // Not while it is read. + await Promise.resolve(); + expect(save).not.toHaveBeenCalled(); + await none.settle(undefined); + await vi.waitFor(() => expect(save).toHaveBeenCalledTimes(4)); + expect(new Set(save.mock.calls.map(([key]) => key))).toEqual( + new Set([ada.key, cara.key]), + ); + // Memory stays. + expect(save).toHaveBeenCalledWith(cara.key, { + config: { instructions: withFeature("Be brief.\n", memory, true) }, + }); + stop(); +}); + +it("keeps the check-ins of an agent made Bestie while the account is read, or while none is stopped", async () => { + const none = community(); + const cara = agent("c", checkingIn, TIMER); + const dara = agent("d"); + const { agents, save } = host([ada, bea, cara, dara]); + const session = none.relay.snapshot().session as unknown as RelaySession; + const stop = stopFormerCheckIns(none.relay, agents); + // Chosen before the read settles: the read's none isn't known. + void none.settle(undefined); + await chooseDefaultAgent(session, SCOPE, cara.pubkey); + await vi.waitFor(() => expect(save).toHaveBeenCalledTimes(2)); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(new Set(save.mock.calls.map(([key]) => key))).toEqual( + new Set([ada.key]), + ); + + // Chosen as Dara's check-in is about to be stopped. + save.mockClear(); + void agents.save(dara.key, { + config: { instructions: checkingIn }, + attention: TIMER, + } as unknown as AgentChange); + await chooseDefaultAgent(session, SCOPE, dara.pubkey); + await new Promise((resolve) => setTimeout(resolve, 0)); + // Only Cara, now former, is stopped; Dara keeps its check-in. + expect(save.mock.calls.map(([key]) => key)).toEqual([ + dara.key, + cara.key, + cara.key, + ]); + + // Made Bestie again as Cara's check-in is about to be stopped, then not: + // Cara is stopped once the choice is known. + save.mockClear(); + void agents.save(cara.key, { + config: { instructions: withFeature("Be brief.\n", checkinHourly, true) }, + attention: TIMER, + } as unknown as AgentChange); + none.write.mockRejectedValueOnce(new Error("offline")); + await expect( + chooseDefaultAgent(session, SCOPE, cara.pubkey), + ).rejects.toThrow(); + await vi.waitFor(() => expect(save).toHaveBeenCalledTimes(3)); + expect(save.mock.calls.map(([key]) => key)).toEqual([ + cara.key, + cara.key, + cara.key, + ]); + stop(); +}); + it("does nothing while the saved Bestie is unknown, or to the Bestie itself", async () => { const unknown = community(); const first = host([ada]); const stopUnknown = stopFormerCheckIns(unknown.relay, first.agents); - // An account with no saved Bestie, or a read that failed, isn't "none". - await unknown.settle(undefined); + // A read that failed isn't "none". + await unknown.fail(); await new Promise((resolve) => setTimeout(resolve, 0)); expect(first.save).not.toHaveBeenCalled(); stopUnknown(); diff --git a/src/bundled/bestie/former.ts b/src/bundled/bestie/former.ts index 50237cf9dc..f768cac476 100644 --- a/src/bundled/bestie/former.ts +++ b/src/bundled/bestie/former.ts @@ -92,11 +92,12 @@ export function subscribePrivateMembers( /** Stops the check-ins of each agent here that isn't the community's Bestie, * whenever the saved Bestie or the agents change, wherever the change was - * made. It acts only on a Bestie saved to the account and read: while the - * choice is loading or unknown, or kept only on this device, where an - * automatic pick looks the same, nobody counts as former. An agent also in - * another community, where it may be Bestie, loses only its timer here, as - * its instructions are shared. Memory and channels stay as they are. */ + * made. It acts only on the account's choice once known: a Bestie, or none, + * when every agent here is stopped. While the choice is loading, unreadable + * or being saved, or kept only on this device, where an automatic pick looks + * the same, nobody counts as former. An agent also in another community, + * where it may be Bestie, loses only its timer here, as its instructions are + * shared. Memory and channels stay as they are. */ export function stopFormerCheckIns(relay: RelayData, agents: ReconcileAgents) { const bestie = savedBestie(relay); const checkIns = FEATURES.filter(({ timer }) => timer); @@ -112,9 +113,12 @@ export function stopFormerCheckIns(relay: RelayData, agents: ReconcileAgents) { session = relaySnapshot.session; tried.clear(); } - const current = session.mainAgent ? bestie.snapshot() : undefined; - if (!current) return; - if (relaySnapshot.scope) rememberBestie(relaySnapshot.scope, current); + if (!session.mainAgent) return; + // Not while the account is read, can't be, or a choice here is saved. + if (!bestie.known()) return; + const current = bestie.snapshot(); + if (current && relaySnapshot.scope) + rememberBestie(relaySnapshot.scope, current); const { agents: here, running = [] } = agents.snapshot(); for (const agent of here) { const on = checkIns.filter((feature) => featurePresent(agent, feature)); @@ -140,11 +144,17 @@ export function stopFormerCheckIns(relay: RelayData, agents: ReconcileAgents) { .then(async () => { for (const feature of on) { // Each save starts from the agent as the last one left it, and - // stops if it was made Bestie again meanwhile. + // stops if the saved Bestie changed or stopped being known + // meanwhile, as when it is made Bestie again. const fresh = agents .snapshot() .agents.find((each) => each.key === agent.key); - if (!fresh || bestie.snapshot() === agent.pubkey) return; + if (!fresh) return; + if (!bestie.known() || bestie.snapshot() !== current) { + // Tried again once the saved Bestie is known. + tried.delete(agent.key); + return; + } await saveFeature(agents, fresh, feature, false, { timerOnly }); } }) diff --git a/src/bundled/bestie/saved.ts b/src/bundled/bestie/saved.ts index 6c4249447b..87f84ab102 100644 --- a/src/bundled/bestie/saved.ts +++ b/src/bundled/bestie/saved.ts @@ -4,7 +4,8 @@ import { relayPartition } from "../../features/relay/partition"; import type { RelayData, RelaySnapshot } from "../../features/relay/service"; import { savedAgentStore } from "../../features/solo/default-agent"; -/** The saved Bestie in the connected community, across reconnections. */ +/** The saved Bestie in the connected community, across reconnections. + * `known`: the account's choice was read, so without a Bestie it saved none. */ export function savedBestie(relay: RelayData) { let current: | Readonly<{ @@ -24,15 +25,19 @@ export function savedBestie(relay: RelayData) { }; return current.store; }; + const state = (store: ReturnType) => + `${store.snapshot()} ${store.known()}`; return { snapshot: () => store().snapshot(), + known: () => store().known(), subscribe(listener: () => void) { // The saved choice's sources also change for unrelated view state, so - // only a different Bestie, or community, is passed on. + // only a different Bestie, or community, or its becoming known, is + // passed on. let following = store(); - let last = following.snapshot(); + let last = state(following); const changed = () => { - const next = following.snapshot(); + const next = state(following); if (next === last) return; last = next; listener(); @@ -42,7 +47,7 @@ export function savedBestie(relay: RelayData) { if (store() === following) return; stop(); following = store(); - last = following.snapshot(); + last = state(following); stop = following.subscribe(changed); listener(); }); diff --git a/src/features/relay/native-sidebar.test.ts b/src/features/relay/native-sidebar.test.ts index cb44084c3a..807580963c 100644 --- a/src/features/relay/native-sidebar.test.ts +++ b/src/features/relay/native-sidebar.test.ts @@ -186,6 +186,15 @@ it("reads a cleared main agent from development builds as none, and replaces it" expect(await transport.readMainAgent?.(signal)).toBe("b".repeat(64)); }); +it("saves none as the account's main agent, and reads it back as none", async () => { + const transport = await connectNativeTransport(community); + await transport.writeMainAgent?.("a".repeat(64), signal); + await transport.writeMainAgent?.(null, signal); + expect(await transport.readMainAgent?.(signal)).toBeUndefined(); + await transport.writeMainAgent?.("b".repeat(64), signal); + expect(await transport.readMainAgent?.(signal)).toBe("b".repeat(64)); +}); + it("prepends a drag-created section through native encrypted readback", async () => { const transport = await connectNativeTransport(community); const existing = "11111111-1111-1111-1111-111111111111"; diff --git a/src/features/relay/native-sidebar.ts b/src/features/relay/native-sidebar.ts index 9d07550de2..d1e99bfdd1 100644 --- a/src/features/relay/native-sidebar.ts +++ b/src/features/relay/native-sidebar.ts @@ -182,8 +182,10 @@ export function nativeSidebar(transport: ReadTransport) { const current = await head("main-agent", signal); return current.createdAt ? mainAgent(current.payload) : undefined; }, - async writeMainAgent(agent: string, signal: AbortSignal) { - if (!agentKey.test(agent)) throw new Error("Invalid main agent intent"); + /** Saves the main agent the viewer chose, or none for null. */ + async writeMainAgent(agent: string | null, signal: AbortSignal) { + if (agent !== null && !agentKey.test(agent)) + throw new Error("Invalid main agent intent"); await mutate( "main-agent", signal, diff --git a/src/features/relay/session-main-agent.test.ts b/src/features/relay/session-main-agent.test.ts new file mode 100644 index 0000000000..51b461af69 --- /dev/null +++ b/src/features/relay/session-main-agent.test.ts @@ -0,0 +1,50 @@ +import { afterEach, expect, it, vi } from "vitest"; +import type { RelayEvent } from "./events"; +import { createRelaySession } from "./session"; +import { keypair, signed } from "./testing"; + +const viewer = keypair(), + relay = keypair(); +const owners: ReturnType[] = []; +afterEach(() => { + for (const owner of owners.splice(0)) owner.dispose(); +}); + +it("saves the main agent, or none, one change at a time", async () => { + const order: (string | null)[] = []; + let release!: () => void; + const held = new Promise((resolve) => { + release = resolve; + }); + const writeMainAgent = vi.fn(async (agent: string | null) => { + if (agent !== null) await held; + order.push(agent); + }); + const owner = createRelaySession({ + viewer: viewer.pubkey, + relayAuthor: relay.pubkey, + media: () => undefined, + query: async () => [] as RelayEvent[], + writer: { + kinds: [30078], + sign: async (event: Parameters[1]) => + signed(viewer, event), + publish: async () => undefined, + }, + readMainAgent: async () => undefined, + writeMainAgent, + }); + owners.push(owner); + const account = owner.session.mainAgent; + if (!account) throw new Error("No main agent record"); + const choose = account.write("a".repeat(64)); + const clear = account.write(null); + release(); + await Promise.all([choose, clear]); + // None, saved after the choice, is the one left. + expect(order).toEqual(["a".repeat(64), null]); + expect(writeMainAgent).toHaveBeenLastCalledWith( + null, + expect.any(AbortSignal), + ); +}); diff --git a/src/features/relay/session.ts b/src/features/relay/session.ts index 1d2fbaa55f..0b67f5e186 100644 --- a/src/features/relay/session.ts +++ b/src/features/relay/session.ts @@ -1452,7 +1452,8 @@ export function createRelaySession( return read && write ? Object.freeze({ read: () => read(bounded(10_000)), - write: (agent: string) => queue(() => write(agent, bounded(20_000))), + write: (agent: string | null) => + queue(() => write(agent, bounded(20_000))), }) : undefined; })(); diff --git a/src/features/relay/transport.ts b/src/features/relay/transport.ts index 3f4b48d30c..ae0f982fbb 100644 --- a/src/features/relay/transport.ts +++ b/src/features/relay/transport.ts @@ -164,7 +164,8 @@ export interface ReadTransport { /** Host-only private account record of the main agent the viewer chose: * undefined when never saved. */ readMainAgent?(signal: AbortSignal): Promise; - writeMainAgent?(agent: string, signal: AbortSignal): Promise; + /** Null saves none. */ + writeMainAgent?(agent: string | null, signal: AbortSignal): Promise; readonly profiling?: RelayProfiler; /** Verified incoming traffic. The session owns this subscription and fences late delivery. */ subscribe?(callbacks: LiveCallbacks): LiveSubscription; diff --git a/src/features/solo/default-agent.test.tsx b/src/features/solo/default-agent.test.tsx index 104c00a4aa..3c22eb2f87 100644 --- a/src/features/solo/default-agent.test.tsx +++ b/src/features/solo/default-agent.test.tsx @@ -8,8 +8,10 @@ import type { RelaySession } from "../relay/session"; import { readView } from "../../shared/view-state"; import { chooseDefaultAgent, + clearDefaultAgent, forgetDefaultAgent, restoreDefaultAgent, + savedAgentStore, setDefaultAgent, useDefaultAgent, } from "./default-agent"; @@ -389,8 +391,8 @@ function accountSession( read = () => resolve(record); }); }), - write: vi.fn(async (agent: string) => { - saved = agent; + write: vi.fn(async (agent: string | null) => { + saved = agent ?? undefined; }), saved: () => saved, resolve: () => read(), @@ -651,3 +653,129 @@ it("still picks a main agent when the account can't be read", async () => { await act(async () => {}); expect(screen.getByRole("status").textContent).toBe("First"); }); + +it("clears the main agent on this device and saves none to the account", async () => { + const { account, session } = accountSession("b".repeat(64)); + const store = savedAgentStore(session, "clear"); + const stop = store.subscribe(() => {}); + // Not known while the account is read. + expect(store.known()).toBe(false); + await act(async () => account.resolve()); + expect(store.snapshot()).toBe("b".repeat(64)); + expect(store.known()).toBe(true); + expect(readView("clear", "solo:default-agent", null)).toBe("b".repeat(64)); + await clearDefaultAgent(session, "clear"); + expect(account.write).toHaveBeenLastCalledWith(null); + expect(account.saved()).toBeUndefined(); + expect(store.snapshot()).toBeUndefined(); + expect(store.known()).toBe(true); + expect(readView("clear", "solo:default-agent", null)).toBeNull(); + stop(); +}); + +it("knows the account saved none only once its read succeeded", async () => { + const failing = accountSession(undefined); + failing.account.read.mockRejectedValueOnce(new Error("offline")); + const unread = savedAgentStore(failing.session, "unknown"); + const stop = unread.subscribe(() => {}); + await act(async () => {}); + expect(unread.snapshot()).toBeUndefined(); + expect(unread.known()).toBe(false); + stop(); + + const none = accountSession(undefined); + const read = savedAgentStore(none.session, "none"); + const stopNone = read.subscribe(() => {}); + await act(async () => none.account.resolve()); + expect(read.known()).toBe(true); + stopNone(); +}); + +it("brings this device's choice back when none can't be saved to the account", async () => { + const { account, session } = accountSession("b".repeat(64)); + setDefaultAgent("unsaved-clear", "b".repeat(64)); + account.write.mockRejectedValueOnce(new Error("offline")); + await expect(clearDefaultAgent(session, "unsaved-clear")).rejects.toThrow( + "Removing your bestie couldn't be saved to your account. Try again.", + ); + expect(readView("unsaved-clear", "solo:default-agent", null)).toBe( + "b".repeat(64), + ); + expect(account.saved()).toBe("b".repeat(64)); + expect(savedAgentStore(session, "unsaved-clear").known()).toBe(false); +}); + +it("doesn't know the account saved none while a save is pending or after one failed during its read", async () => { + const { account, session } = accountSession("b".repeat(64)); + const store = savedAgentStore(session, "pending-clear"); + const stop = store.subscribe(() => {}); + account.write.mockRejectedValueOnce(new Error("offline")); + const clearing = expect( + clearDefaultAgent(session, "pending-clear"), + ).rejects.toThrow(); + await act(async () => account.resolve()); + await clearing; + // The read named B, but a removal was asked for meanwhile, so neither is + // known. + expect(store.snapshot()).toBeUndefined(); + expect(store.known()).toBe(false); + + let save!: () => void; + account.write.mockImplementationOnce( + () => new Promise((resolve) => (save = resolve)), + ); + const retry = clearDefaultAgent(session, "pending-clear"); + expect(store.known()).toBe(false); + await act(async () => { + save(); + await retry; + }); + expect(store.known()).toBe(true); + stop(); +}); + +it("brings the removed Bestie back over an automatic pick, but not over a newer choice", async () => { + const { account, session } = accountSession("b".repeat(64)); + setDefaultAgent("auto-clear", "b".repeat(64)); + let fail!: () => void; + account.write.mockImplementationOnce( + () => new Promise((_, reject) => (fail = () => reject(new Error()))), + ); + const clearing = clearDefaultAgent(session, "auto-clear"); + // A composer picks a fallback while the save is pending. + setDefaultAgent("auto-clear", "a".repeat(64)); + fail(); + await expect(clearing).rejects.toThrow(); + expect(readView("auto-clear", "solo:default-agent", null)).toBe( + "b".repeat(64), + ); + + account.write.mockImplementationOnce( + () => new Promise((_, reject) => (fail = () => reject(new Error()))), + ); + const again = clearDefaultAgent(session, "auto-clear"); + const choosing = chooseDefaultAgent(session, "auto-clear", "a".repeat(64)); + fail(); + await expect(again).rejects.toThrow(); + await choosing; + expect(readView("auto-clear", "solo:default-agent", null)).toBe( + "a".repeat(64), + ); +}); + +it("a choice in another community doesn't keep a failed removal from coming back", async () => { + const here = accountSession("b".repeat(64)); + const there = accountSession("a".repeat(64)); + setDefaultAgent("turn-here", "b".repeat(64)); + let fail!: () => void; + here.account.write.mockImplementationOnce( + () => new Promise((_, reject) => (fail = () => reject(new Error()))), + ); + const clearing = clearDefaultAgent(here.session, "turn-here"); + await chooseDefaultAgent(there.session, "turn-there", "a".repeat(64)); + fail(); + await expect(clearing).rejects.toThrow(); + expect(readView("turn-here", "solo:default-agent", null)).toBe( + "b".repeat(64), + ); +}); diff --git a/src/features/solo/default-agent.ts b/src/features/solo/default-agent.ts index 1b5d350a46..93dcf065df 100644 --- a/src/features/solo/default-agent.ts +++ b/src/features/solo/default-agent.ts @@ -19,11 +19,31 @@ const adopted = new WeakMap>(); // The account's saved choice once read: undefined when never saved or // unreadable. const saved = new WeakMap(); +// Accounts whose saved choice is known: read before any choice here, or +// confirmed by a save here, and not while a save is pending. +const known = new WeakSet(); +const saving = new WeakMap(); const listeners = new Set<() => void>(); function remember(account: Account, agent: string | undefined) { saved.set(account, agent); for (const listener of listeners) listener(); } +/** Saves `agent`, or none, to the account; its choice is known once saved. */ +async function save(account: Account, agent: string | null) { + saving.set(account, (saving.get(account) ?? 0) + 1); + for (const listener of listeners) listener(); + try { + await account.write(agent); + // Known with its value, before it stops counting as pending. + saved.set(account, agent ?? undefined); + known.add(account); + } finally { + saving.set(account, (saving.get(account) ?? 1) - 1); + } +} +// Explicit choices on this device by scope, so a failed one gives way to a +// newer one but not to an automatic pick. +const turns = new Map(); function subscribeSaved(listener: () => void) { listeners.add(listener); return () => void listeners.delete(listener); @@ -64,13 +84,16 @@ export async function chooseDefaultAgent( ) { const account = session.mainAgent; if (account) explicit.add(account); + turns.set(scope, (turns.get(scope) ?? 0) + 1); // A choice this device can't cache still applies here, so it is still saved // to the account. setDefaultAgent(scope, pubkey); if (!account) return; try { - await account.write(pubkey); + await save(account, pubkey); } catch { + // What was known before it is again. + remember(account, saved.get(account)); throw new Error( "Your bestie couldn't be saved to your account. Try again.", ); @@ -78,6 +101,31 @@ export async function chooseDefaultAgent( remember(account, pubkey); } +/** The viewer's choice of no main agent: cleared here, then saved to their + * account as none. If the account's save fails, this device's choice comes + * back, unless another was made meanwhile. This device may still pick one + * automatically, as without a choice. */ +export async function clearDefaultAgent(session: RelaySession, scope: string) { + const account = session.mainAgent; + if (account) explicit.add(account); + const turn = (turns.get(scope) ?? 0) + 1; + turns.set(scope, turn); + const previous = chosenAgent(scope); + cache(scope, null); + if (!account) return; + try { + await save(account, null); + } catch { + // Over an automatic pick made meanwhile, not a newer choice. + if (previous && turn === turns.get(scope)) setDefaultAgent(scope, previous); + remember(account, saved.get(account)); + throw new Error( + "Removing your bestie couldn't be saved to your account. Try again.", + ); + } + remember(account, undefined); +} + /** A deleted agent stops being the main agent everywhere it was chosen; the * next available one takes over and is saved on this device. One left on the * account is stood in for like any deleted choice, until the next choice. */ @@ -97,6 +145,8 @@ function adopt(account: Account, scope: string) { (agent) => { const adopts = !explicit.has(account); if (adopts && agent !== undefined) cache(scope, agent); + // A choice made meanwhile is known only once it is saved. + if (adopts) known.add(account); remember(account, adopts ? agent : saved.get(account)); }, () => { @@ -139,6 +189,9 @@ export function savedAgentStore(session: RelaySession, scope: string) { const account = session.mainAgent; return Object.freeze({ snapshot: () => (account ? saved.get(account) : chosenAgent(scope)), + /** Whether the account's choice is known, so an undefined snapshot means + * none was saved rather than not read yet, or unreadable. */ + known: () => !!account && known.has(account) && !saving.get(account), subscribe(listener: () => void) { if (account) void adopt(account, scope); // The device's choice is browser storage; without a window there is none diff --git a/tests/fixtures/bestie-tab.tsx b/tests/fixtures/bestie-tab.tsx index c552f66e51..3d7c20fcf9 100644 --- a/tests/fixtures/bestie-tab.tsx +++ b/tests/fixtures/bestie-tab.tsx @@ -80,9 +80,18 @@ const relay = { snapshot: () => connection, subscribe: () => () => {}, } as unknown as RelayData; +const here = { + agents: [ + { pubkey: ADA, place: "here" }, + { pubkey: BEA, place: "here" }, + ], +}; const agents = { save: async () => {}, types: () => [{ key: "claude-code", adopt: () => ({}) }], + // Both are on this device, so either can be made Bestie. + snapshot: () => here, + subscribe: () => () => {}, }; const agent = { key: ADA,