diff --git a/src/App.tsx b/src/App.tsx index ac8dddf4..717538b9 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1850,6 +1850,7 @@ function MainApp() { connectWorkspace, sendUserMessageToThread, setSelectedCollaborationModeId, + persistThreadCodexParams, }); const { handleMoveWorkspace } = useWorkspaceOrderingOrchestration({ diff --git a/src/features/app/hooks/usePlanReadyActions.test.tsx b/src/features/app/hooks/usePlanReadyActions.test.tsx new file mode 100644 index 00000000..9155f511 --- /dev/null +++ b/src/features/app/hooks/usePlanReadyActions.test.tsx @@ -0,0 +1,260 @@ +// @vitest-environment jsdom +import { act, renderHook } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import type { CollaborationModeOption, WorkspaceInfo } from "@/types"; +import { usePlanReadyActions } from "@app/hooks/usePlanReadyActions"; + +function makeMode(id: string): CollaborationModeOption { + return { + id, + label: id, + mode: id, + model: "", + reasoningEffort: null, + developerInstructions: null, + value: {}, + }; +} + +const connectedWorkspace: WorkspaceInfo = { + id: "ws-1", + name: "Workspace", + path: "/tmp/workspace", + connected: true, + settings: { sidebarCollapsed: false }, +}; + +const disconnectedWorkspace: WorkspaceInfo = { + ...connectedWorkspace, + connected: false, +}; + +function renderPlanReadyActions(overrides?: { + activeWorkspace?: WorkspaceInfo | null; + collaborationModes?: CollaborationModeOption[]; + resolvedModel?: string | null; + resolvedEffort?: string | null; +}) { + const connectWorkspace = vi.fn().mockResolvedValue(undefined); + const sendUserMessageToThread = vi.fn().mockResolvedValue(undefined); + const setSelectedCollaborationModeId = vi.fn(); + const persistThreadCodexParams = vi.fn(); + + const options = { + activeWorkspace: connectedWorkspace as WorkspaceInfo | null, + activeThreadId: "thread-1" as string | null, + collaborationModes: [makeMode("plan"), makeMode("default"), makeMode("code")], + resolvedModel: "gpt-5.2" as string | null, + resolvedEffort: "high" as string | null, + ...overrides, + connectWorkspace, + sendUserMessageToThread, + setSelectedCollaborationModeId, + persistThreadCodexParams, + }; + + const hook = renderHook(() => usePlanReadyActions(options)); + return { + ...hook, + connectWorkspace, + sendUserMessageToThread, + setSelectedCollaborationModeId, + persistThreadCodexParams, + }; +} + +describe("usePlanReadyActions", () => { + it("uses default mode when implementing a plan and persists selection", async () => { + const { + result, + setSelectedCollaborationModeId, + persistThreadCodexParams, + sendUserMessageToThread, + } = renderPlanReadyActions(); + + await act(async () => { + await result.current.handlePlanAccept(); + }); + + expect(setSelectedCollaborationModeId).toHaveBeenCalledWith("default"); + expect(persistThreadCodexParams).toHaveBeenCalledWith({ + collaborationModeId: "default", + }); + expect(sendUserMessageToThread).toHaveBeenCalledWith( + connectedWorkspace, + "thread-1", + "[[cm_plan_ready:accept]] Implement this plan.", + [], + { + collaborationMode: { + mode: "default", + settings: { + developer_instructions: null, + model: "gpt-5.2", + reasoning_effort: "high", + }, + }, + }, + ); + }); + + it("falls back to first non-plan mode when default/code are unavailable", async () => { + const { + result, + setSelectedCollaborationModeId, + persistThreadCodexParams, + sendUserMessageToThread, + } = renderPlanReadyActions({ + collaborationModes: [makeMode("plan"), makeMode("review")], + }); + + await act(async () => { + await result.current.handlePlanAccept(); + }); + + expect(setSelectedCollaborationModeId).toHaveBeenCalledWith("review"); + expect(persistThreadCodexParams).toHaveBeenCalledWith({ + collaborationModeId: "review", + }); + expect(sendUserMessageToThread).toHaveBeenCalledWith( + connectedWorkspace, + "thread-1", + "[[cm_plan_ready:accept]] Implement this plan.", + [], + expect.objectContaining({ + collaborationMode: expect.objectContaining({ + mode: "review", + }), + }), + ); + }); + + it("forces neutral mode override when only plan mode is available", async () => { + const { + result, + setSelectedCollaborationModeId, + persistThreadCodexParams, + sendUserMessageToThread, + } = renderPlanReadyActions({ + collaborationModes: [makeMode("plan")], + }); + + await act(async () => { + await result.current.handlePlanAccept(); + }); + + expect(setSelectedCollaborationModeId).toHaveBeenCalledWith(null); + expect(persistThreadCodexParams).toHaveBeenCalledWith({ + collaborationModeId: null, + }); + expect(sendUserMessageToThread).toHaveBeenCalledWith( + connectedWorkspace, + "thread-1", + "[[cm_plan_ready:accept]] Implement this plan.", + [], + { collaborationMode: null }, + ); + }); + + it("forces neutral mode override when no collaboration modes are available", async () => { + const { + result, + setSelectedCollaborationModeId, + persistThreadCodexParams, + sendUserMessageToThread, + } = renderPlanReadyActions({ + collaborationModes: [], + }); + + await act(async () => { + await result.current.handlePlanAccept(); + }); + + expect(setSelectedCollaborationModeId).toHaveBeenCalledWith(null); + expect(persistThreadCodexParams).toHaveBeenCalledWith({ + collaborationModeId: null, + }); + expect(sendUserMessageToThread).toHaveBeenCalledWith( + connectedWorkspace, + "thread-1", + "[[cm_plan_ready:accept]] Implement this plan.", + [], + { collaborationMode: null }, + ); + }); + + it("connects workspace before sending plan accept message", async () => { + const { result, connectWorkspace, sendUserMessageToThread } = + renderPlanReadyActions({ + activeWorkspace: disconnectedWorkspace, + }); + + await act(async () => { + await result.current.handlePlanAccept(); + }); + + expect(connectWorkspace).toHaveBeenCalledWith(disconnectedWorkspace); + expect(sendUserMessageToThread).toHaveBeenCalledTimes(1); + }); + + it("keeps plan mode for plan-change follow-up and persists it", async () => { + const { + result, + setSelectedCollaborationModeId, + persistThreadCodexParams, + sendUserMessageToThread, + } = renderPlanReadyActions({ + collaborationModes: [makeMode("default"), makeMode("plan")], + }); + + await act(async () => { + await result.current.handlePlanSubmitChanges(" Add tests "); + }); + + expect(setSelectedCollaborationModeId).toHaveBeenCalledWith("plan"); + expect(persistThreadCodexParams).toHaveBeenCalledWith({ + collaborationModeId: "plan", + }); + expect(sendUserMessageToThread).toHaveBeenCalledWith( + connectedWorkspace, + "thread-1", + "[[cm_plan_ready:changes]] Update the plan with these changes:\n\nAdd tests", + [], + { + collaborationMode: { + mode: "plan", + settings: { + developer_instructions: null, + model: "gpt-5.2", + reasoning_effort: "high", + }, + }, + }, + ); + }); + + it("uses neutral mode override for plan-change follow-up when plan mode is unavailable", async () => { + const { + result, + setSelectedCollaborationModeId, + persistThreadCodexParams, + sendUserMessageToThread, + } = renderPlanReadyActions({ + collaborationModes: [makeMode("default")], + }); + + await act(async () => { + await result.current.handlePlanSubmitChanges(" Add tests "); + }); + + expect(setSelectedCollaborationModeId).not.toHaveBeenCalled(); + expect(persistThreadCodexParams).not.toHaveBeenCalled(); + expect(sendUserMessageToThread).toHaveBeenCalledWith( + connectedWorkspace, + "thread-1", + "[[cm_plan_ready:changes]] Update the plan with these changes:\n\nAdd tests", + [], + { collaborationMode: null }, + ); + }); +}); diff --git a/src/features/app/hooks/usePlanReadyActions.ts b/src/features/app/hooks/usePlanReadyActions.ts index e9c028ed..a54e6163 100644 --- a/src/features/app/hooks/usePlanReadyActions.ts +++ b/src/features/app/hooks/usePlanReadyActions.ts @@ -25,7 +25,8 @@ type UsePlanReadyActionsOptions = { resolvedEffort: string | null; connectWorkspace: (workspace: WorkspaceInfo) => Promise; sendUserMessageToThread: SendUserMessageToThread; - setSelectedCollaborationModeId: (modeId: string) => void; + setSelectedCollaborationModeId: (modeId: string | null) => void; + persistThreadCodexParams: (patch: { collaborationModeId?: string | null }) => void; }; export function usePlanReadyActions({ @@ -37,6 +38,7 @@ export function usePlanReadyActions({ connectWorkspace, sendUserMessageToThread, setSelectedCollaborationModeId, + persistThreadCodexParams, }: UsePlanReadyActionsOptions) { const findCollaborationMode = useCallback( (wanted: string) => { @@ -57,6 +59,28 @@ export function usePlanReadyActions({ [collaborationModes], ); + const isPlanMode = useCallback((mode: CollaborationModeOption | null) => { + if (!mode) { + return false; + } + const modeValue = (mode.mode || mode.id).trim().toLowerCase(); + return modeValue === "plan"; + }, []); + + const findImplementationMode = useCallback(() => { + const defaultMode = findCollaborationMode("default"); + if (defaultMode && !isPlanMode(defaultMode)) { + return defaultMode; + } + + const codeMode = findCollaborationMode("code"); + if (codeMode && !isPlanMode(codeMode)) { + return codeMode; + } + + return collaborationModes.find((mode) => !isPlanMode(mode)) ?? null; + }, [collaborationModes, findCollaborationMode, isPlanMode]); + const buildCollaborationModePayloadFor = useCallback( (mode: CollaborationModeOption | null) => { if (!mode) { @@ -93,31 +117,28 @@ export function usePlanReadyActions({ await connectWorkspace(activeWorkspace); } - const defaultMode = - findCollaborationMode("default") ?? - findCollaborationMode("code") ?? - collaborationModes[0] ?? - null; - - if (defaultMode?.id) { - setSelectedCollaborationModeId(defaultMode.id); - } + const implementationMode = findImplementationMode(); + const implementationModeId = implementationMode?.id ?? null; + setSelectedCollaborationModeId(implementationModeId); + persistThreadCodexParams({ + collaborationModeId: implementationModeId, + }); - const collaborationMode = buildCollaborationModePayloadFor(defaultMode); + const collaborationMode = buildCollaborationModePayloadFor(implementationMode); await sendUserMessageToThread( activeWorkspace, activeThreadId, makePlanReadyAcceptMessage(), [], - collaborationMode ? { collaborationMode } : undefined, + { collaborationMode: collaborationMode ?? null }, ); }, [ activeThreadId, activeWorkspace, buildCollaborationModePayloadFor, - collaborationModes, connectWorkspace, - findCollaborationMode, + findImplementationMode, + persistThreadCodexParams, sendUserMessageToThread, setSelectedCollaborationModeId, ]); @@ -136,6 +157,9 @@ export function usePlanReadyActions({ const planMode = findCollaborationMode("plan"); if (planMode?.id) { setSelectedCollaborationModeId(planMode.id); + persistThreadCodexParams({ + collaborationModeId: planMode.id, + }); } const collaborationMode = buildCollaborationModePayloadFor(planMode); const message = makePlanReadyChangesMessage(trimmed); @@ -144,7 +168,7 @@ export function usePlanReadyActions({ activeThreadId, message, [], - collaborationMode ? { collaborationMode } : undefined, + { collaborationMode: collaborationMode ?? null }, ); }, [ @@ -153,6 +177,7 @@ export function usePlanReadyActions({ buildCollaborationModePayloadFor, connectWorkspace, findCollaborationMode, + persistThreadCodexParams, sendUserMessageToThread, setSelectedCollaborationModeId, ],