test(desktop): repair the share tests and simplify the comments

The change to `TeamsSection.tsx` made 3 tests fail in
`desktop/tests/e2e/team-snapshot.spec.ts`. CI found the failures. This
commit corrects them.

The 3 tests share a team. Before, they used the default mock team
"Engineering", which has no members (`e2eBridge.ts`). The earlier gate
used `hasMissingPersonas`, which is false for a team that has no
members, and thus the Share item was enabled. The new gate uses
`isUsable`, which is correctly false. The Share item is now disabled,
and each test stopped at a disabled menu item.

The new gate is correct. The `build_team_export_snapshot` function makes
one member for each entry in `team.persona_ids`. A team that has no
members thus gives a snapshot that has no members, and
`validate_team_snapshot` refuses it. The mock replaces the encode
command, and therefore the tests did not show this. The 3 tests now seed
their own team, and that team has one member.

This commit also writes all the comments of this change again in
Simplified Technical English (ASD-STE100): short sentences, active
voice, no idioms. The text of the delete dialog is simpler too.

No production behavior changes in this commit.

Co-authored-by: Storme Briscoe <storme@squareup.com>
Signed-off-by: Storme Briscoe <storme@squareup.com>
This commit is contained in:
Fizz
2026-08-24 14:23:23 -06:00
parent 281d730af4
commit 8ef3d2d88f
8 changed files with 92 additions and 57 deletions

View File

@@ -263,14 +263,18 @@ 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.
/// A detached agent must not prevent the deletion of a team.
///
/// You cannot delete a team and its agents in one step. The
/// `delete_team_with_cascade` function stops if an agent points to the team.
/// The `validate_persona_deletion` function stops if the agent is in a team.
/// Thus only one sequence is possible: first remove all the members from the
/// team, then delete the team.
///
/// When you remove a member, the `apply_team_membership_delta` function in
/// `commands::teams` clears the `team_id` field of that member. This test makes
/// sure that the guard then finds no agents. If the guard still found the
/// agent, you could not delete the team.
#[test]
fn detached_agents_no_longer_reference_the_team() {
let t = team("json-team-3", "Emptied Team");

View File

@@ -93,11 +93,11 @@ test("getUsableTeams keeps only fully-resolved teams with at least one persona",
);
});
// 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.
// An emptied team is now possible in the editor. You remove all the members to
// make it possible to delete the team. But the team must still be unusable. A
// snapshot of it has no members, and the import of that snapshot fails with the
// message "Team snapshot must have at least one member". A deployment of it
// would also add no agents.
test("resolveTeamPersonas marks a deliberately emptied team complete but unusable", () => {
const resolution = resolveTeamPersonas(createTeam("team-empty", []), [
createPersona("persona-1", "Solo"),

View File

@@ -31,7 +31,7 @@ export function TeamDeleteDialog({
<AlertDialogTitle>Delete team?</AlertDialogTitle>
<AlertDialogDescription>
{team
? `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 "${team.name}". This deletes the team template only. Deployed agents stay in place. You cannot delete the team while an agent is still a member of it.`
: "Delete this team."}
</AlertDialogDescription>
</AlertDialogHeader>

View File

@@ -94,13 +94,18 @@ export function TeamsSection({
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.
// The Deploy, Duplicate and Share items need a roster that has
// members, and each member must be a known agent. The `isUsable`
// flag shows both conditions. It is false if a member is not in My
// Agents. It is also false if the team has no members.
//
// You must not share a team that has no members. The snapshot then
// has no members, and the import of that snapshot fails with the
// message "Team snapshot must have at least one member".
//
// The Edit and Delete items stay enabled. You use these two items
// to delete a team: first remove all the members, then delete the
// team.
const canUseRoster = resolution.isUsable;
return (

View File

@@ -94,9 +94,9 @@ 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.
// These tests show the correction of a defect. Before, the submit button also
// needed `selectedPersonaIds.length > 0`. Thus you could not save a team after
// you removed all the members, and you could not delete the team.
test("canSubmitTeamDialog allows saving a team with an empty roster", () => {
assert.equal(canSubmitTeamDialog({ name: "Hive", isPending: false }), true);

View File

@@ -9,19 +9,23 @@ export function copySelectedPersonaIds(personaIds: string[]): string[] {
}
/**
* Whether the team dialog's submit button should be enabled.
* Tells you if the submit button in the team dialog is 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.
* The team must have a name. The team does not need a member. An empty roster
* is correct.
*
* 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.
* An empty roster is necessary. It is the only way to delete a team. The
* `delete_team` command stops if an agent points to the team. The agent delete
* command stops if the agent is in a team. Thus you must first remove all the
* members from the team. Then you can delete the team.
*
* The Rust code permits a team that has no members. The `update_team` command
* does not count the members. The `apply_team_membership_delta` function clears
* the `team_id` field of each member that you remove. Only the import of a
* snapshot needs one member or more.
*
* This is a separate function because a test can then read the rule. Before,
* the rule was in a JSX `disabled` expression, where a test cannot read it.
*/
export function canSubmitTeamDialog({
name,

View File

@@ -2709,12 +2709,12 @@ test("duplicate instances move from the agents gallery into the agent profile",
).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.
// This test shows the correction of a defect. You could not delete a team. The
// `delete_team` command stops if an agent points to the team. The agent delete
// command stops if the agent is in a team. Thus you must first remove all the
// members from the team, then delete the team. But the editor did not permit
// this, because the submit button needed one member or more. This test does the
// full sequence: edit the team, remove each member, save, then delete.
test("a team can be emptied and then deleted", async ({ page }) => {
await installMockBridge(page, {
personas: [
@@ -2751,15 +2751,16 @@ test("a team can be emptied and then deleted", async ({ page }) => {
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.
// This is the corrected behavior. No member is selected, and the save
// button must 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.
// The app asks what to do with the agents of the removed members. Keep them.
await page.getByRole("button", { name: "Keep agents" }).click();
// The team survives with an empty roster and says so.
// The team is still present, it has no members, and the card shows this.
await expect(teamCard).toContainText("This team has no agents");
const teams = await invokeTauri<Array<{ id: string; persona_ids: string[] }>>(
page,
@@ -2769,7 +2770,7 @@ test("a team can be emptied and then deleted", async ({ page }) => {
teams.find((team) => team.id === "team-deadlock")?.persona_ids,
).toEqual([]);
// With nothing referencing it, the team is now deletable — the deadlock is gone.
// No agent points to the team now. Thus you can delete the team.
await page.getByLabel("Deadlock Team team actions").click();
await page.getByRole("menuitem", { name: "Delete" }).click();
await page

View File

@@ -49,6 +49,22 @@ const ANALYST_PERSONA_ID = "test-analyst";
const ANALYST_PUBKEY =
"953d3363262e86b770419834c53d2446409db6d918a57f8f339d495d54ab001f";
// The share tests use this team. The team has one member.
//
// You cannot share a team that has no members. A team snapshot must have one
// member or more, and the Share item is disabled for a team that has no
// members. The default mock team "Engineering" has no members. Therefore each
// share test seeds this team, and the team holds the seeded Analyst persona.
// The name must be different from "Engineering". If the names are the same, a
// locator finds two teams and the test stops.
const SHARE_TEAM_NAME = "Delivery Crew";
const SHARE_TEAM_SEED = {
id: "team-share-001",
name: SHARE_TEAM_NAME,
description: "Team for the share tests",
personaIds: [ANALYST_PERSONA_ID],
};
// ── (a) Confirm-fail + retry ────────────────────────────────────────────────
test("team_snapshot_import_confirm_fail_renders_error_and_retry_succeeds", async ({
@@ -251,16 +267,17 @@ test("team sharing uses the people picker and gates memory before sending", asyn
displayName: "Charlie",
},
],
teams: [SHARE_TEAM_SEED],
uploadDescriptors: [MOCK_UPLOAD_DESCRIPTOR],
});
await gotoAgentsPage(page);
await page.getByLabel("Engineering team actions").click();
await page.getByLabel(`${SHARE_TEAM_NAME} team actions`).click();
await page.getByRole("menuitem", { name: "Share" }).click();
const shareDialog = page.getByTestId("team-share-dialog");
await expect(shareDialog).toBeVisible();
await expect(
shareDialog.getByRole("heading", { name: "Share Engineering" }),
shareDialog.getByRole("heading", { name: `Share ${SHARE_TEAM_NAME}` }),
).toBeVisible();
const search = shareDialog.getByTestId("team-share-recipient-search");
@@ -289,9 +306,11 @@ test("team sharing uses the people picker and gates memory before sending", asyn
expect(encodeLevelsBeforeConfirmation).toEqual([]);
await memoryConfirmation.getByTestId("team-share-memory-confirm").click();
await expect(page.getByText("Sent a copy of Engineering")).toBeVisible({
timeout: 8_000,
});
await expect(page.getByText(`Sent a copy of ${SHARE_TEAM_NAME}`)).toBeVisible(
{
timeout: 8_000,
},
);
const log = await readCommandLog(page);
expect(
@@ -307,7 +326,7 @@ test("team sharing uses the people picker and gates memory before sending", asyn
);
expect(sendEntry).toBeTruthy();
const sendPayload = sendEntry?.payload as { content?: string } | undefined;
expect(sendPayload?.content).toContain("[Engineering](");
expect(sendPayload?.content).toContain(`[${SHARE_TEAM_NAME}](`);
expect(sendPayload?.content).not.toContain("![image](");
});
@@ -332,11 +351,12 @@ test("team share level carries memories onto the link path too", async ({
},
],
agentMemory: createMockAgentMemoryListing(),
teams: [SHARE_TEAM_SEED],
uploadDescriptors: [MOCK_UPLOAD_DESCRIPTOR],
});
await gotoAgentsPage(page);
await page.getByLabel("Engineering team actions").click();
await page.getByLabel(`${SHARE_TEAM_NAME} team actions`).click();
await page.getByRole("menuitem", { name: "Share" }).click();
const shareDialog = page.getByTestId("team-share-dialog");
await expect(shareDialog).toBeVisible();
@@ -397,12 +417,13 @@ test("team sharing keeps link copy and export in the shared surface", async ({
personaId: ANALYST_PERSONA_ID,
},
],
teams: [SHARE_TEAM_SEED],
uploadDescriptors: [MOCK_UPLOAD_DESCRIPTOR],
uploadDelayMs: 800,
});
await gotoAgentsPage(page);
await page.getByLabel("Engineering team actions").click();
await page.getByLabel(`${SHARE_TEAM_NAME} team actions`).click();
const menu = page.getByRole("menu");
await expect(
menu.getByRole("menuitem", { name: "Export snapshot" }),
@@ -508,24 +529,24 @@ test("team sharing keeps link copy and export in the shared surface", async ({
const composerTeamCard = page.getByTestId("composer-team-snapshot-card");
await expect(composerTeamCard).toBeVisible();
await expect(composerTeamCard).toContainText("Engineering");
await expect(composerTeamCard).toContainText(SHARE_TEAM_NAME);
await expect(composerTeamCard.locator("img")).toHaveCount(0);
await page.getByTestId("send-message").click();
const sentTeamCard = page.getByTestId("agent-snapshot-card").last();
await expect(sentTeamCard).toBeVisible();
await expect(sentTeamCard).toContainText("Engineering");
await expect(sentTeamCard).toContainText(SHARE_TEAM_NAME);
await expect(sentTeamCard).toContainText("Add team");
await expect(sentTeamCard.locator("img")).toHaveCount(0);
await page.getByTestId("open-agents-view").click();
await page.getByLabel("Engineering team actions").click();
await page.getByLabel(`${SHARE_TEAM_NAME} team actions`).click();
await page.getByRole("menuitem", { name: "Share" }).click();
await page.getByTestId("team-share-export").click();
const exportDialog = page.getByTestId("team-snapshot-export-dialog");
await expect(exportDialog).toBeVisible();
await expect(
exportDialog.getByRole("heading", { name: "Export Engineering" }),
exportDialog.getByRole("heading", { name: `Export ${SHARE_TEAM_NAME}` }),
).toBeVisible();
await expect(
exportDialog.getByTestId("team-snapshot-memory-trigger"),