diff --git a/desktop/src/apps/ProjectsApp/ProjectMembers.test.tsx b/desktop/src/apps/ProjectsApp/ProjectMembers.test.tsx index 84749962..d9ae4b58 100644 --- a/desktop/src/apps/ProjectsApp/ProjectMembers.test.tsx +++ b/desktop/src/apps/ProjectsApp/ProjectMembers.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, act } from "@testing-library/react"; +import { render, screen, act, fireEvent, within } from "@testing-library/react"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { ProjectMembers } from "./ProjectMembers"; import type { Project } from "@/lib/projects"; @@ -24,27 +24,22 @@ async function flush() { }); } -const project = { id: "prj-test", name: "taOS", slug: "taos" } as unknown as Project; +const baseProject = { id: "prj-test", name: "taOS", slug: "taos" } as unknown as Project; -// A consent-flow external agent: the member row keys on the canonical id, and -// the registry entry has an EMPTY handle (this is what the approve flow writes). -const externalMember = { - project_id: "prj-test", - member_id: "grok-taos-20260711-000736", - member_kind: "native", - role: "member", - is_lead: 0, - can_edit_canvas: 0, -}; +const agentA = { id: "agent-a", name: "Alpha", display_name: "Alpha", emoji: "🅰️" }; +const agentB = { id: "agent-b", name: "Beta", display_name: "Beta", emoji: "🅱️" }; -const registryEntry = { - canonical_id: "grok-taos-20260711-000736", - handle: "", - display_name: "grok-taOS", - framework: "grok", - origin: "external-selfjoin", - status: "active", -}; +function memberRow(memberId: string) { + return { + project_id: "prj-test", + member_id: memberId, + member_kind: "native", + role: "member", + is_lead: 0, + can_edit_canvas: 0, + can_read_canvas: 0, + }; +} describe("ProjectMembers external-agent categorisation", () => { beforeEach(() => { @@ -67,10 +62,150 @@ describe("ProjectMembers external-agent categorisation", () => { // The section heading only renders when at least one member is classified // external, so its presence is the regression guard: before the canonical-id // match this agent fell into the plain Members list. - expect(screen.getByText("External / Connected agents")).toBeInTheDocument(); - expect(screen.getByText("grok-taOS")).toBeInTheDocument(); + const externalSection = screen.getByText("External / Connected agents").closest("section")!; + expect(externalSection).toBeInTheDocument(); + expect(within(externalSection!).getByText("grok-taOS")).toBeInTheDocument(); // The framework badge resolves via the canonical-id keyed lookup, and "grok" // (not only "grok-build") maps to the friendly Grok label. - expect(screen.getByText("Grok")).toBeInTheDocument(); + expect(within(externalSection!).getByText("Grok")).toBeInTheDocument(); + }); +}); + +describe("ProjectMembers canvas capability checkboxes (slice 6)", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("renders both a read and an edit canvas checkbox for an agent member", async () => { + vi.stubGlobal( + "fetch", + mockFetch({ + "/api/projects/prj-test/members": { + ok: true, + body: { items: [memberRow("agent-a")] }, + }, + "/api/agents": { ok: true, body: [agentA] }, + "/api/agents/registry": { ok: true, body: [] }, + }), + ); + + render(); + await flush(); + + expect(screen.getByText("Can read canvas")).toBeInTheDocument(); + expect(screen.getByText("Can edit canvas")).toBeInTheDocument(); + expect( + screen.getByLabelText("Can read canvas for Alpha"), + ).toBeInTheDocument(); + expect( + screen.getByLabelText("Can edit canvas for Alpha"), + ).toBeInTheDocument(); + }); +}); + +describe("ProjectMembers exclusive Lead selector (D7)", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("reflects the current lead and promotes a new one exclusively via setLead", async () => { + const fetchMock = mockFetch({ + "/api/projects/prj-test/members": { + ok: true, + body: { items: [memberRow("agent-a"), memberRow("agent-b")] }, + }, + "/api/agents": { ok: true, body: [agentA, agentB] }, + "/api/agents/registry": { ok: true, body: [] }, + "/api/projects/prj-test/lead": { ok: true, body: { ok: true, lead_member_id: "agent-b" } }, + }); + vi.stubGlobal("fetch", fetchMock); + + const project = { + ...baseProject, + lead_member_id: "agent-a", + } as unknown as Project; + + render(); + await flush(); + + const select = screen.getByLabelText("Project lead") as HTMLSelectElement; + // The project's exclusive lead is pre-selected. + expect(select.value).toBe("agent-a"); + + // Promote Beta. Because the lead is a single project pointer, only one + // option can ever be selected, so setting a new lead is inherently + // exclusive (the server unsets the previous one). + await act(async () => { + fireEvent.change(select, { target: { value: "agent-b" } }); + await flush(); + }); + + expect(fetchMock).toHaveBeenCalledWith( + "/api/projects/prj-test/lead", + expect.objectContaining({ + method: "PATCH", + body: JSON.stringify({ member_id: "agent-b" }), + }), + ); + expect((screen.getByLabelText("Project lead") as HTMLSelectElement).value).toBe("agent-b"); + }); + + it("offers a 'No lead' option that clears the lead via setLead(null)", async () => { + const fetchMock = mockFetch({ + "/api/projects/prj-test/members": { + ok: true, + body: { items: [memberRow("agent-a")] }, + }, + "/api/agents": { ok: true, body: [agentA] }, + "/api/agents/registry": { ok: true, body: [] }, + "/api/projects/prj-test/lead": { ok: true, body: { ok: true, lead_member_id: null } }, + }); + vi.stubGlobal("fetch", fetchMock); + + const project = { + ...baseProject, + lead_member_id: "agent-a", + } as unknown as Project; + + render(); + await flush(); + + const select = screen.getByLabelText("Project lead") as HTMLSelectElement; + expect(select.value).toBe("agent-a"); + + await act(async () => { + fireEvent.change(select, { target: { value: "" } }); + await flush(); + }); + + expect(fetchMock).toHaveBeenCalledWith( + "/api/projects/prj-test/lead", + expect.objectContaining({ + method: "PATCH", + body: JSON.stringify({ member_id: null }), + }), + ); }); }); + +const project = { id: "prj-test", name: "taOS", slug: "taos" } as unknown as Project; + +// A consent-flow external agent: the member row keys on the canonical id, and +// the registry entry has an EMPTY handle (this is what the approve flow writes). +const externalMember = { + project_id: "prj-test", + member_id: "grok-taos-20260711-000736", + member_kind: "native", + role: "member", + is_lead: 0, + can_edit_canvas: 0, +}; + +const registryEntry = { + canonical_id: "grok-taos-20260711-000736", + handle: "", + display_name: "grok-taOS", + framework: "grok", + origin: "external-selfjoin", + status: "active", +}; diff --git a/desktop/src/apps/ProjectsApp/ProjectMembers.tsx b/desktop/src/apps/ProjectsApp/ProjectMembers.tsx index 19dab0e4..5e580a5e 100644 --- a/desktop/src/apps/ProjectsApp/ProjectMembers.tsx +++ b/desktop/src/apps/ProjectsApp/ProjectMembers.tsx @@ -75,6 +75,7 @@ function MemberRow({ isExternal, framework, projectId, + isLead, onRefresh, onChanged, }: { @@ -85,6 +86,7 @@ function MemberRow({ isExternal?: boolean; framework?: string; projectId: string; + isLead?: boolean; onRefresh: () => void; onChanged: () => void; }) { @@ -115,7 +117,7 @@ function MemberRow({ {typeLabel} )} - {!!member.is_lead && ( + {isLead && ( ★ Lead @@ -129,36 +131,37 @@ function MemberRow({ {isAgent && ( )} {isAgent && ( )} +
+ + + + exclusive per project + +
    {mainMembers.map((m) => { const { label, emoji, hint } = formatMemberLabel(m.member_id, byId); @@ -311,6 +369,7 @@ export function ProjectMembers({ project, onChanged }: { project: Project; onCha label={label} emoji={emoji} hint={hint} + isLead={m.member_id === leadMemberId} projectId={project.id} onRefresh={refresh} onChanged={onChanged} @@ -330,6 +389,7 @@ export function ProjectMembers({ project, onChanged }: { project: Project; onCha member={m} label={label} hint={hint} + isLead={m.member_id === leadMemberId} isExternal framework={byHandle.get(m.member_id)?.framework} projectId={project.id} diff --git a/desktop/src/apps/ProjectsApp/canvas/canvas-api.ts b/desktop/src/apps/ProjectsApp/canvas/canvas-api.ts index 432dac65..8a41f250 100644 --- a/desktop/src/apps/ProjectsApp/canvas/canvas-api.ts +++ b/desktop/src/apps/ProjectsApp/canvas/canvas-api.ts @@ -92,15 +92,22 @@ export const canvasApi = { return r.ok; }, + // Set a single canvas capability checkbox for an agent member. `flag` selects + // which capability to flip: "read" (can_read_canvas) or "edit" (can_edit_canvas). + // Both default OFF in the store, so the human ticks exactly what each agent + // may do. The backend permission PATCH accepts either field independently. async setPermission( - projectId: string, agentId: string, canEdit: boolean, + projectId: string, agentId: string, flag: "read" | "edit", allowed: boolean, ): Promise { + const body = flag === "read" + ? { can_read_canvas: allowed } + : { can_edit_canvas: allowed }; const r = await fetch( `/api/projects/${projectId}/canvas/permissions/${agentId}`, { method: "PATCH", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ can_edit_canvas: canEdit }), + body: JSON.stringify(body), }, ); if (!r.ok) throw new Error(`setPermission failed: ${r.status}`); diff --git a/desktop/src/lib/projects.test.ts b/desktop/src/lib/projects.test.ts index 19de4398..ba2f7e84 100644 --- a/desktop/src/lib/projects.test.ts +++ b/desktop/src/lib/projects.test.ts @@ -269,27 +269,27 @@ describe("projectsApi.members.remove", () => { }); }); -describe("projectsApi.members.setLead", () => { +describe("projectsApi.setLead (D7, exclusive project pointer)", () => { it("returns result on 200", async () => { - mockFetch({ ok: true, is_lead: true }); - const result = await projectsApi.members.setLead("p-1", "m-1", true); - expect(result.is_lead).toBe(true); + mockFetch({ ok: true, lead_member_id: "m-1" }); + const result = await projectsApi.setLead("p-1", "m-1"); + expect(result.lead_member_id).toBe("m-1"); }); - it("patches with is_lead body", async () => { - const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: async () => ({ ok: true, is_lead: true }) }); + it("patches the project lead endpoint with a member_id body", async () => { + const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: async () => ({ ok: true, lead_member_id: null }) }); global.fetch = fetchMock; - await projectsApi.members.setLead("p-1", "m-1", false); + await projectsApi.setLead("p-1", null); const [url, opts] = fetchMock.mock.calls[0]; - expect(url).toBe("/api/projects/p-1/members/m-1/lead"); + expect(url).toBe("/api/projects/p-1/lead"); expect(opts.method).toBe("PATCH"); const body = JSON.parse(opts.body); - expect(body.is_lead).toBe(false); + expect(body.member_id).toBe(null); }); it("throws on non-ok response", async () => { mockFetchError(403, "forbidden"); - await expect(projectsApi.members.setLead("p-1", "m-1", true)).rejects.toThrow("403"); + await expect(projectsApi.setLead("p-1", "m-1")).rejects.toThrow("403"); }); }); diff --git a/desktop/src/lib/projects.ts b/desktop/src/lib/projects.ts index 6ff38513..51578dfd 100644 --- a/desktop/src/lib/projects.ts +++ b/desktop/src/lib/projects.ts @@ -7,6 +7,7 @@ export type Project = { created_by: string; created_at: number; updated_at: number; + lead_member_id?: string | null; }; export type ProjectMember = { @@ -18,6 +19,7 @@ export type ProjectMember = { memory_seed: "none" | "snapshot" | "empty"; added_at: number; can_edit_canvas?: boolean; + can_read_canvas?: boolean; is_lead?: number; }; @@ -155,6 +157,13 @@ export const projectsApi = { http(`/api/projects/${id}/archive`, { method: "POST" }), remove: (id: string) => http(`/api/projects/${id}`, { method: "DELETE" }), + // D7: the Lead is an exclusive, project-level designation. Pass a member id + // to promote that member, or null to clear the lead. + setLead: (id: string, member_id: string | null) => + http<{ ok: boolean; lead_member_id: string | null }>( + `/api/projects/${id}/lead`, + { method: "PATCH", body: JSON.stringify({ member_id }) }, + ), members: { list: (pid: string) => @@ -171,11 +180,6 @@ export const projectsApi = { }), remove: (pid: string, member_id: string) => http<{ ok: boolean }>(`/api/projects/${pid}/members/${member_id}`, { method: "DELETE" }), - setLead: (pid: string, member_id: string, is_lead: boolean) => - http<{ ok: boolean; is_lead: boolean }>( - `/api/projects/${pid}/members/${member_id}/lead`, - { method: "PATCH", body: JSON.stringify({ is_lead }) }, - ), }, tasks: { diff --git a/tests/projects/test_a2a.py b/tests/projects/test_a2a.py index d6b6290c..5d0235e0 100644 --- a/tests/projects/test_a2a.py +++ b/tests/projects/test_a2a.py @@ -370,13 +370,13 @@ async def enqueue_user_message(self, slug: str, msg: dict) -> None: # --------------------------------------------------------------------------- -# Lead agent — ensure_a2a_channel syncs settings.leads +# Lead agent — ensure_a2a_channel syncs settings.leads from lead_member_id (D7) # --------------------------------------------------------------------------- @pytest.mark.asyncio -async def test_ensure_populates_leads_from_is_lead_flag(stores): - """When a member has is_lead=1, their name appears in settings.leads.""" +async def test_ensure_populates_leads_from_lead_member_id(stores): + """When a member is the project's lead_member_id, their name appears in settings.leads.""" project_store, channel_store = stores p = await project_store.create_project(name="P", slug="lead-basic", created_by="u1") @@ -386,7 +386,7 @@ async def test_ensure_populates_leads_from_is_lead_flag(stores): await project_store.add_member(p["id"], coord_id, member_kind="native") await project_store.add_member(p["id"], worker_id, member_kind="native") - await project_store.set_member_lead(p["id"], coord_id, True) + await project_store.set_lead(p["id"], coord_id) ch = await ensure_a2a_channel(channel_store, project_store, p["id"], config=config) @@ -396,7 +396,7 @@ async def test_ensure_populates_leads_from_is_lead_flag(stores): @pytest.mark.asyncio async def test_ensure_leads_empty_when_no_leads(stores): - """settings.leads is an empty list when no member is marked as lead.""" + """settings.leads is an empty list when no member is the lead.""" project_store, channel_store = stores p = await project_store.create_project(name="P", slug="no-leads", created_by="u1") @@ -411,7 +411,7 @@ async def test_ensure_leads_empty_when_no_leads(stores): @pytest.mark.asyncio async def test_ensure_updates_leads_on_subsequent_call(stores): - """Toggling is_lead and calling ensure again updates settings.leads.""" + """Changing lead_member_id and calling ensure again updates settings.leads.""" project_store, channel_store = stores p = await project_store.create_project(name="P", slug="lead-update", created_by="u1") @@ -422,18 +422,18 @@ async def test_ensure_updates_leads_on_subsequent_call(stores): ch1 = await ensure_a2a_channel(channel_store, project_store, p["id"], config=config) assert ch1["settings"]["leads"] == [] - await project_store.set_member_lead(p["id"], coord_id, True) + await project_store.set_lead(p["id"], coord_id) ch2 = await ensure_a2a_channel(channel_store, project_store, p["id"], config=config) assert ch2["settings"]["leads"] == ["coord"] - await project_store.set_member_lead(p["id"], coord_id, False) + await project_store.set_lead(p["id"], None) ch3 = await ensure_a2a_channel(channel_store, project_store, p["id"], config=config) assert ch3["settings"]["leads"] == [] @pytest.mark.asyncio -async def test_ensure_multiple_leads(stores): - """Multiple lead members all appear in settings.leads, sorted.""" +async def test_ensure_lead_is_exclusive(stores): + """Setting a second lead replaces the first; only one name is ever in settings.leads.""" project_store, channel_store = stores p = await project_store.create_project(name="P", slug="multi-lead", created_by="u1") @@ -443,17 +443,20 @@ async def test_ensure_multiple_leads(stores): await project_store.add_member(p["id"], alpha_id, member_kind="native") await project_store.add_member(p["id"], beta_id, member_kind="native") - await project_store.set_member_lead(p["id"], alpha_id, True) - await project_store.set_member_lead(p["id"], beta_id, True) + await project_store.set_lead(p["id"], alpha_id) - ch = await ensure_a2a_channel(channel_store, project_store, p["id"], config=config) + ch1 = await ensure_a2a_channel(channel_store, project_store, p["id"], config=config) + assert ch1["settings"]["leads"] == ["alpha"] - assert ch["settings"]["leads"] == ["alpha", "beta"] + # Promoting beta must atomically unset alpha. + await project_store.set_lead(p["id"], beta_id) + ch2 = await ensure_a2a_channel(channel_store, project_store, p["id"], config=config) + assert ch2["settings"]["leads"] == ["beta"] @pytest.mark.asyncio async def test_ensure_lead_removed_drops_from_leads(stores): - """Removing a lead member from the project naturally drops them from settings.leads.""" + """Removing the lead member from the project naturally drops them from settings.leads.""" project_store, channel_store = stores p = await project_store.create_project(name="P", slug="lead-drop", created_by="u1") @@ -463,7 +466,7 @@ async def test_ensure_lead_removed_drops_from_leads(stores): await project_store.add_member(p["id"], coord_id, member_kind="native") await project_store.add_member(p["id"], worker_id, member_kind="native") - await project_store.set_member_lead(p["id"], coord_id, True) + await project_store.set_lead(p["id"], coord_id) ch1 = await ensure_a2a_channel(channel_store, project_store, p["id"], config=config) assert ch1["settings"]["leads"] == ["coord"] @@ -473,3 +476,28 @@ async def test_ensure_lead_removed_drops_from_leads(stores): ch2 = await ensure_a2a_channel(channel_store, project_store, p["id"], config=config) assert ch2["settings"]["leads"] == [] assert ch2["members"] == ["worker"] + + +@pytest.mark.asyncio +async def test_ensure_lead_follows_backfilled_column(stores): + """A legacy is_lead=1 flag is backfilled into lead_member_id on init and synced.""" + project_store, channel_store = stores + p = await project_store.create_project(name="P", slug="lead-backfill", created_by="u1") + + coord_id = "iiiijjjjjjjj" + config = _config(_agent("coord", coord_id)) + await project_store.add_member(p["id"], coord_id, member_kind="native") + + # Simulate pre-D7 data: a per-member is_lead flag, no project pointer yet. + await project_store._db.execute( + "UPDATE project_members SET is_lead = 1 WHERE project_id = ? AND member_id = ?", + (p["id"], coord_id), + ) + await project_store._db.commit() + await project_store._backfill_lead_member_id() + + refreshed = await project_store.get_project(p["id"]) + assert refreshed["lead_member_id"] == coord_id + + ch = await ensure_a2a_channel(channel_store, project_store, p["id"], config=config) + assert ch["settings"]["leads"] == ["coord"] diff --git a/tests/projects/test_routes_a2a.py b/tests/projects/test_routes_a2a.py index 1618080f..2ce1d3c7 100644 --- a/tests/projects/test_routes_a2a.py +++ b/tests/projects/test_routes_a2a.py @@ -110,13 +110,13 @@ async def test_project_delete_archives_a2a_channel(client): # --------------------------------------------------------------------------- -# Lead endpoint — PATCH /api/projects/{pid}/members/{mid}/lead +# Lead endpoint — PATCH /api/projects/{pid}/lead (D7, exclusive designee) # --------------------------------------------------------------------------- @pytest.mark.asyncio async def test_set_lead_updates_channel_settings(client): - """PATCHing the lead flag resynchronises settings.leads in the A2A channel.""" + """PATCHing the lead pointer resynchronises settings.leads in the A2A channel.""" agent_id, agent_name = await _test_agent_id(client) pid = (await client.post("/api/projects", json={"name": "P", "slug": "ra2a-lead1"})).json()["id"] await client.post(f"/api/projects/{pid}/members", json={"mode": "native", "agent_id": agent_id}) @@ -128,11 +128,11 @@ async def test_set_lead_updates_channel_settings(client): # Promote to lead res = await client.patch( - f"/api/projects/{pid}/members/{agent_id}/lead", - json={"is_lead": True}, + f"/api/projects/{pid}/lead", + json={"member_id": agent_id}, ) assert res.status_code == 200 - assert res.json()["is_lead"] is True + assert res.json()["lead_member_id"] == agent_id a2a_after = _a2a(await _list_channels(client, pid)) assert a2a_after is not None @@ -140,29 +140,55 @@ async def test_set_lead_updates_channel_settings(client): @pytest.mark.asyncio -async def test_set_lead_false_removes_from_leads(client): - """Unsetting is_lead removes the agent name from settings.leads.""" +async def test_set_lead_null_clears_from_leads(client): + """Clearing the lead pointer (member_id: null) removes the agent from settings.leads.""" agent_id, agent_name = await _test_agent_id(client) pid = (await client.post("/api/projects", json={"name": "P", "slug": "ra2a-lead2"})).json()["id"] await client.post(f"/api/projects/{pid}/members", json={"mode": "native", "agent_id": agent_id}) - await client.patch(f"/api/projects/{pid}/members/{agent_id}/lead", json={"is_lead": True}) + await client.patch(f"/api/projects/{pid}/lead", json={"member_id": agent_id}) res = await client.patch( - f"/api/projects/{pid}/members/{agent_id}/lead", - json={"is_lead": False}, + f"/api/projects/{pid}/lead", + json={"member_id": None}, ) assert res.status_code == 200 + assert res.json()["lead_member_id"] is None a2a = _a2a(await _list_channels(client, pid)) assert a2a is not None assert agent_name not in (a2a["settings"].get("leads") or []) +@pytest.mark.asyncio +async def test_set_lead_exclusive_replaces_previous(client): + """Setting a second lead unsets the first; only one lead ever remains.""" + a1, n1 = await _test_agent_id(client) + # A second distinct agent: mint via registry-independent member add is not + # possible (members must reference config agents), so drive exclusivity + # through two calls against the same test agent path by reusing the route + # with the same member after clearing — verify the pointer is a single cell. + pid = (await client.post("/api/projects", json={"name": "P", "slug": "ra2a-lead-excl"})).json()["id"] + await client.post(f"/api/projects/{pid}/members", json={"mode": "native", "agent_id": a1}) + + first = await client.patch(f"/api/projects/{pid}/lead", json={"member_id": a1}) + assert first.status_code == 200 + assert first.json()["lead_member_id"] == a1 + + # Clear then set again: still exactly one lead (the pointer cannot hold two). + cleared = await client.patch(f"/api/projects/{pid}/lead", json={"member_id": None}) + assert cleared.json()["lead_member_id"] is None + second = await client.patch(f"/api/projects/{pid}/lead", json={"member_id": a1}) + assert second.json()["lead_member_id"] == a1 + + project = (await client.get(f"/api/projects/{pid}")).json() + assert project["lead_member_id"] == a1 + + @pytest.mark.asyncio async def test_set_lead_nonexistent_member_returns_404(client): pid = (await client.post("/api/projects", json={"name": "P", "slug": "ra2a-lead3"})).json()["id"] res = await client.patch( - f"/api/projects/{pid}/members/nonexistent-id/lead", - json={"is_lead": True}, + f"/api/projects/{pid}/lead", + json={"member_id": "nonexistent-id"}, ) assert res.status_code == 404 diff --git a/tests/test_routes_projects.py b/tests/test_routes_projects.py index aab5cdd3..a1a0fd9a 100644 --- a/tests/test_routes_projects.py +++ b/tests/test_routes_projects.py @@ -1,4 +1,5 @@ import pytest +from httpx import ASGITransport, AsyncClient @pytest.mark.asyncio @@ -701,3 +702,78 @@ async def test_ready_tasks_filter_by_element(client): )).json()["items"] assert all(t["element_id"] is None for t in none_items) assert len(none_items) == 1 + + +# --------------------------------------------------------------------------- +# Slice 6 (D7): the Lead designation is an exclusive, project-level pointer +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_set_lead_exclusive_leaves_only_last(client): + """Setting lead B after lead A leaves only B; the pointer holds one lead.""" + pid = (await client.post("/api/projects", json={"name": "P", "slug": "lead-excl"})).json()["id"] + await client.post(f"/api/projects/{pid}/members", json={"mode": "native", "agent_id": "agent-a"}) + await client.post(f"/api/projects/{pid}/members", json={"mode": "native", "agent_id": "agent-b"}) + + r1 = await client.patch(f"/api/projects/{pid}/lead", json={"member_id": "agent-a"}) + assert r1.status_code == 200 + assert r1.json()["lead_member_id"] == "agent-a" + + r2 = await client.patch(f"/api/projects/{pid}/lead", json={"member_id": "agent-b"}) + assert r2.status_code == 200 + assert r2.json()["lead_member_id"] == "agent-b" + + # The pointer cannot hold two leads; only the last one remains. + proj = (await client.get(f"/api/projects/{pid}")).json() + assert proj["lead_member_id"] == "agent-b" + + +@pytest.mark.asyncio +async def test_set_lead_non_member_returns_404(client): + pid = (await client.post("/api/projects", json={"name": "P", "slug": "lead-404"})).json()["id"] + await client.post(f"/api/projects/{pid}/members", json={"mode": "native", "agent_id": "agent-a"}) + + r = await client.patch(f"/api/projects/{pid}/lead", json={"member_id": "not-a-member"}) + assert r.status_code == 404 + + +@pytest.mark.asyncio +async def test_clear_lead_to_null(client): + pid = (await client.post("/api/projects", json={"name": "P", "slug": "lead-null"})).json()["id"] + await client.post(f"/api/projects/{pid}/members", json={"mode": "native", "agent_id": "agent-a"}) + + r = await client.patch(f"/api/projects/{pid}/lead", json={"member_id": "agent-a"}) + assert r.status_code == 200 + assert r.json()["lead_member_id"] == "agent-a" + assert (await client.get(f"/api/projects/{pid}")).json()["lead_member_id"] == "agent-a" + + r = await client.patch(f"/api/projects/{pid}/lead", json={"member_id": None}) + assert r.status_code == 200 + assert r.json()["lead_member_id"] is None + assert (await client.get(f"/api/projects/{pid}")).json()["lead_member_id"] is None + + +@pytest.mark.asyncio +async def test_set_lead_requires_session(app, client): + """The lead route is session-only: an unauthenticated request is rejected.""" + pid = (await client.post("/api/projects", json={"name": "P", "slug": "lead-auth"})).json()["id"] + await client.post(f"/api/projects/{pid}/members", json={"mode": "native", "agent_id": "agent-a"}) + + transport = ASGITransport(app=app) + async with AsyncClient(transport=transport, base_url="http://test") as anon: + r = await anon.patch(f"/api/projects/{pid}/lead", json={"member_id": "agent-a"}) + assert r.status_code in (401, 403) + + +@pytest.mark.asyncio +async def test_old_per_member_lead_route_removed(client): + """The retired per-member lead route is gone (replaced by the project pointer).""" + pid = (await client.post("/api/projects", json={"name": "P", "slug": "lead-old"})).json()["id"] + await client.post(f"/api/projects/{pid}/members", json={"mode": "native", "agent_id": "agent-a"}) + + r = await client.patch( + f"/api/projects/{pid}/members/agent-a/lead", json={"is_lead": True} + ) + assert r.status_code == 404 + diff --git a/tinyagentos/projects/a2a.py b/tinyagentos/projects/a2a.py index 5d944952..85bdddcc 100644 --- a/tinyagentos/projects/a2a.py +++ b/tinyagentos/projects/a2a.py @@ -104,31 +104,28 @@ def _resolve_member_names(member_rows: list[dict], config) -> set[str]: return resolved -def _resolve_lead_names(member_rows: list[dict], config) -> list[str]: - """Return sorted list of resolved agent names for members where is_lead = 1. - - Uses the same config-lookup strategy as _resolve_member_names. Returns - names in sorted order for deterministic storage. +def _resolve_lead_names(project: dict, config) -> list[str]: + """Return the resolved agent name for the project's single exclusive lead. + + The lead designation lives on the project as a single ``lead_member_id`` + pointer (D7), not on the members. At most one name is ever returned; an + empty list means no lead. Uses the same config-lookup strategy as + _resolve_member_names so the quiet-filter and settings.leads stay in the + name-space the @mention parser and router expect. """ - leads = [m for m in member_rows if m.get("is_lead")] - if not leads: + lead_id = project.get("lead_member_id") + if not lead_id: return [] by_id, by_name = _build_agent_lookups(config) - result: list[str] = [] - for m in leads: - mid = m["member_id"] - if by_id is None: - # No config (test path): use member_id as name directly. - result.append(mid) - continue - agent = by_id.get(mid) or by_name.get(mid) - if agent is None: - logger.debug("a2a: lead member_id %r not found in config — skipping", mid) - continue - name = agent.get("name") - if name: - result.append(name) - return sorted(set(result)) + if by_id is None: + # No config (test path): use the member id directly. + return [lead_id] + agent = by_id.get(lead_id) or by_name.get(lead_id) + if agent is None: + logger.debug("a2a: lead member_id %r not found in config — skipping", lead_id) + return [] + name = agent.get("name") + return [name] if name else [] async def ensure_a2a_channel( @@ -143,21 +140,21 @@ async def ensure_a2a_channel( router both operate on names. When None (test path with name-keyed members), member_ids are used as-is. - Also syncs settings.leads from project members where is_lead = 1 so that - the router can give leads visibility into all channel messages regardless + Also syncs settings.leads from the project's exclusive lead_member_id so that + the router can give the lead visibility into all channel messages regardless of response_mode. Idempotent. Serialized per project_id via _A2A_LOCKS to prevent racing member-sync diffs when multiple callers fire concurrently. """ async with _A2A_LOCKS[project_id]: + project = await project_store.get_project(project_id) project_members = await project_store.list_members(project_id) expected = _resolve_member_names(project_members, config) - expected_leads = _resolve_lead_names(project_members, config) + expected_leads = _resolve_lead_names(project, config) matches = await _find_a2a_channels(channel_store, project_id) if not matches: - project = await project_store.get_project(project_id) created_by = project.get("created_by", "system") if project else "system" return await channel_store.create_channel( name=A2A_NAME, diff --git a/tinyagentos/projects/project_store.py b/tinyagentos/projects/project_store.py index 7f19aba1..ecabdbeb 100644 --- a/tinyagentos/projects/project_store.py +++ b/tinyagentos/projects/project_store.py @@ -20,6 +20,7 @@ updated_at REAL NOT NULL, archived_at REAL, deleted_at REAL, + lead_member_id TEXT, settings TEXT NOT NULL DEFAULT '{}' ); CREATE INDEX IF NOT EXISTS idx_projects_status ON projects(status); @@ -70,6 +71,7 @@ async def _post_init(self) -> None: "ALTER TABLE project_members ADD COLUMN can_read_canvas INTEGER NOT NULL DEFAULT 0", "ALTER TABLE project_members ADD COLUMN is_lead INTEGER NOT NULL DEFAULT 0", "ALTER TABLE projects ADD COLUMN user_id TEXT NOT NULL DEFAULT ''", + "ALTER TABLE projects ADD COLUMN lead_member_id TEXT", ): try: await self._db.execute(col_def) @@ -77,16 +79,50 @@ async def _post_init(self) -> None: except Exception: # Column already exists on fresh installs (created by SCHEMA). pass + # D7 backfill: the exclusive per-project lead moves from the per-member + # is_lead flag (non-exclusive) to the project's single lead_member_id + # pointer. Only projects with EXACTLY one flagged member are migrated; + # any with zero or several flagged members are left NULL so a human picks + # in the UI (the old flag never promised exclusivity). + await self._backfill_lead_member_id() - async def set_member_lead(self, project_id: str, member_id: str, is_lead: bool) -> None: - val = 1 if is_lead else 0 - cur = await self._db.execute( - "UPDATE project_members SET is_lead = ? WHERE project_id = ? AND member_id = ?", - (val, project_id, member_id), + async def _backfill_lead_member_id(self) -> None: + rows = await (await self._db.execute( + "SELECT id FROM projects WHERE lead_member_id IS NULL" + )).fetchall() + for (pid,) in rows: + cur = await self._db.execute( + "SELECT member_id FROM project_members WHERE project_id = ? AND is_lead = 1", + (pid,), + ) + flagged = [r[0] for r in await cur.fetchall()] + if len(flagged) == 1: + await self._db.execute( + "UPDATE projects SET lead_member_id = ? WHERE id = ?", + (flagged[0], pid), + ) + await self._db.commit() + + async def set_lead(self, project_id: str, member_id: "str | None") -> None: + """Set (or clear, when member_id is None) the project's exclusive lead. + + The single pointer column makes the one-lead-per-project invariant + structural: setting a new lead atomically unsets any previous one, and + there is no partial-update window. A member_id that is not a current + member of the project raises KeyError (the route maps it to 404). + """ + p = await self.get_project(project_id) + if p is None: + raise KeyError(f"project {project_id!r} not found") + if member_id is not None: + member = await self.get_member(project_id, member_id) + if member is None: + raise KeyError(f"member {member_id!r} not in project {project_id!r}") + await self._db.execute( + "UPDATE projects SET lead_member_id = ? WHERE id = ?", + (member_id, project_id), ) await self._db.commit() - if cur.rowcount == 0: - raise KeyError(f"member {member_id!r} not in project {project_id!r}") async def create_project( self, @@ -234,6 +270,13 @@ async def remove_member(self, project_id: str, member_id: str) -> None: "DELETE FROM project_members WHERE project_id = ? AND member_id = ?", (project_id, member_id), ) + # Removing the designated lead unsets the pointer so it can never dangle + # on a member that no longer belongs to the project. + await self._db.execute( + "UPDATE projects SET lead_member_id = NULL " + "WHERE id = ? AND lead_member_id = ?", + (project_id, member_id), + ) await self._db.commit() async def list_members(self, project_id: str) -> list[dict]: diff --git a/tinyagentos/routes/projects.py b/tinyagentos/routes/projects.py index c7a6d4b1..e2bd8d5e 100644 --- a/tinyagentos/routes/projects.py +++ b/tinyagentos/routes/projects.py @@ -296,27 +296,38 @@ async def list_members( return {"items": await store.list_members(project_id)} -class LeadIn(BaseModel): - is_lead: bool +class ProjectLeadIn(BaseModel): + member_id: "str | None" = None -@router.patch("/api/projects/{project_id}/members/{member_id}/lead") -async def set_lead( +@router.patch("/api/projects/{project_id}/lead") +async def set_project_lead( project_id: str, - member_id: str, - body: LeadIn, + body: ProjectLeadIn, request: Request, user: CurrentUser = Depends(current_user), ): + """Set the project's exclusive Lead (D7). The single lead_member_id pointer + makes the one-lead-per-project invariant structural: setting a new lead + atomically unsets the previous one. ``member_id: null`` clears the lead. + + Session-only (owner or admin, same gate as the members routes). A member id + not in the project returns 404. + """ store = request.app.state.project_store p = await store.get_project(project_id) if p is None: return JSONResponse({"error": "not found"}, status_code=404) require_owner_or_admin(user, p["user_id"]) try: - await store.set_member_lead(project_id, member_id, body.is_lead) + await store.set_lead(project_id, body.member_id) except KeyError as e: return JSONResponse({"error": str(e)}, status_code=404) + await store.log_activity( + project_id, user.user_id, "project.lead_changed", {"member_id": body.member_id} + ) + members = await store.list_members(project_id) + _mirror(request, {**p, "members": members}) try: from tinyagentos.projects.a2a import ensure_a2a_channel await ensure_a2a_channel( @@ -327,7 +338,7 @@ async def set_lead( ) except Exception: logger.warning("a2a ensure failed for project %s on set_lead", project_id, exc_info=True) - return {"ok": True, "is_lead": body.is_lead} + return {"ok": True, "lead_member_id": body.member_id} @router.delete("/api/projects/{project_id}/members/{member_id}")