Keep approvals with the user in every Codex mode but Auto-review (#5239)

A Codex mode preset named its approvals reviewer only for Auto-review.
Every other mode left the field off the thread/start and turn/start
request, so the Codex app-server fell back to `approvals_reviewer` from
the user's global config, or to whatever reviewer the live thread was
already using. Default Permissions and Read-only then behaved like
Auto-review: escalations were approved by Codex, with no approval card
and no notification.

The reviewer belongs to the Paseo mode, so the preset now always names
it and the request always carries it. An agent with no Paseo mode still
inherits the Codex configuration, unchanged.

Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com>
Co-authored-by: HMWCS <121323913+HMWCS@users.noreply.github.com>
This commit is contained in:
paseo-bot[bot]
2026-09-23 05:46:12 +02:00
committed by GitHub
co-authored by HMWCS
parent 5303313632
commit 8269a0ea78
2 changed files with 95 additions and 12 deletions
@@ -744,6 +744,94 @@ describe("Codex app-server provider", () => {
});
});
test("Default Permissions pins the user as approvals reviewer on thread/start", async () => {
const requests: Array<{ method: string; params: unknown }> = [];
const session = createSession(
{ modeId: "auto", thinkingOptionId: "medium" },
{ autoReviewEnabled: true },
);
session.currentThreadId = null;
session.activeForegroundTurnId = null;
session.client = {
request: vi.fn(async (method: string, params: unknown) => {
requests.push({ method, params });
if (method === "thread/start") {
return { thread: { id: "default-mode-thread" } };
}
if (method === "turn/start") {
return {};
}
throw new Error(`Unexpected request: ${method}`);
}),
};
await session.startTurn("trigger thread creation");
const startCall = requests.find((req) => req.method === "thread/start");
expect(startCall?.params).toMatchObject({
approvalPolicy: "on-request",
sandbox: "workspace-write",
approvalsReviewer: "user",
});
});
test("switching from auto-review back to Default returns approvals to the user", async () => {
const session = createSession({ modeId: "auto-review" }, { autoReviewEnabled: true });
const request = vi.fn(async (method: string) => {
if (method === "thread/loaded/list") {
return { data: ["test-thread"] };
}
if (method === "turn/start") {
return {};
}
throw new Error(`Unexpected request: ${method}`);
});
session.activeForegroundTurnId = null;
session.client = createStub<CodexClientLike>({ request });
await session.setMode("auto");
await session.startTurn("needs approval");
const turnStartCall = request.mock.calls.find(([method]) => method === "turn/start");
expect(turnStartCall?.[1]).toEqual(
expect.objectContaining({
approvalPolicy: "on-request",
approvalsReviewer: "user",
}),
);
});
test("Read-only pins the user as approvals reviewer on thread/start", async () => {
const requests: Array<{ method: string; params: unknown }> = [];
const session = createSession(
{ modeId: "read-only", thinkingOptionId: "medium" },
{ autoReviewEnabled: true },
);
session.currentThreadId = null;
session.activeForegroundTurnId = null;
session.client = {
request: vi.fn(async (method: string, params: unknown) => {
requests.push({ method, params });
if (method === "thread/start") {
return { thread: { id: "read-only-thread" } };
}
if (method === "turn/start") {
return {};
}
throw new Error(`Unexpected request: ${method}`);
}),
};
await session.startTurn("trigger thread creation");
const startCall = requests.find((req) => req.method === "thread/start");
expect(startCall?.params).toMatchObject({
approvalPolicy: "on-request",
sandbox: "read-only",
approvalsReviewer: "user",
});
});
test("setMode and setThinkingOption return a next-turn notice while a turn is active", async () => {
const session = createSession({ modeId: "auto", thinkingOptionId: "medium" });
@@ -834,6 +922,7 @@ describe("Codex app-server provider", () => {
const turnStart = request.mock.calls.find(([method]) => method === "turn/start")?.[1];
expect(turnStart).not.toHaveProperty("approvalPolicy");
expect(turnStart).not.toHaveProperty("sandboxPolicy");
expect(turnStart).not.toHaveProperty("approvalsReviewer");
});
test("carries the complete native workspace-write policy including writable roots", async () => {
@@ -273,17 +273,19 @@ interface CodexAppServerAgentDeps {
interface CodexModePreset {
approvalPolicy: string;
sandbox: string;
approvalsReviewer?: "auto_review";
approvalsReviewer: "user" | "auto_review";
}
const MODE_PRESETS: Record<string, CodexModePreset> = {
"read-only": {
approvalPolicy: "on-request",
sandbox: "read-only",
approvalsReviewer: "user",
},
auto: {
approvalPolicy: "on-request",
sandbox: "workspace-write",
approvalsReviewer: "user",
},
"auto-review": {
approvalPolicy: "on-request",
@@ -293,6 +295,7 @@ const MODE_PRESETS: Record<string, CodexModePreset> = {
"full-access": {
approvalPolicy: "never",
sandbox: "danger-full-access",
approvalsReviewer: "user",
},
};
@@ -300,15 +303,6 @@ function isAutoReviewReviewer(value: string | undefined): boolean {
return value === "auto_review" || value === "guardian_subagent";
}
function applyApprovalsReviewerParam(
params: Record<string, unknown>,
preset: CodexModePreset,
): void {
if (preset.approvalsReviewer) {
params.approvalsReviewer = preset.approvalsReviewer;
}
}
function shouldPromoteThreadResponseToAutoReview(params: {
approvalsReviewer: string | undefined;
approvalPolicy: string;
@@ -4151,7 +4145,7 @@ export class CodexAppServerAgentSession implements AgentSession {
: toSandboxPolicy(sandboxPolicyType, workspaceWrite);
}
if (this.hasWorkflowModeOverride) {
applyApprovalsReviewerParam(params, preset);
params.approvalsReviewer = preset.approvalsReviewer;
}
return { approvalPolicy, sandboxPolicyType };
}
@@ -5205,7 +5199,7 @@ export class CodexAppServerAgentSession implements AgentSession {
...(this.ephemeral ? { ephemeral: true } : {}),
};
if (this.hasWorkflowModeOverride) {
applyApprovalsReviewerParam(params, preset);
params.approvalsReviewer = preset.approvalsReviewer;
}
return { params, approvalPolicy, sandbox };
}