Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1850,6 +1850,7 @@ function MainApp() {
connectWorkspace,
sendUserMessageToThread,
setSelectedCollaborationModeId,
persistThreadCodexParams,
});

const { handleMoveWorkspace } = useWorkspaceOrderingOrchestration({
Expand Down
260 changes: 260 additions & 0 deletions src/features/app/hooks/usePlanReadyActions.test.tsx
Original file line number Diff line number Diff line change
@@ -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 },
);
});

Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a test case for when the collaborationModes array is completely empty. While the current test covers the "only plan mode" scenario, an empty array is another edge case that should behave identically (returning null for implementationMode). This would provide more comprehensive coverage of the fallback logic in findImplementationMode.

Suggested change
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 },
);
});

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 9d5c8e7. Added explicit coverage for an empty collaboration mode list in handlePlanAccept, asserting neutral override (collaborationMode: null) and persisted collaborationModeId: 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",
},
},
},
);
});
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a test case for when plan mode is not available when calling handlePlanSubmitChanges. While the code handles this gracefully by setting the mode to null (line 158), having explicit test coverage would document this edge case behavior and ensure it continues to work as expected.

Suggested change
});
});
it("handles plan-change follow-up when plan mode is not available", 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,
},
);
});

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 9d5c8e7 with a safety adjustment: when handlePlanSubmitChanges cannot find plan mode, we now keep selected/persisted mode untouched and still send explicit neutral override (collaborationMode: null). Added regression test coverage for this path.


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 },
);
});
});
55 changes: 40 additions & 15 deletions src/features/app/hooks/usePlanReadyActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ type UsePlanReadyActionsOptions = {
resolvedEffort: string | null;
connectWorkspace: (workspace: WorkspaceInfo) => Promise<void>;
sendUserMessageToThread: SendUserMessageToThread;
setSelectedCollaborationModeId: (modeId: string) => void;
setSelectedCollaborationModeId: (modeId: string | null) => void;
persistThreadCodexParams: (patch: { collaborationModeId?: string | null }) => void;
};

export function usePlanReadyActions({
Expand All @@ -37,6 +38,7 @@ export function usePlanReadyActions({
connectWorkspace,
sendUserMessageToThread,
setSelectedCollaborationModeId,
persistThreadCodexParams,
}: UsePlanReadyActionsOptions) {
const findCollaborationMode = useCallback(
(wanted: string) => {
Expand All @@ -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) {
Expand Down Expand Up @@ -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({

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist plan mode against the triggering thread

Calling persistThreadCodexParams here can write the mode to the wrong thread when the workspace is disconnected: handlePlanAccept awaits connectWorkspace, and during that await the user can switch threads, after which persistThreadCodexParams resolves the target from activeThreadIdRef.current (useThreadCodexOrchestration.ts), while sendUserMessageToThread still uses the originally captured activeThreadId. That mismatch causes thread-scoped collaborationModeId to be saved on a different thread than the one receiving the plan-followup message; the same pattern appears in handlePlanSubmitChanges.

Useful? React with 👍 / 👎.

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,
]);
Expand All @@ -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);
Expand All @@ -144,7 +168,7 @@ export function usePlanReadyActions({
activeThreadId,
message,
[],
collaborationMode ? { collaborationMode } : undefined,
{ collaborationMode: collaborationMode ?? null },
);
},
[
Expand All @@ -153,6 +177,7 @@ export function usePlanReadyActions({
buildCollaborationModePayloadFor,
connectWorkspace,
findCollaborationMode,
persistThreadCodexParams,
sendUserMessageToThread,
setSelectedCollaborationModeId,
],
Expand Down
Loading