mirror of
https://github.com/block/buzz.git
synced 2026-11-11 20:01:09 +00:00
fix(desktop): allow a team to have zero agents so it can be deleted
Deleting an agent team was impossible without a workaround. Three guards
formed a cycle:
- `delete_team` refuses while agents still reference the team ("Delete or
reconfigure them first")
- `validate_persona_deletion` refuses to delete a team's agent ("Delete the
team to remove all team agents together")
- and the team editor's submit button required at least one selected member,
so the roster could not be emptied either
Delete-team said "remove the agents first", delete-agent said "delete the
team first", and emptying the roster — the one sequence that breaks the
cycle — was blocked by the UI. The only escape was to reassign a member to
another team to drain the original.
The backend already supported zero-member teams: `update_team` has no
minimum-member check, `apply_team_membership_delta` detaches the removed
members' instances (clearing the `team_id` that `delete_team` keys on), and
`persona_ids: []` is representable in `teams.json`. Only the frontend
enforced the minimum, so this drops that one condition — extracted as
`canSubmitTeamDialog` so the contract is unit-testable.
Removing the minimum makes an empty team reachable, which would have
silently exposed Deploy/Duplicate/Share on a team with nothing to deploy.
Sharing one would mint a snapshot that Buzz's own importer rejects
("Team snapshot must have at least one member" — snapshot import keeps its
minimum, correctly). Those three actions are now gated on
`resolution.isUsable`, which was already false for an empty roster, and the
team card explains the state. Edit and Delete stay enabled: emptying a team
and then deleting it is the intended way out.
Tests: an e2e walk of the full user sequence (edit → deselect every member →
save → delete), which fails on the disabled Save button without this change;
a Rust test pinning that clearing `team_id` really drops the delete guard;
and unit coverage for the submit gate and the emptied-team resolution.
Signed-off-by: Fizz <550f2cb2562fc05f87c4839dc3c9b1bfca4d68db4183f8b54d3d477f6836a91d@buzz.block.builderlab.xyz>
Co-authored-by: Storme Briscoe <storme@squareup.com>
Signed-off-by: Storme Briscoe <storme@squareup.com>
This commit is contained in:
@@ -263,6 +263,37 @@ fn agents_referencing_team_empty_when_no_matches() {
|
||||
assert!(agents_referencing_team(&agents, &t).is_empty());
|
||||
}
|
||||
|
||||
/// The escape hatch from the team/agent delete deadlock, asserted end to end at
|
||||
/// this layer: `delete_team_with_cascade` refuses while any agent references the
|
||||
/// team, and `validate_persona_deletion` refuses to delete a team's agent — so
|
||||
/// the only non-circular sequence is "empty the team's roster, then delete the
|
||||
/// team". Emptying the roster clears `team_id` on the members' instances (see
|
||||
/// `apply_team_membership_delta` in `commands::teams`), and once it is cleared
|
||||
/// this guard must report zero referencing agents. If it still matched, the team
|
||||
/// would stay permanently undeletable and the deadlock would be back.
|
||||
#[test]
|
||||
fn detached_agents_no_longer_reference_the_team() {
|
||||
let t = team("json-team-3", "Emptied Team");
|
||||
|
||||
let mut bound = managed_agent("Bound Agent");
|
||||
bound.team_id = Some("json-team-3".to_string());
|
||||
assert_eq!(
|
||||
agents_referencing_team(std::slice::from_ref(&bound), &t),
|
||||
vec!["Bound Agent"],
|
||||
"a bound agent blocks team deletion"
|
||||
);
|
||||
|
||||
// What emptying the roster does: clear the binding, keep the agent.
|
||||
let detached = ManagedAgentRecord {
|
||||
team_id: None,
|
||||
..bound
|
||||
};
|
||||
assert!(
|
||||
agents_referencing_team(std::slice::from_ref(&detached), &t).is_empty(),
|
||||
"a detached agent must not block team deletion"
|
||||
);
|
||||
}
|
||||
|
||||
// Migration pins — exercise the real merge_teams wrapper (with production consts).
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -92,3 +92,20 @@ test("getUsableTeams keeps only fully-resolved teams with at least one persona",
|
||||
["team-ready"],
|
||||
);
|
||||
});
|
||||
|
||||
// An emptied team is now reachable through the editor (removing every member is
|
||||
// how you escape the team/agent delete deadlock). It must still be treated as
|
||||
// unusable: a zero-member snapshot fails the importer's own
|
||||
// "Team snapshot must have at least one member" check, and deploying it would
|
||||
// attach no agents at all.
|
||||
test("resolveTeamPersonas marks a deliberately emptied team complete but unusable", () => {
|
||||
const resolution = resolveTeamPersonas(createTeam("team-empty", []), [
|
||||
createPersona("persona-1", "Solo"),
|
||||
]);
|
||||
|
||||
assert.equal(resolution.hasMissingPersonas, false);
|
||||
assert.equal(resolution.isComplete, true);
|
||||
assert.equal(resolution.isUsable, false);
|
||||
assert.equal(resolution.missingPersonaCount, 0);
|
||||
assert.deepEqual(resolution.resolvedPersonaIds, []);
|
||||
});
|
||||
|
||||
@@ -31,7 +31,7 @@ export function TeamDeleteDialog({
|
||||
<AlertDialogTitle>Delete team?</AlertDialogTitle>
|
||||
<AlertDialogDescription>
|
||||
{team
|
||||
? `Delete "${team.name}". Already-deployed agents are not affected, but this team template will no longer be available.`
|
||||
? `Delete "${team.name}". This removes the team template only — deployed agents are left in place, but deletion is blocked while any of them still reference this team.`
|
||||
: "Delete this team."}
|
||||
</AlertDialogDescription>
|
||||
</AlertDialogHeader>
|
||||
|
||||
@@ -21,6 +21,7 @@ import { Textarea } from "@/shared/ui/textarea";
|
||||
import { personaCatalogCopy } from "./personaLibraryCopy";
|
||||
import { RemoveMembersConfirmDialog } from "./RemoveMembersConfirmDialog";
|
||||
import {
|
||||
canSubmitTeamDialog,
|
||||
copySelectedPersonaIds,
|
||||
countMissingPersonaIds,
|
||||
filterAvailablePersonaIds,
|
||||
@@ -325,11 +326,7 @@ export function TeamDialog({
|
||||
Cancel
|
||||
</Button>
|
||||
<Button
|
||||
disabled={
|
||||
name.trim().length === 0 ||
|
||||
selectedPersonaIds.length === 0 ||
|
||||
isPending
|
||||
}
|
||||
disabled={!canSubmitTeamDialog({ name, isPending })}
|
||||
onClick={() => void handleSubmit()}
|
||||
size="sm"
|
||||
type="button"
|
||||
|
||||
@@ -93,6 +93,15 @@ export function TeamsSection({
|
||||
const resolution = resolveTeamPersonas(team, personas);
|
||||
const missingPersonaCount = resolution.missingPersonaCount;
|
||||
const hasMissingPersonas = resolution.hasMissingPersonas;
|
||||
const isEmptyTeam = team.personaIds.length === 0;
|
||||
// Deploy/Duplicate/Share need a roster that fully resolves to real
|
||||
// agents. `isUsable` covers both failure modes: a member that is no
|
||||
// longer in My Agents, and a deliberately emptied team. Sharing an
|
||||
// empty team would mint a snapshot that its own importer rejects
|
||||
// ("Team snapshot must have at least one member"), so gate it here.
|
||||
// Edit and Delete stay enabled — emptying a team then deleting it is
|
||||
// the intended way out of the team/agent delete deadlock.
|
||||
const canUseRoster = resolution.isUsable;
|
||||
|
||||
return (
|
||||
<TeamIdentityCard
|
||||
@@ -112,7 +121,7 @@ export function TeamsSection({
|
||||
onCloseAutoFocus={(event) => event.preventDefault()}
|
||||
>
|
||||
<DropdownMenuItem
|
||||
disabled={isPending || hasMissingPersonas}
|
||||
disabled={isPending || !canUseRoster}
|
||||
onClick={() => onAddToChannel(team)}
|
||||
>
|
||||
<Rocket className="h-4 w-4" />
|
||||
@@ -127,14 +136,14 @@ export function TeamsSection({
|
||||
Edit
|
||||
</DropdownMenuItem>
|
||||
<DropdownMenuItem
|
||||
disabled={isPending || hasMissingPersonas}
|
||||
disabled={isPending || !canUseRoster}
|
||||
onClick={() => onDuplicate(team)}
|
||||
>
|
||||
<CopyPlus className="h-4 w-4" />
|
||||
Duplicate
|
||||
</DropdownMenuItem>
|
||||
<DropdownMenuItem
|
||||
disabled={isPending || hasMissingPersonas}
|
||||
disabled={isPending || !canUseRoster}
|
||||
onClick={() => onShare(team)}
|
||||
>
|
||||
<Share2 className="h-4 w-4" />
|
||||
@@ -171,6 +180,12 @@ export function TeamsSection({
|
||||
agents. Edit the team to fix it before deploying or sharing.
|
||||
</p>
|
||||
) : null}
|
||||
{isEmptyTeam ? (
|
||||
<p className="border-t border-border/60 bg-muted/40 px-3 py-2 text-xs text-muted-foreground">
|
||||
This team has no agents. Add one to deploy or share it, or
|
||||
delete the team.
|
||||
</p>
|
||||
) : null}
|
||||
</TeamIdentityCard>
|
||||
);
|
||||
})}
|
||||
|
||||
@@ -2,6 +2,7 @@ import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import {
|
||||
canSubmitTeamDialog,
|
||||
copySelectedPersonaIds,
|
||||
countMissingPersonaIds,
|
||||
filterAvailablePersonaIds,
|
||||
@@ -90,3 +91,22 @@ test("orderPersonasByInitiallySelected keeps initially selected personas at top"
|
||||
],
|
||||
);
|
||||
});
|
||||
|
||||
// ── canSubmitTeamDialog ───────────────────────────────────────────────────
|
||||
//
|
||||
// The regression these cover: the submit button used to also require
|
||||
// `selectedPersonaIds.length > 0`, which made "remove every member" unsavable
|
||||
// and left the team/agent delete deadlock with no way out.
|
||||
|
||||
test("canSubmitTeamDialog allows saving a team with an empty roster", () => {
|
||||
assert.equal(canSubmitTeamDialog({ name: "Hive", isPending: false }), true);
|
||||
});
|
||||
|
||||
test("canSubmitTeamDialog still requires a non-blank name", () => {
|
||||
assert.equal(canSubmitTeamDialog({ name: "", isPending: false }), false);
|
||||
assert.equal(canSubmitTeamDialog({ name: " ", isPending: false }), false);
|
||||
});
|
||||
|
||||
test("canSubmitTeamDialog blocks while a save is in flight", () => {
|
||||
assert.equal(canSubmitTeamDialog({ name: "Hive", isPending: true }), false);
|
||||
});
|
||||
|
||||
@@ -8,6 +8,31 @@ export function copySelectedPersonaIds(personaIds: string[]): string[] {
|
||||
return [...personaIds];
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the team dialog's submit button should be enabled.
|
||||
*
|
||||
* A team name is required; an **empty roster is deliberately allowed**. Emptying
|
||||
* a team is the only way out of the delete deadlock — `delete_team` refuses
|
||||
* while agents still reference the team, and agent deletion refuses because the
|
||||
* agent belongs to a team. "Remove every member, then delete the team" is the
|
||||
* escape hatch, so the editor must be able to save a zero-member team. The Rust
|
||||
* side already supports it (`update_team` has no minimum-member check and
|
||||
* `apply_team_membership_delta` detaches the removed members' instances); the
|
||||
* min-1 rule that remains applies to snapshot *import* only.
|
||||
*
|
||||
* Extracted as a pure function so the "empty roster is submittable" contract is
|
||||
* covered by a test rather than living in a JSX `disabled` expression.
|
||||
*/
|
||||
export function canSubmitTeamDialog({
|
||||
name,
|
||||
isPending,
|
||||
}: {
|
||||
name: string;
|
||||
isPending: boolean;
|
||||
}): boolean {
|
||||
return name.trim().length > 0 && !isPending;
|
||||
}
|
||||
|
||||
export function countMissingPersonaIds(
|
||||
personaIds: string[],
|
||||
personas: AgentPersona[],
|
||||
|
||||
@@ -2708,3 +2708,79 @@ test("duplicate instances move from the agents gallery into the agent profile",
|
||||
page.getByTestId(`user-profile-agent-delete-${additionalPubkey}`),
|
||||
).toHaveCount(0);
|
||||
});
|
||||
|
||||
// Regression for the team-deletion deadlock: `delete_team` refuses while agents
|
||||
// still reference the team, and deleting a team's agent refuses because it
|
||||
// belongs to a team. The only non-circular way out is to empty the roster and
|
||||
// then delete the team — which the editor used to forbid, because the submit
|
||||
// button also required at least one selected member. This walks the full user
|
||||
// sequence: edit → deselect every member → save → delete.
|
||||
test("a team can be emptied and then deleted", async ({ page }) => {
|
||||
await installMockBridge(page, {
|
||||
personas: [
|
||||
{
|
||||
id: "custom:deadlock-a",
|
||||
displayName: "Deadlock A",
|
||||
systemPrompt: "First member of the team under test.",
|
||||
},
|
||||
{
|
||||
id: "custom:deadlock-b",
|
||||
displayName: "Deadlock B",
|
||||
systemPrompt: "Second member of the team under test.",
|
||||
},
|
||||
],
|
||||
teams: [
|
||||
{
|
||||
id: "team-deadlock",
|
||||
name: "Deadlock Team",
|
||||
personaIds: ["custom:deadlock-a", "custom:deadlock-b"],
|
||||
},
|
||||
],
|
||||
});
|
||||
await gotoApp(page);
|
||||
await page.getByTestId("open-agents-view").click();
|
||||
|
||||
const teamCard = page.getByTestId("team-card-team-deadlock");
|
||||
await expect(teamCard).toBeVisible();
|
||||
|
||||
// Empty the roster via the edit dialog.
|
||||
await page.getByLabel("Deadlock Team team actions").click();
|
||||
await page.getByRole("menuitem", { name: "Edit" }).click();
|
||||
|
||||
const roster = page.getByRole("listbox", { name: "Agents" });
|
||||
await roster.getByRole("option", { name: /Deadlock A/ }).click();
|
||||
await roster.getByRole("option", { name: /Deadlock B/ }).click();
|
||||
|
||||
// The regression: with no members selected, save must still be enabled.
|
||||
const save = page.getByRole("button", { name: "Save changes" });
|
||||
await expect(save).toBeEnabled();
|
||||
await save.click();
|
||||
|
||||
// Removing members prompts about the underlying agents; keep them.
|
||||
await page.getByRole("button", { name: "Keep agents" }).click();
|
||||
|
||||
// The team survives with an empty roster and says so.
|
||||
await expect(teamCard).toContainText("This team has no agents");
|
||||
const teams = await invokeTauri<Array<{ id: string; persona_ids: string[] }>>(
|
||||
page,
|
||||
"list_teams",
|
||||
);
|
||||
expect(
|
||||
teams.find((team) => team.id === "team-deadlock")?.persona_ids,
|
||||
).toEqual([]);
|
||||
|
||||
// With nothing referencing it, the team is now deletable — the deadlock is gone.
|
||||
await page.getByLabel("Deadlock Team team actions").click();
|
||||
await page.getByRole("menuitem", { name: "Delete" }).click();
|
||||
await page
|
||||
.getByRole("button", { name: "Delete", exact: true })
|
||||
.last()
|
||||
.click();
|
||||
|
||||
await expect(teamCard).toHaveCount(0);
|
||||
const remaining = await invokeTauri<Array<{ id: string }>>(
|
||||
page,
|
||||
"list_teams",
|
||||
);
|
||||
expect(remaining.some((team) => team.id === "team-deadlock")).toBe(false);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user