diff --git a/frontend/src/adapters/agent.ts b/frontend/src/adapters/agent.ts index deee107b..ebbc3642 100644 --- a/frontend/src/adapters/agent.ts +++ b/frontend/src/adapters/agent.ts @@ -240,8 +240,8 @@ export class TauriAgentGateway implements AgentGateway { // backend select a chat session, whose reply stream can never feed the // xterm output channel and leaves the cell looking blank. cellKind: "pty", - // Resume id: the leaf's persisted conversation id, when any (T4b). The - // backend resumes it; absent ⇒ a fresh cell that may get a new id. + // Explicit resume id, when the caller intentionally selected one. An + // ordinary launch never derives this value from the hosting leaf (#371). conversationId: options.conversationId ?? null, freshStart: options.freshStart ?? false, // Hosting cell: lets the backend enforce one-live-session-per-agent. @@ -255,8 +255,8 @@ export class TauriAgentGateway implements AgentGateway { } const base = makeTerminalHandle(res.sessionId, channel); - // Surface the id assigned by this launch (so the caller persists it on the - // leaf and resumes next time). + // Surface the id assigned by this launch so the caller can persist it as a + // runtime/history pivot without making it the next default UI selection. return { ...base, ...(res.assignedConversationId diff --git a/frontend/src/domain/index.ts b/frontend/src/domain/index.ts index 8d2a045f..7daef582 100644 --- a/frontend/src/domain/index.ts +++ b/frontend/src/domain/index.ts @@ -1060,8 +1060,9 @@ export type PreferredView = "tui" | "chat"; /** A terminal-hosting leaf cell. `session` is the hosted SessionId, if any. * `agent` is the agent id if an agent is pinned to this cell (absent = plain terminal). - * `conversationId` is the persistent CLI conversation id (survives PTY close/reopen, - * lets the agent resume); omitted when absent (mirrors the backend `skip_serializing_if`). + * `conversationId` is a persistent runtime/history pivot. It can support an + * explicit resume, but is not the default active UI selection (#371); omitted + * when absent (mirrors the backend `skip_serializing_if`). * `agentWasRunning` records whether the cell's agent process was running at close time; * omitted when `false`. */ export interface LeafCell { diff --git a/frontend/src/features/agents/CustomAgentChatView.test.tsx b/frontend/src/features/agents/CustomAgentChatView.test.tsx index 74f96608..bc190d9c 100644 --- a/frontend/src/features/agents/CustomAgentChatView.test.tsx +++ b/frontend/src/features/agents/CustomAgentChatView.test.tsx @@ -4839,6 +4839,7 @@ describe("CustomAgentChatView", () => { rows: 24, cols: 80, conversationId: undefined, + freshStart: true, nodeId: "node-1", }), ); @@ -6719,6 +6720,7 @@ describe("CustomAgentChatView", () => { await waitFor(() => expect(agent.launchAgentChat).toHaveBeenCalledTimes(1)); expect(agent.launchAgentChat.mock.calls[0]?.[2].conversationId).toBeUndefined(); + expect(agent.launchAgentChat.mock.calls[0]?.[2].freshStart).toBe(true); fireEvent.click(screen.getByRole("button", { name: "Conversations" })); const past = await screen.findByRole("menuitem", { name: /Saved question/ }); fireEvent.click(past); @@ -6735,6 +6737,13 @@ describe("CustomAgentChatView", () => { ); fireEvent.click(screen.getByRole("button", { name: "Envoyer" })); expect(await screen.findByText("Conversation poursuivie")).toBeTruthy(); + expect(agent.launchAgentChat).toHaveBeenNthCalledWith(2, "project-1", "agent-1", { + cwd: "/repo", + rows: 24, + cols: 80, + conversationId: "past-conversation", + nodeId: "node-1", + }); await waitFor(() => expect(agent.sendAgentChat).toHaveBeenCalledWith( "resumed-session", "Continue", diff --git a/frontend/src/features/agents/CustomAgentChatView.tsx b/frontend/src/features/agents/CustomAgentChatView.tsx index 7018bf39..2919f3c0 100644 --- a/frontend/src/features/agents/CustomAgentChatView.tsx +++ b/frontend/src/features/agents/CustomAgentChatView.tsx @@ -1069,8 +1069,8 @@ export function CustomAgentChatView({ () => new Set(), ); const [currentSession, setCurrentSession] = useState(sessionId); - // `freshOnOpen` is reserved for an explicit reset intent. Ordinary React - // remounts keep the durable conversation selected by the hosting cell. + // A normal cell opening starts fresh when no live session exists. React + // remounts with a live session still preserve that runtime continuity. const [activeConversationId, setActiveConversationId] = useState( sessionId || !freshOnOpen ? conversationId : null, ); @@ -2005,6 +2005,7 @@ export function CustomAgentChatView({ options.conversationId === undefined ? activeConversationIdRef.current ?? undefined : options.conversationId ?? undefined, + ...(options.conversationId === null ? { freshStart: true } : {}), ...(nodeId ? { nodeId } : {}), }; console.debug("[ticket149] recoverStructuredSession:launchAgentChat:start", { @@ -2266,6 +2267,29 @@ export function CustomAgentChatView({ setSavedConversationLoadState("idle"); setLiveSessionAttached(false); if (freshOnOpen) { + // A missing layout session can be a transient remount while this + // agent is still active. Preserve that live continuity without + // treating the leaf's persisted conversation id as a UI choice. + const live = agent.listLiveAgents + ? (await agent.listLiveAgents(projectId)).find( + (candidate) => + candidate.agentId === agentId && candidate.kind === "structured", + ) + : undefined; + if (live) { + const rebound = + nodeId && live.nodeId !== nodeId && agent.attachLiveAgent + ? await agent.attachLiveAgent(projectId, agentId, nodeId) + : live; + const attachedSessionId = await reattachStructuredSession(rebound.sessionId, { + applyScrollback: true, + preserveViewOnEmptyScrollback: true, + }); + if (cancelled) return; + publishSessionId(attachedSessionId); + setResumedConversation(false); + return; + } await recoverStructuredSession({ applyScrollback: false, retryAttachNotFound: true, diff --git a/frontend/src/features/layout/LayoutGrid.chat.test.tsx b/frontend/src/features/layout/LayoutGrid.chat.test.tsx index 0b062f0c..5bad6024 100644 --- a/frontend/src/features/layout/LayoutGrid.chat.test.tsx +++ b/frontend/src/features/layout/LayoutGrid.chat.test.tsx @@ -210,7 +210,7 @@ describe("LayoutGrid custom agent CLI (#147)", () => { expect(window.localStorage.getItem(`idea.agent-cell-mode.p1.${leafId}`)).toBe("custom"); }); - it("restores durable messages after project A → B → A without starting a provider", async () => { + it("starts fresh on first project open, then preserves the live session across A → B → A", async () => { const layout = new MockLayoutGateway(); const agent = new MockAgentGateway(); const profileGateway = new MockProfileGateway(); @@ -271,8 +271,11 @@ describe("LayoutGrid custom agent CLI (#147)", () => { const view = renderGrid(gateways, "p1"); expect(await screen.findByTestId("custom-agent-chat-view")).toBeTruthy(); - expect(await screen.findByText("Question projet A")).toBeTruthy(); - expect(screen.getByText("Réponse projet A")).toBeTruthy(); + await waitFor(() => expect(launchChat).toHaveBeenCalledTimes(1)); + expect(launchChat.mock.calls[0]?.[2]).toMatchObject({ freshStart: true }); + expect(launchChat.mock.calls[0]?.[2].conversationId).toBeUndefined(); + expect(screen.queryByText("Question projet A")).toBeNull(); + expect(await screen.findByText("Nouvelle conversation")).toBeTruthy(); const persistedA = leaves(await layout.loadLayout("p1"))[0]; view.rerender( @@ -293,13 +296,13 @@ describe("LayoutGrid custom agent CLI (#147)", () => { expect(screen.queryByRole("alertdialog", { name: "Confirmer le changement de CLI" })) .toBeNull(); expect(screen.queryByText(/Aucune conversation reprenable/)).toBeNull(); - expect(await screen.findByText("Question projet A")).toBeTruthy(); - expect(screen.getByText("Réponse projet A")).toBeTruthy(); + expect(screen.queryByText("Question projet A")).toBeNull(); + expect(await screen.findByText("Nouvelle conversation")).toBeTruthy(); const restoredA = leaves(await layout.loadLayout("p1"))[0]; expect(restoredA.preferredView).toBe("chat"); expect(restoredA.session).toBe(persistedA.session); expect(restoredA.conversationId).toBe(persistedA.conversationId); - expect(launchChat).not.toHaveBeenCalled(); + expect(launchChat).toHaveBeenCalledTimes(1); }); it("keeps custom CLI and composer active while busy, then queues without sending (#247/#352)", async () => { @@ -367,6 +370,9 @@ describe("LayoutGrid custom agent CLI (#147)", () => { const leaf = leaves(tree)[0]; const requesterId = leaf.agent; if (!requesterId) throw new Error("expected seeded requester"); + vi.spyOn(gateways.agent, "launchAgentChat").mockImplementation( + () => new Promise(() => {}), + ); window.localStorage.setItem(`idea.agent-cell-mode.p1.${leaf.id}`, "custom"); workState._setProjectWorkState("p1", { agents: [ @@ -497,7 +503,7 @@ describe("LayoutGrid custom agent CLI (#147)", () => { expect(screen.queryByText(/déjà actif dans une autre cellule/)).toBeNull(); }); - it("restores a saved custom CLI conversation when returning without a live session (#366)", async () => { + it("starts a fresh custom CLI conversation instead of restoring the leaf pivot (#371)", async () => { const layout = new MockLayoutGateway(); const agent = new MockAgentGateway(); const profileGateway = new MockProfileGateway(); @@ -545,14 +551,39 @@ describe("LayoutGrid custom agent CLI (#147)", () => { conversation, } as unknown as Gateways); - expect(await screen.findByText("known prompt")).toBeTruthy(); - expect(screen.getByText("known response")).toBeTruthy(); - expect(screen.queryByText("Nouvelle conversation")).toBeNull(); - expect(launchChat).not.toHaveBeenCalled(); - expect(reattachChat).not.toHaveBeenCalled(); + await waitFor(() => expect(launchChat).toHaveBeenCalledTimes(1)); + expect(launchChat.mock.calls[0]?.[2]).toMatchObject({ freshStart: true }); + expect(launchChat.mock.calls[0]?.[2].conversationId).toBeUndefined(); + expect(screen.queryByText("known prompt")).toBeNull(); + expect(screen.queryByText("known response")).toBeNull(); + expect(await screen.findByText("Nouvelle conversation")).toBeTruthy(); + expect(reattachChat).toHaveBeenCalledWith("chat-fresh", expect.any(Function)); const updated = await layout.loadLayout("p1"); - expect(leaves(updated)[0].conversationId).toBe("conversation-known"); - expect(leaves(updated)[0].session).toBeNull(); + expect(leaves(updated)[0].conversationId).toBe("conversation-fresh"); + expect(leaves(updated)[0].session).toBe("chat-fresh"); + }); + + it("launches a normal native agent fresh even when the leaf stores an old conversation (#371)", async () => { + const gateways = await seeded(ptyProfile); + const layout = gateways.layout as MockLayoutGateway; + const agent = gateways.agent as MockAgentGateway; + const leaf = leaves(await layout.loadLayout("p1"))[0]; + await layout.mutateLayout("p1", { + type: "setCellConversation", + target: leaf.id, + conversationId: "old-conversation", + }); + const launch = vi.spyOn(agent, "launchAgent"); + + renderGrid(gateways); + + await waitFor(() => expect(launch).toHaveBeenCalledTimes(1)); + expect(launch.mock.calls[0]?.[2]).toMatchObject({ + cwd: "/home/me/proj", + nodeId: leaf.id, + freshStart: true, + }); + expect(launch.mock.calls[0]?.[2].conversationId).toBeUndefined(); }); it("relaunches a stopped custom CLI agent from scratch (#197)", async () => { diff --git a/frontend/src/features/layout/LayoutGrid.tsx b/frontend/src/features/layout/LayoutGrid.tsx index ed6a7b0e..8779abe9 100644 --- a/frontend/src/features/layout/LayoutGrid.tsx +++ b/frontend/src/features/layout/LayoutGrid.tsx @@ -1297,7 +1297,6 @@ function LeafView({ const doLaunch = async ( opts: OpenTerminalOptions, onData: (bytes: Uint8Array) => void, - convId: string | undefined, ): Promise => { const background = backgroundLive(agentId!); if (background?.sessionId && agentGateway!.attachLiveAgent) { @@ -1313,7 +1312,7 @@ function LeafView({ .launchAgent( projectId, agentId!, - { ...opts, conversationId: convId, nodeId: id }, + { ...opts, freshStart: true, nodeId: id }, onData, ) .catch(async (err: unknown) => { @@ -1326,8 +1325,8 @@ function LeafView({ } throw err; }); - // First launch on a fresh cell mints a conversation id: persist it on the - // leaf so the next open resumes (T4b — closes the persistence loop). + // A normal cell launch always starts clean. Persist the assigned id only as + // a runtime/history pivot; it is not the next default UI selection (#371). if (handle.assignedConversationId) { void vm.setCellConversation(id, handle.assignedConversationId); } @@ -1336,7 +1335,7 @@ function LeafView({ const terminalOpener = agentGateway && agentId ? (opts: OpenTerminalOptions, onData: (bytes: Uint8Array) => void): Promise => { - return doLaunch(opts, onData, conversationId ?? undefined); + return doLaunch(opts, onData); } : undefined; // Agent cells re-attach through the agent gateway; plain cells fall back to @@ -1704,6 +1703,7 @@ function LeafView({ nodeId={id} sessionId={session} conversationId={conversationId} + freshOnOpen composerHeightPx={customChatComposerHeight} composerDefaultHeightPx={CUSTOM_CHAT_COMPOSER_DEFAULT_HEIGHT} onComposerHeightPxChange={setCustomChatComposerHeight} diff --git a/frontend/src/features/layout/setCellConversation.test.tsx b/frontend/src/features/layout/setCellConversation.test.tsx index 51f253cc..798c8cde 100644 --- a/frontend/src/features/layout/setCellConversation.test.tsx +++ b/frontend/src/features/layout/setCellConversation.test.tsx @@ -1,21 +1,50 @@ /** - * T4b — closing the conversation-id persistence loop. + * T4b / #371 — conversation-id persistence without implicit UI resume. * * Three layers, mirroring the SetCellAgent coverage: * 1. The mock agent gateway assigns a fresh conversation id on a first launch * (cell had none) and resumes (no new id) when one is passed in. * 2. `MockLayoutGateway` applies the `setCellConversation` op, persisting/clearing * the id on the leaf. - * 3. The grid wiring: when an agent cell launches and the backend assigns an id, - * `setCellConversation` is emitted so the next open resumes; and a leaf that - * already carries an id passes it back into `launchAgent`. - * - * As elsewhere (TerminalView.test), xterm may bail under jsdom, so the render-level - * assertions only fire when the opener actually ran. + * 3. The grid wiring persists an id assigned by a normal launch as a runtime / + * history pivot, but never passes a leaf's stored id into another ordinary + * launch. Only an explicit resume flow may do that. */ import { describe, it, expect, vi } from "vitest"; import { render, screen, waitFor } from "@testing-library/react"; +vi.mock("@xterm/xterm", () => ({ + Terminal: class { + loadAddon() {} + open() {} + onData() { + return { dispose() {} }; + } + write() {} + dispose() {} + get cols() { + return 80; + } + get rows() { + return 24; + } + }, +})); +vi.mock("@xterm/addon-fit", () => ({ + FitAddon: class { + fit() {} + }, +})); +vi.mock("@xterm/xterm/css/xterm.css", () => ({})); + +if (typeof globalThis.ResizeObserver === "undefined") { + globalThis.ResizeObserver = class { + observe() {} + unobserve() {} + disconnect() {} + } as unknown as typeof ResizeObserver; +} + import type { Gateways, OpenTerminalOptions, TerminalHandle } from "@/ports"; import { MockLayoutGateway, @@ -132,19 +161,14 @@ describe("LayoutGrid — conversation-id loop wiring", () => { renderGrid(layout, stubAgent, "p1"); await waitFor(() => expect(screen.getByRole("combobox")).toBeTruthy()); - - // Only assert the loop when the opener actually ran (xterm may bail in jsdom). + await waitFor(() => expect(launchAgent).toHaveBeenCalledTimes(1)); await waitFor(async () => { - if (launchAgent.mock.calls.length > 0) { - const fresh = await layout.loadLayout("p1"); - expect(leaves(fresh)[0].conversationId).toBe("assigned-conv-1"); - } else { - expect(true).toBe(true); - } + const fresh = await layout.loadLayout("p1"); + expect(leaves(fresh)[0].conversationId).toBe("assigned-conv-1"); }); }); - it("passes the leaf's existing conversation id into launchAgent (resume)", async () => { + it("does not pass the leaf's stored conversation id into an ordinary launch", async () => { const layout = new MockLayoutGateway(); const agent = new MockAgentGateway(); const a = await agent.createAgent("p1", { name: "MyAgent", profileId: "p" }); @@ -186,14 +210,9 @@ describe("LayoutGrid — conversation-id loop wiring", () => { renderGrid(layout, stubAgent, "p1"); await waitFor(() => expect(screen.getByRole("combobox")).toBeTruthy()); - - await waitFor(() => { - if (launchAgent.mock.calls.length > 0) { - const opts = launchAgent.mock.calls[0][2] as OpenTerminalOptions; - expect(opts.conversationId).toBe("prior-conv"); - } else { - expect(true).toBe(true); - } - }); + await waitFor(() => expect(launchAgent).toHaveBeenCalledTimes(1)); + const opts = launchAgent.mock.calls[0][2] as OpenTerminalOptions; + expect(opts.conversationId).toBeUndefined(); + expect(opts.freshStart).toBe(true); }); }); diff --git a/frontend/src/features/layout/useLayout.ts b/frontend/src/features/layout/useLayout.ts index f50354db..f083e10b 100644 --- a/frontend/src/features/layout/useLayout.ts +++ b/frontend/src/features/layout/useLayout.ts @@ -64,8 +64,9 @@ export interface LayoutViewModel { session: string, ) => Promise; /** - * Records (or clears) the persistent CLI conversation id on a cell (T4b). Used - * to persist the id assigned at first launch so the next open resumes it. + * Records (or clears) the persistent CLI conversation id on a cell (T4b). + * This is a runtime/history pivot; it does not select that conversation for + * the next ordinary UI launch (#371). */ setCellConversation: (target: string, conversationId: string | null) => Promise; /** Persists the preferred TUI or structured-chat surface for one agent cell. */ diff --git a/frontend/src/ports/index.ts b/frontend/src/ports/index.ts index 4fed026c..e4620419 100644 --- a/frontend/src/ports/index.ts +++ b/frontend/src/ports/index.ts @@ -357,10 +357,10 @@ export interface AgentGateway { * * The returned handle carries an optional {@link TerminalHandle.assignedConversationId}: * when the agent's profile assigns a fresh CLI conversation id on this first - * launch (the cell had none yet), it is surfaced here so the caller can persist - * it on the hosting leaf (`setCellConversation`) and resume on the next open. - * Pass the leaf's current `conversationId` via {@link OpenTerminalOptions} to - * resume an existing conversation instead. + * launch, it is surfaced here so the caller can persist it on the hosting leaf + * (`setCellConversation`) as a runtime/history pivot. Ordinary launches omit + * that persisted id and start clean; only an explicit resume action passes a + * `conversationId` via {@link OpenTerminalOptions}. */ launchAgent( projectId: string, @@ -568,10 +568,11 @@ export interface TerminalHandle { readonly sessionId: string; /** * Conversation id **assigned** by an agent launch when the profile minted a - * fresh one (the cell had none yet). Present only on handles returned by + * fresh one. Present only on handles returned by * {@link AgentGateway.launchAgent}; `undefined` for plain terminals, resumes, - * or profiles without a session block. The caller persists it on the hosting - * leaf so the next open resumes instead of re-assigning. + * or profiles without a session block. The caller may persist it on the + * hosting leaf as a runtime/history pivot; persistence alone never requests + * an implicit UI resume (#371). */ readonly assignedConversationId?: string; /** Sends bytes (xterm keystrokes) to the PTY. */