Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 168 additions & 0 deletions desktop/src/features/agents/lib/managedAgentControlActions.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import test from "node:test";
import {
startManagedAgentWithRules,
respawnManagedAgentWithRules,
deleteManagedAgentsForPersonaWithRules,
} from "./managedAgentControlActions.ts";

function agent(overrides = {}) {
Expand Down Expand Up @@ -166,3 +167,170 @@ test("test_respawn_onStopped_fires_before_start_resolves", async () => {
"onStopped must fire after stop resolves and before start is called",
);
});

// --- deleteManagedAgentsForPersonaWithRules ---
//
// Contract under test: the persona cascade must abort on the first cancelled
// or failed instance delete, so `deletePersona` is never reached with a
// half-torn persona. Mirrors deleteProfileManagedAgentsForPersona.

function personaAgent(pubkeyChar, overrides = {}) {
return agent({
pubkey: pubkeyChar.repeat(64),
personaId: "persona-1",
...overrides,
});
}

test("persona cascade deletes every instance backed by that persona", async () => {
const deleted = [];
const result = await deleteManagedAgentsForPersonaWithRules({
persona: { id: "persona-1" },
managedAgents: [personaAgent("a"), personaAgent("b")],
channels: [],
relayAgents: [],
deleteManagedAgent: async ({ pubkey }) => {
deleted.push(pubkey);
},
});

assert.deepEqual(result, { deletedCount: 2 });
assert.deepEqual(deleted, ["a".repeat(64), "b".repeat(64)]);
});

test("persona cascade ignores instances backed by other personas", async () => {
const deleted = [];
await deleteManagedAgentsForPersonaWithRules({
persona: { id: "persona-1" },
managedAgents: [
personaAgent("a"),
personaAgent("c", { personaId: "persona-2" }),
],
channels: [],
relayAgents: [],
deleteManagedAgent: async ({ pubkey }) => {
deleted.push(pubkey);
},
});

assert.deepEqual(deleted, ["a".repeat(64)]);
});

test("persona cascade deletes a duplicated instance only once", async () => {
const deleted = [];
await deleteManagedAgentsForPersonaWithRules({
persona: { id: "persona-1" },
managedAgents: [personaAgent("a"), personaAgent("A"), personaAgent("a")],
channels: [],
relayAgents: [],
deleteManagedAgent: async ({ pubkey }) => {
deleted.push(pubkey);
},
});

assert.equal(deleted.length, 1, "same pubkey must not be deleted twice");
});

test("persona cascade stops at a declined orphan confirm", async () => {
const originalWindow = globalThis.window;
globalThis.window = { confirm: () => false };
try {
const deleted = [];
const result = await deleteManagedAgentsForPersonaWithRules({
persona: { id: "persona-1" },
// First instance is provider-deployed and in no channel, so the
// orphan-warning confirm is what decides the cascade.
managedAgents: [
personaAgent("a", {
backend: { type: "provider" },
backendAgentId: "remote-1",
}),
personaAgent("b"),
],
channels: [],
relayAgents: [],
deleteManagedAgent: async ({ pubkey }) => {
deleted.push(pubkey);
},
});

assert.equal(result.cancelled, true, "declining must report cancelled");
assert.equal(
result.deletedCount,
0,
"nothing was deleted before the abort",
);
assert.deepEqual(
deleted,
[],
"no instance may be deleted once the confirm is declined",
);
} finally {
globalThis.window = originalWindow;
}
});

test("a declined confirm does not undo instances already deleted", async () => {
// The aborting instance is deliberately NOT first: a local instance deletes
// with no prompt, then the provider instance's confirm is declined. The
// earlier delete is permanent, so the cascade must report it rather than let
// it vanish silently behind a cancelled persona delete.
const originalWindow = globalThis.window;
globalThis.window = { confirm: () => false };
try {
const deleted = [];
const result = await deleteManagedAgentsForPersonaWithRules({
persona: { id: "persona-1" },
managedAgents: [
personaAgent("a"),
personaAgent("b", {
backend: { type: "provider" },
backendAgentId: "remote-1",
}),
personaAgent("c"),
],
channels: [],
relayAgents: [],
deleteManagedAgent: async ({ pubkey }) => {
deleted.push(pubkey);
},
});

assert.equal(result.cancelled, true);
assert.deepEqual(
deleted,
["a".repeat(64)],
"the local instance ahead of the declined one is already gone",
);
assert.equal(
result.deletedCount,
1,
"deletedCount must surface the partial teardown so the caller can report it",
);
} finally {
globalThis.window = originalWindow;
}
});

test("persona cascade stops at the first failed instance delete", async () => {
const deleted = [];
await assert.rejects(
deleteManagedAgentsForPersonaWithRules({
persona: { id: "persona-1" },
managedAgents: [personaAgent("a"), personaAgent("b")],
channels: [],
relayAgents: [],
deleteManagedAgent: async ({ pubkey }) => {
if (pubkey === "a".repeat(64)) throw new Error("backend refused");
deleted.push(pubkey);
},
}),
/backend refused/,
);

assert.deepEqual(
deleted,
[],
"instances after the failure must be left untouched",
);
});
57 changes: 57 additions & 0 deletions desktop/src/features/agents/lib/managedAgentControlActions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { sendChannelMessage } from "@/shared/api/tauri";
import type {
AgentPersona,
Channel,
ManagedAgent,
PresenceLookup,
Expand Down Expand Up @@ -141,6 +142,62 @@ export async function stopManagedAgentWithRules({
return {};
}

/**
* Delete every managed-agent instance backed by `persona`, applying the same
* per-agent rules as {@link deleteManagedAgentWithRules} — including the
* orphan-warning confirm for provider deployments.
*
* Callers must run this to completion *before* deleting the persona itself:
* `delete_persona` refuses to cascade over a provider-deployed instance, so a
* persona whose instances are still deployed cannot be deleted at all.
*
* Contract, shared with `deleteProfileManagedAgentsForPersona` in
* `features/profile/ui/UserProfilePanelDeletion.ts`: stop the cascade at the
* first cancelled or failed instance delete, so the persona is not deleted on
* top of a partial teardown.
*
* Aborting stops *further* deletes — it cannot undo the ones that already ran,
* and instance deletes are not reversible. Instances are visited in
* `managedAgents` order and only provider-deployed ones prompt, so a local
* instance ahead of a declined provider instance is already gone by the time
* the user cancels. `deletedCount` reports how many were destroyed precisely so
* the caller can tell the user about that partial state instead of silently
* keeping the persona.
*
* The profile variant additionally removes each agent from its channels between
* deletes, which is why the two loops are kept separate rather than
* parameterized — keep both in step.
*/
export async function deleteManagedAgentsForPersonaWithRules({
persona,
managedAgents,
...context
}: {
persona: Pick<AgentPersona, "id">;
managedAgents: readonly ManagedAgent[];
deleteManagedAgent: DeleteManagedAgent;
} & ManagedAgentActionContext): Promise<
ManagedAgentActionResult & { deletedCount: number }
> {
// Dedup by pubkey so a list carrying the same instance twice cannot delete it
// twice — mirrors the profile variant's Map for the same reason.
const agentsByPubkey = new Map<string, ManagedAgent>();
for (const agent of managedAgents) {
if (agent.personaId === persona.id) {
agentsByPubkey.set(normalizePubkey(agent.pubkey), agent);
}
}

let deletedCount = 0;
for (const agent of agentsByPubkey.values()) {
const result = await deleteManagedAgentWithRules({ agent, ...context });
if (result.cancelled) return { ...result, deletedCount };
deletedCount += 1;
}

return { deletedCount };
}

export async function deleteManagedAgentWithRules({
agent,
channels,
Expand Down
13 changes: 12 additions & 1 deletion desktop/src/features/agents/ui/AgentsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,19 @@ export function AgentsView() {
).length
}
onConfirm={(persona) => {
void personas.handleDelete(persona);
void personas.handleDelete(
persona,
agents.handleDeleteInstancesForPersona,
);
}}
providerInstanceCount={
(agents.managedAgents ?? []).filter(
(a) =>
a.personaId === personas.personaToDelete?.id &&
a.backend.type === "provider" &&
a.backendAgentId,
).length
}
onOpenChange={(open) => {
if (!open) {
personas.setPersonaToDelete(null);
Expand Down
41 changes: 41 additions & 0 deletions desktop/src/features/agents/ui/PersonaDeleteDialog.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,44 @@ test("no instances → no archival claim (nothing is archived)", () => {
test("null persona keeps the generic fallback", () => {
assert.equal(personaDeleteDescription(null, 2), "Delete this agent.");
});

// Provider-hosted instances are NOT torn down by this cascade — delete_managed_agent
// never contacts the provider, and force_remote_delete only bypasses the backend's
// orphan guard. The dialog must say the deployment survives, because the failure it
// prevents is a user believing their billed infrastructure was removed when it wasn't.

test("provider-hosted instances are disclosed as surviving (singular)", () => {
const copy = personaDeleteDescription(persona, 2, 1);
assert.match(copy, /1 of them is hosted by a provider/);
assert.match(copy, /not torn down/);
assert.match(copy, /until you remove it at the provider/);
});

test("provider-hosted instances are disclosed as surviving (plural)", () => {
const copy = personaDeleteDescription(persona, 3, 2);
assert.match(copy, /2 of them are hosted by a provider/);
assert.match(copy, /not torn down/);
});

test("never claims the provider deployment is removed", () => {
// Regression guard: an earlier draft promised "will also be removed from that
// provider", which contradicts the orphan-warning confirm shown moments later
// in the same flow and would hide ongoing provider spend.
for (const count of [1, 2, 5]) {
const copy = personaDeleteDescription(persona, count + 1, count);
assert.doesNotMatch(copy, /removed from that provider/);
assert.doesNotMatch(copy, /will also be removed/);
}
});

test("no provider instances → no provider claim", () => {
const copy = personaDeleteDescription(persona, 2, 0);
assert.doesNotMatch(copy, /provider/i);
});

test("provider disclosure defaults off for existing callers", () => {
assert.equal(
personaDeleteDescription(persona, 2),
personaDeleteDescription(persona, 2, 0),
);
});
27 changes: 25 additions & 2 deletions desktop/src/features/agents/ui/PersonaDeleteDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ type PersonaDeleteDialogProps = {
persona: AgentPersona | null;
/** Number of managed-agent instances backed by this persona. Omit or pass 0 to suppress the instance-count sentence. */
instanceCount?: number;
/** How many of those instances are provider-hosted. Omit or pass 0 to suppress the provider-removal sentence. */
providerInstanceCount?: number;
onConfirm: (persona: AgentPersona) => void;
onOpenChange: (open: boolean) => void;
};
Expand All @@ -26,10 +28,19 @@ type PersonaDeleteDialogProps = {
* cascade-deleted, each one's identity is also archived on the relay
* (NIP-IA), and that durable side effect must be disclosed before the
* destructive confirm — matching the direct agent-delete dialog.
*
* Provider-hosted instances are disclosed separately because the cascade does
* NOT tear them down. `delete_managed_agent` stops the local process, drops the
* record and key, and tombstones/archives the identity — it never contacts the
* provider, and `force_remote_delete` only bypasses the backend's orphan guard.
* Provider teardown is a future `undeploy` operation (see the note in
* `managed_agents/runtime.rs`), so the remote deployment keeps running and
* keeps costing money. Saying otherwise would be worse than saying nothing.
*/
export function personaDeleteDescription(
persona: AgentPersona | null,
instanceCount: number,
providerInstanceCount = 0,
): string {
if (!persona) {
return "Delete this agent.";
Expand All @@ -41,13 +52,21 @@ export function personaDeleteDescription(
instanceCount === 1
? "Also deletes 1 agent instance and archives its identity on the relay, so it no longer appears in member lists or mention suggestions."
: `Also deletes ${instanceCount} agent instances and archives their identities on the relay, so they no longer appear in member lists or mention suggestions.`;
return `Delete ${persona.displayName}. ${cascade}`;
if (providerInstanceCount === 0) {
return `Delete ${persona.displayName}. ${cascade}`;
}
const provider =
providerInstanceCount === 1
? "1 of them is hosted by a provider: the remote deployment is not torn down and keeps running until you remove it at the provider."
: `${providerInstanceCount} of them are hosted by a provider: those remote deployments are not torn down and keep running until you remove them at the provider.`;
return `Delete ${persona.displayName}. ${cascade} ${provider}`;
}

export function PersonaDeleteDialog({
open,
persona,
instanceCount = 0,
providerInstanceCount = 0,
onConfirm,
onOpenChange,
}: PersonaDeleteDialogProps) {
Expand All @@ -57,7 +76,11 @@ export function PersonaDeleteDialog({
<AlertDialogHeader>
<AlertDialogTitle>Delete agent?</AlertDialogTitle>
<AlertDialogDescription>
{personaDeleteDescription(persona, instanceCount)}
{personaDeleteDescription(
persona,
instanceCount,
providerInstanceCount,
)}
</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
Expand Down
Loading