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
64 changes: 58 additions & 6 deletions apps/desktop/src/main/services/chat/agentChatService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11632,6 +11632,53 @@ describe("createAgentChatService", () => {
expect(persisted.reasoningEffort).toBe("xhigh");
});

it("applies fresh Codex thread effective sandbox when it differs from requested flags", async () => {
vi.mocked(mapPermissionToCodex).mockImplementation((mode) => {
if (mode === "default") return { approvalPolicy: "on-request", sandbox: "workspace-write" };
return { approvalPolicy: "on-request", sandbox: "read-only" };
});
mockState.codexResponseOverrides.set("thread/start", () => ({
thread: { id: "thread-effective-start-readonly" },
approvalPolicy: "onRequest",
sandbox: "read-only",
}));

const { service } = createService();
const session = await service.createSession({
laneId: "lane-1",
provider: "codex",
model: "gpt-5.4",
permissionMode: "default",
});

await service.sendMessage({
sessionId: session.id,
text: "Inspect the repo.",
});

await vi.waitFor(() => {
expect(mockState.codexRequestPayloads.some((payload) => payload.method === "turn/start")).toBe(true);
});

const threadStartRequest = mockState.codexRequestPayloads.find((payload) => payload.method === "thread/start");
const threadStartParams = threadStartRequest?.params as { approvalPolicy?: unknown; sandbox?: unknown } | undefined;
expect(threadStartParams?.approvalPolicy).toBe("on-request");
expect(threadStartParams?.sandbox).toBe("workspace-write");

const turnStartRequest = mockState.codexRequestPayloads.find((payload) => payload.method === "turn/start");
const turnStartParams = turnStartRequest?.params as {
approvalPolicy?: unknown;
sandboxPolicy?: { type?: unknown };
} | undefined;
expect(turnStartParams?.approvalPolicy).toBe("on-request");
expect(turnStartParams?.sandboxPolicy?.type).toBe("readOnly");

const summary = await service.getSessionSummary(session.id);
expect(summary?.codexApprovalPolicy).toBe("on-request");
expect(summary?.codexSandbox).toBe("read-only");
expect(summary?.permissionMode).toBe("plan");
});

it("re-resumes Codex threads when permission mode changes mid-session", async () => {
vi.mocked(mapPermissionToCodex).mockImplementation((mode) => {
if (mode === "full-auto") {
Expand Down Expand Up @@ -11675,6 +11722,11 @@ describe("createAgentChatService", () => {
});

mockState.codexRequestPayloads = [];
mockState.codexResponseOverrides.set("thread/resume", () => ({
thread: { id: "thread-after-mode-switch" },
approvalPolicy: "onRequest",
sandbox: "read-only",
}));

await service.updateSession({
sessionId: session.id,
Expand Down Expand Up @@ -12141,7 +12193,7 @@ describe("createAgentChatService", () => {
expect(sessionService.reopen).toHaveBeenCalledWith(session.id);
});

it("keeps requested Codex reasoning effort while applying effective policy on resume", async () => {
it("keeps requested Codex policy and reasoning effort across resume", async () => {
mockState.codexResponseOverrides.set("thread/resume", () => ({
thread: { id: "thread-effective-resume" },
approvalPolicy: "onFailure",
Expand Down Expand Up @@ -12175,15 +12227,15 @@ describe("createAgentChatService", () => {
expect(resumeParams?.effort).toBe("xhigh");
expect(resumeParams?.reasoningEffort).toBeUndefined();
expect(resumeParams?.reasoning_effort).toBeUndefined();
expect(resumed.codexApprovalPolicy).toBe("on-failure");
expect(resumed.codexSandbox).toBe("workspace-write");
expect(resumed.permissionMode).toBe("default");
expect(resumed.codexApprovalPolicy).toBe("never");
expect(resumed.codexSandbox).toBe("danger-full-access");
expect(resumed.permissionMode).toBe("full-auto");
expect(resumed.reasoningEffort).toBe("xhigh");

const persistedAfter = readPersistedChatState(session.id);
expect(persistedAfter.threadId).toBe("thread-effective-resume");
expect(persistedAfter.codexApprovalPolicy).toBe("on-failure");
expect(persistedAfter.codexSandbox).toBe("workspace-write");
expect(persistedAfter.codexApprovalPolicy).toBe("never");
expect(persistedAfter.codexSandbox).toBe("danger-full-access");
expect(persistedAfter.reasoningEffort).toBe("xhigh");
});

Expand Down
39 changes: 33 additions & 6 deletions apps/desktop/src/main/services/chat/agentChatService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3465,10 +3465,26 @@ function normalizeCodexRuntimeSandbox(value: unknown): AgentChatCodexSandbox | u
return typeof type === "string" ? CODEX_SANDBOX_CAMEL_CASE_ALIASES[type] : undefined;
}

function shouldPreserveRequestedCodexPolicy(
requested: { approvalPolicy: AgentChatCodexApprovalPolicy; sandbox: AgentChatCodexSandbox } | null,
runtime: { approvalPolicy?: AgentChatCodexApprovalPolicy; sandbox?: AgentChatCodexSandbox },
): boolean {
if (!requested) return false;
// Resume responses can echo stale thread policy from before ADE re-sent the
// picker/config flags. Fresh starts still adopt the runtime-reported policy.
if (runtime.sandbox && runtime.sandbox !== requested.sandbox) return true;
if (!runtime.approvalPolicy || runtime.approvalPolicy === requested.approvalPolicy) return false;
return requested.approvalPolicy === "never" || requested.approvalPolicy === "untrusted";
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.

function applyCodexEffectiveThreadState(
managed: ManagedChatSession,
response: CodexThreadLifecycleResponse | null | undefined,
options: {
requestedCodexPolicy?: {
approvalPolicy: AgentChatCodexApprovalPolicy;
sandbox: AgentChatCodexSandbox;
} | null;
requestedReasoningEffort?: string | null;
onReasoningMismatch?: (mismatch: {
requestedReasoningEffort: string;
Expand All @@ -3478,16 +3494,25 @@ function applyCodexEffectiveThreadState(
): void {
if (!response) return;

const requestedCodexPolicy = options.requestedCodexPolicy ?? null;
const approvalPolicy = normalizePersistedCodexApprovalPolicy(response.approvalPolicy);
if (approvalPolicy) {
managed.session.codexApprovalPolicy = approvalPolicy;
}

const sandbox = normalizeCodexRuntimeSandbox(response.sandbox);
if (sandbox) {
managed.session.codexSandbox = sandbox;
const runtime = {
...(approvalPolicy ? { approvalPolicy } : {}),
...(sandbox ? { sandbox } : {}),
};
const preserveRequestedPolicy = shouldPreserveRequestedCodexPolicy(requestedCodexPolicy, runtime);

if (preserveRequestedPolicy && requestedCodexPolicy) {
managed.session.codexApprovalPolicy = requestedCodexPolicy.approvalPolicy;
managed.session.codexSandbox = requestedCodexPolicy.sandbox;
} else {
if (approvalPolicy) managed.session.codexApprovalPolicy = approvalPolicy;
if (sandbox) managed.session.codexSandbox = sandbox;
}

managed.session.permissionMode = syncLegacyPermissionMode(managed.session) ?? managed.session.permissionMode;

const reasoningEffort = validateReasoningEffort(
"codex",
normalizeReasoningEffort(
Expand Down Expand Up @@ -19258,6 +19283,7 @@ export function createAgentChatService(args: {
persistExtendedHistory: true
});
applyCodexEffectiveThreadState(managed, resumeResponse, {
requestedCodexPolicy: codexPolicy,
requestedReasoningEffort: resumeReasoningEffort,
onReasoningMismatch: (mismatch) => logger.warn("agent_chat.codex_reasoning_runtime_mismatch", {
sessionId: managed.session.id,
Expand Down Expand Up @@ -20246,6 +20272,7 @@ export function createAgentChatService(args: {
persistExtendedHistory: true
});
applyCodexEffectiveThreadState(managed, resumeResponse, {
requestedCodexPolicy: codexPolicy,
requestedReasoningEffort: managed.session.reasoningEffort,
onReasoningMismatch: (mismatch) => logger.warn("agent_chat.codex_reasoning_runtime_mismatch", {
sessionId: managed.session.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1515,6 +1515,84 @@ describe("AgentChatMessageList transcript rendering", () => {
// "renders structured question blocks" tests removed: tested specific
// CSS classes and rendering details that change with UI iterations.

it("renders completed Codex plan markdown without requiring expansion", () => {
renderMessageList([
{
sessionId: "session-1",
timestamp: "2026-03-17T10:00:00.000Z",
event: {
type: "plan",
itemId: "plan-1",
turnId: "turn-1",
state: "complete",
steps: [],
streamingText: [
"# Plan",
"",
"- Inspect the app-server wiring.",
"- Patch the native plan handoff.",
].join("\n"),
},
},
]);

expect(screen.getByText("Plan")).toBeTruthy();
expect(screen.getByText("Inspect the app-server wiring.")).toBeTruthy();
expect(screen.getByText("Patch the native plan handoff.")).toBeTruthy();
});

it("does not duplicate completed Codex plan markdown when structured steps exist", () => {
renderMessageList([
{
sessionId: "session-1",
timestamp: "2026-03-17T10:00:00.000Z",
event: {
type: "plan",
itemId: "plan-1",
turnId: "turn-1",
state: "complete",
steps: [{ text: "Inspect once", status: "pending" }],
streamingText: "- Inspect once",
},
},
]);

expect(screen.getAllByText("Inspect once")).toHaveLength(1);
});

it("renders plan approval request bodies in the transcript", () => {
renderMessageList([
{
sessionId: "session-1",
timestamp: "2026-03-17T10:00:00.000Z",
event: {
type: "approval_request",
itemId: "approval-plan",
kind: "tool_call",
description: "Plan ready for approval",
turnId: "turn-1",
detail: {
request: {
requestId: "approval-plan",
itemId: "approval-plan",
source: "codex",
kind: "plan_approval",
title: "Plan Ready for Review",
description: "# Plan\n\n- Show the plan body.",
questions: [],
allowsFreeform: true,
blocking: true,
canProceedWithoutAnswer: false,
},
},
},
},
]);

expect(screen.getByText("Presenting plan for approval")).toBeTruthy();
expect(screen.getByText("Show the plan body.")).toBeTruthy();
});

it("renders structured ask-user requests inline and submits option answers", () => {
const onApproval = vi.fn();
renderMessageList([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2817,6 +2817,11 @@ function renderEvent(
</>
)}
</div>
{isPlanApproval && bodyText.trim().length > 0 ? (
<div className="mt-2 max-h-[360px] overflow-y-auto rounded-lg border border-white/[0.06] bg-black/15 px-3 py-2">
<MarkdownBlock markdown={bodyText} onOpenWorkspacePath={options?.onOpenWorkspacePath} />
</div>
) : null}
</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -708,8 +708,10 @@ function renderAutoCreateDraftPane(args?: {
) => void | Promise<void>;
workDraftKind?: "chat" | "cli";
onLaunchCliSession?: React.ComponentProps<typeof AgentChatPane>["onLaunchCliSession"];
onLaneChange?: React.ComponentProps<typeof AgentChatPane>["onLaneChange"];
lanes?: any[];
}) {
const lanes = [
const lanes = args?.lanes ?? [
{
id: "lane-primary",
name: "Primary",
Expand Down Expand Up @@ -745,7 +747,7 @@ function renderAutoCreateDraftPane(args?: {
embeddedWorkLayout
workDraftKind={args?.workDraftKind}
availableLanes={lanes}
onLaneChange={vi.fn()}
onLaneChange={args?.onLaneChange ?? vi.fn()}
onSessionCreated={args?.onSessionCreated}
onLaunchCliSession={args?.onLaunchCliSession}
/>
Expand Down Expand Up @@ -1484,6 +1486,9 @@ describe("AgentChatPane submit recovery", () => {
renderPane(session);

expect(await screen.findByRole("button", { name: "Implement" })).toBeTruthy();
expect((await screen.findAllByText("Inspect")).length).toBeGreaterThan(0);
expect(screen.getAllByText("Patch").length).toBeGreaterThan(0);
expect(screen.getAllByText("Verify").length).toBeGreaterThan(0);
const textbox = screen.getByRole("textbox") as HTMLTextAreaElement;
expect(textbox.disabled).toBe(false);

Expand Down Expand Up @@ -2843,6 +2848,77 @@ describe("AgentChatPane submit recovery", () => {
});
});

it("routes Work sidebar draft insertions into the visible draft composer", async () => {
installAdeMocks({ sessions: [] });

render(
<MemoryRouter>
<AgentChatPane
laneId="lane-1"
forceDraftMode
embeddedWorkLayout
draftContextTargetId="work:draft:lane-1:chat"
/>
</MemoryRouter>,
);

const textbox = await screen.findByRole("textbox") as HTMLTextAreaElement;
window.dispatchEvent(new CustomEvent("ade:agent-chat:insert-draft", {
detail: {
draftTargetId: "work:draft:lane-other:chat",
text: "wrong draft",
},
}));
expect(textbox.value).toBe("");

window.dispatchEvent(new CustomEvent("ade:agent-chat:insert-draft", {
detail: {
draftTargetId: "work:draft:lane-1:chat",
text: "Use the selected browser context.",
},
}));

await waitFor(() => {
expect(textbox.value).toBe("Use the selected browser context.");
});
});

it("keeps Auto-create selected while reporting Primary as the Work tools lane", async () => {
installAdeMocks({ sessions: [] });
const onLaneChange = vi.fn();
renderAutoCreateDraftPane({ onLaneChange });

fireEvent.click(await screen.findByRole("button", { name: "Select lane" }));
fireEvent.click(await screen.findByRole("button", { name: /Auto-create lane/i }));

expect(onLaneChange).toHaveBeenCalledWith("lane-primary");
expect(await screen.findByText("Auto-create lane")).toBeTruthy();
expect(await screen.findByText("Tools use Primary until the lane is created.")).toBeTruthy();
});

it("falls back to the first available Work tools lane when Auto-create has no Primary lane", async () => {
installAdeMocks({ sessions: [] });
const onLaneChange = vi.fn();
renderAutoCreateDraftPane({
onLaneChange,
lanes: [
{
id: "lane-worktree",
name: "current-lane",
laneType: "worktree",
branchRef: "refs/heads/current-lane",
worktreePath: "/tmp/project-under-test/current-lane",
},
],
});

fireEvent.click(await screen.findByRole("button", { name: "Select lane" }));
fireEvent.click(await screen.findByRole("button", { name: /Auto-create lane/i }));

expect(onLaneChange).toHaveBeenCalledWith("lane-worktree");
expect(await screen.findByText("Tools use current-lane until the lane is created.")).toBeTruthy();
});

it("background auto-create reports the new chat without stealing focus and shows a dismissible notice", async () => {
const onSessionCreated = vi.fn();
const { createLane, suggestLaneName } = installAdeMocks({ sessions: [] });
Expand Down
Loading
Loading