From 4099b0d1c49461d536a1d441893442b649216e71 Mon Sep 17 00:00:00 2001 From: Blomios Date: Tue, 14 Jul 2026 18:56:34 +0200 Subject: [PATCH] =?UTF-8?q?fix(layout):=20rendre=20res=C3=A9lectionnable?= =?UTF-8?q?=20un=20agent=20live=20non=20r=C3=A9ellement=20affich=C3=A9=20(?= =?UTF-8?q?#56)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le sélecteur d'agent de LayoutGrid désactivait un agent en se fondant sur le seul liveAgents.nodeId : un agent live dont l'ancienne cellule affiche désormais un autre agent restait grisé (« visible ailleurs ») alors qu'il n'était plus affiché nulle part, donc impossible à resélectionner. La dérivation « visible ailleurs » s'appuie maintenant sur le layout courant (feuille visible ≠ cellule courante ET leaf.agent === candidate). Un agent live dont l'ancienne cellule montre un autre agent redevient sélectionnable dans une autre cellule ; la session n'est jamais tuée (attachLiveAgent conservé). Le prop mort visibleNodeIds est retiré du threading. Tests : frontend/src/features/layout/singletonAgent.test.tsx. Suite frontend verte 706/706. Co-Authored-By: Claude Opus 4.8 --- frontend/src/features/layout/LayoutGrid.tsx | 70 +++++----- .../features/layout/singletonAgent.test.tsx | 120 +++++++++++++++++- 2 files changed, 154 insertions(+), 36 deletions(-) diff --git a/frontend/src/features/layout/LayoutGrid.tsx b/frontend/src/features/layout/LayoutGrid.tsx index 33d4df5..f6b7e0f 100644 --- a/frontend/src/features/layout/LayoutGrid.tsx +++ b/frontend/src/features/layout/LayoutGrid.tsx @@ -98,8 +98,6 @@ export function LayoutGrid({ projectId, cwd, layoutId, onOpenConversation }: Lay ); } - const visibleNodeIds = new Set(leaves(vm.layout).map((l) => l.id)); - return (
; workState: ProjectWorkState | null; refreshWorkState: () => Promise; onOpenConversation?: (conversationId: string) => void; @@ -144,7 +140,6 @@ function NodeView({ vm, parentSplit, projectId, - visibleNodeIds, workState, refreshWorkState, onOpenConversation, @@ -162,7 +157,6 @@ function NodeView({ vm={vm} parentSplit={parentSplit} projectId={projectId} - visibleNodeIds={visibleNodeIds} workState={workState} refreshWorkState={refreshWorkState} onOpenConversation={onOpenConversation} @@ -175,7 +169,6 @@ function NodeView({ cwd={cwd} vm={vm} projectId={projectId} - visibleNodeIds={visibleNodeIds} workState={workState} refreshWorkState={refreshWorkState} onOpenConversation={onOpenConversation} @@ -188,7 +181,6 @@ function NodeView({ cwd={cwd} vm={vm} projectId={projectId} - visibleNodeIds={visibleNodeIds} workState={workState} refreshWorkState={refreshWorkState} onOpenConversation={onOpenConversation} @@ -207,7 +199,6 @@ interface LeafViewProps { vm: LayoutViewModel; parentSplit: { container: string; index: number; siblings: number } | null; projectId: string; - visibleNodeIds: Set; workState: ProjectWorkState | null; refreshWorkState: () => Promise; onOpenConversation?: (conversationId: string) => void; @@ -289,7 +280,6 @@ function LeafView({ vm, parentSplit, projectId, - visibleNodeIds, workState, refreshWorkState, onOpenConversation, @@ -448,20 +438,37 @@ function LeafView({ }; }; - /** True when the live session is already displayed by another visible cell. */ - const visibleElsewhere = (candidate: string): LiveAgent | undefined => { - const live = liveFor(candidate); - return live && live.nodeId !== id && visibleNodeIds.has(live.nodeId) - ? live - : undefined; + /** + * The cell that still displays `candidate` in the CURRENT layout, if any: a + * visible leaf other than this one whose pinned agent IS `candidate`. + * + * Ticket #56 — derived from the layout, NOT from `liveAgents[].nodeId`. The + * live registry keeps pointing at an agent's LAST host cell even after that + * cell swapped to another agent (or none): reading `live.nodeId` as "displayed + * here" wrongly disabled the agent everywhere else, although it is no longer + * shown anywhere. An agent truly shown in a visible cell stays non-selectable + * elsewhere (singleton display invariant); a stale live association does not. + */ + const visibleElsewhere = (candidate: string): { nodeId: string } | undefined => { + if (!vm.layout) return undefined; + const host = leaves(vm.layout).find( + (leaf) => leaf.id !== id && (leaf.agent ?? null) === candidate, + ); + return host ? { nodeId: host.id } : undefined; }; - /** A live session whose previous host cell no longer exists in the layout. */ + /** + * A live PTY session for `candidate` that this cell can re-attach without + * re-spawning: it exists, is not recorded on THIS cell (`nodeId === id`, so a + * fresh self-launch never triggers a re-attach loop), and is not currently + * displayed by another visible cell ({@link visibleElsewhere} — the singleton + * double-display case). A session whose former host cell now shows a different + * agent (or none) is a background session → selectable here (ticket #56). + */ const backgroundLive = (candidate: string): LiveAgent | undefined => { const live = liveFor(candidate); - return live && live.kind === "pty" && live.nodeId !== id && !visibleNodeIds.has(live.nodeId) - ? live - : undefined; + if (!live || live.kind !== "pty" || live.nodeId === id) return undefined; + return visibleElsewhere(candidate) ? undefined : live; }; // A transient notice shown when an action is blocked by the singleton @@ -665,21 +672,20 @@ function LeafView({ ? await agentGateway.listLiveAgents(projectId).catch(() => liveAgents) : liveAgents; setLiveAgents(current); - const live = current.find((la) => la.agentId === val); - const isVisible = - live && live.nodeId !== id && visibleNodeIds.has(live.nodeId); - if (isVisible) { + // Visibility is derived from the CURRENT layout, not the live + // registry's stale last-known nodeId (ticket #56): the agent is + // "visible ailleurs" only while a visible cell still pins it. + const elsewhere = visibleElsewhere(val); + if (elsewhere) { setBusyNotice({ message: "Cet agent est déjà actif dans une autre cellule.", - goToNodeId: live!.nodeId, + goToNodeId: elsewhere.nodeId, }); return; } + const live = current.find((la) => la.agentId === val); const isBackground = - live && - live.kind === "pty" && - live.nodeId !== id && - !visibleNodeIds.has(live.nodeId); + live && live.kind === "pty" && live.nodeId !== id; if (isBackground) { if (!agentGateway?.attachLiveAgent || !live.sessionId) { setBusyNotice({ @@ -1143,7 +1149,6 @@ interface SplitViewProps { cwd: string; vm: LayoutViewModel; projectId: string; - visibleNodeIds: Set; workState: ProjectWorkState | null; refreshWorkState: () => Promise; onOpenConversation?: (conversationId: string) => void; @@ -1154,7 +1159,6 @@ function SplitView({ cwd, vm, projectId, - visibleNodeIds, workState, refreshWorkState, onOpenConversation, @@ -1199,7 +1203,6 @@ function SplitView({ cwd={cwd} vm={vm} projectId={projectId} - visibleNodeIds={visibleNodeIds} workState={workState} refreshWorkState={refreshWorkState} onOpenConversation={onOpenConversation} @@ -1294,7 +1297,6 @@ interface GridViewProps { cwd: string; vm: LayoutViewModel; projectId: string; - visibleNodeIds: Set; workState: ProjectWorkState | null; refreshWorkState: () => Promise; onOpenConversation?: (conversationId: string) => void; @@ -1305,7 +1307,6 @@ function GridView({ cwd, vm, projectId, - visibleNodeIds, workState, refreshWorkState, onOpenConversation, @@ -1346,7 +1347,6 @@ function GridView({ vm={vm} parentSplit={null} projectId={projectId} - visibleNodeIds={visibleNodeIds} workState={workState} refreshWorkState={refreshWorkState} onOpenConversation={onOpenConversation} diff --git a/frontend/src/features/layout/singletonAgent.test.tsx b/frontend/src/features/layout/singletonAgent.test.tsx index 47a9c48..fb77eda 100644 --- a/frontend/src/features/layout/singletonAgent.test.tsx +++ b/frontend/src/features/layout/singletonAgent.test.tsx @@ -274,7 +274,14 @@ describe("R0d — dropdown disables agents live elsewhere + go-to-cell", () => { const ag = await agent.createAgent("p1", { name: "Busy", profileId: "p" }); const { a, b } = await splitInTwo(layout); - // The agent is live in cell B (a currently-visible leaf). + // The agent is DISPLAYED in cell B (a currently-visible leaf) and live there. + // "Visible ailleurs" is derived from the layout, so the cell must actually + // pin the agent, not merely have the live registry point at it (ticket #56). + await layout.mutateLayout("p1", { + type: "setCellAgent", + target: b, + agent: ag.id, + }); vi.spyOn(agent, "listLiveAgents").mockResolvedValue([ { agentId: ag.id, nodeId: b, sessionId: "s-1", kind: "pty" }, ]); @@ -328,3 +335,114 @@ describe("R0d — dropdown disables agents live elsewhere + go-to-cell", () => { }); }); + +describe("ticket #56 — 'visible ailleurs' derives from the layout, not stale liveAgents.nodeId", () => { + function renderGrid(layout: MockLayoutGateway, agent: MockAgentGateway) { + const system = { + onDomainEvent: vi.fn().mockResolvedValue(() => {}), + }; + const gateways = { layout, agent, system } as unknown as Gateways; + return render( + + + , + ); + } + + /** Splits the project's single leaf into two visible leaves; returns both ids. */ + async function splitInTwo( + layout: MockLayoutGateway, + ): Promise<{ a: string; b: string }> { + const initial = await layout.loadLayout("p1"); + const a = leaves(initial)[0].id; + await layout.mutateLayout("p1", { + type: "split", + target: a, + direction: "row", + newLeaf: "b", + container: "c", + }); + const after = leaves(await layout.loadLayout("p1")); + return { a, b: after.find((l) => l.id !== a)!.id }; + } + + it("an agent whose former host cell now shows ANOTHER agent is selectable (background, not 'visible ailleurs')", async () => { + const layout = new MockLayoutGateway(); + const agent = new MockAgentGateway(); + const x = await agent.createAgent("p1", { name: "Xavier", profileId: "p" }); + const y = await agent.createAgent("p1", { name: "Yann", profileId: "p" }); + const { a, b } = await splitInTwo(layout); + + // X was launched in cell A → the live registry records X → nodeId A. + await agent.launchAgent( + "p1", + x.id, + { cwd: "/x", rows: 24, cols: 80, nodeId: a }, + noop, + ); + // …then cell A was switched to display agent Y. The live registry still + // points X at A (stale), but A no longer displays X — X is displayed nowhere. + await layout.mutateLayout("p1", { + type: "setCellAgent", + target: a, + agent: y.id, + }); + + const attach = vi.spyOn(agent, "attachLiveAgent"); + renderGrid(layout, agent); + + // In cell B's dropdown, X is ENABLED and flagged as a background session — + // NOT "visible ailleurs" (regression: it used to be disabled via the stale + // live nodeId still pointing at the now-visible cell A). + const selectB = (await screen.findByLabelText( + `agent selector ${b}`, + )) as HTMLSelectElement; + await waitFor(() => { + const opt = Array.from(selectB.options).find((o) => o.value === x.id)!; + expect(opt.disabled).toBe(false); + expect(opt.textContent).toMatch(/arrière-plan/); + expect(opt.textContent).not.toMatch(/visible ailleurs/); + }); + + // Selecting X in B re-attaches its live session (no re-spawn) and pins it here. + fireEvent.change(selectB, { target: { value: x.id } }); + await waitFor(async () => { + const leaf = leaves(await layout.loadLayout("p1")).find((l) => l.id === b)!; + expect(leaf.agent).toBe(x.id); + expect(leaf.session).toBe("mock-agent-session-1"); + }); + expect(attach).toHaveBeenCalledWith("p1", x.id, b); + }); + + it("an agent STILL pinned in another visible cell stays disabled ('visible ailleurs')", async () => { + const layout = new MockLayoutGateway(); + const agent = new MockAgentGateway(); + const x = await agent.createAgent("p1", { name: "Xavier", profileId: "p" }); + const { a, b } = await splitInTwo(layout); + + // Cell A still displays X (and X is live there): the singleton display + // invariant must keep X non-selectable in cell B. + await layout.mutateLayout("p1", { + type: "setCellAgent", + target: a, + agent: x.id, + }); + await agent.launchAgent( + "p1", + x.id, + { cwd: "/x", rows: 24, cols: 80, nodeId: a }, + noop, + ); + + renderGrid(layout, agent); + + const selectB = (await screen.findByLabelText( + `agent selector ${b}`, + )) as HTMLSelectElement; + await waitFor(() => { + const opt = Array.from(selectB.options).find((o) => o.value === x.id)!; + expect(opt.disabled).toBe(true); + expect(opt.textContent).toMatch(/visible ailleurs/); + }); + }); +});