feat(codex): implement Auto-review permission mode (#963)
* feat(codex): plumb auto-review approvals reviewer * feat(codex): add Auto-review mode visuals * test(codex): cover Auto-review mode plumbing * test(codex): prove Auto-review approval routing
This commit is contained in:
@@ -27,6 +27,7 @@ import {
|
|||||||
ShieldAlert,
|
ShieldAlert,
|
||||||
ShieldCheck,
|
ShieldCheck,
|
||||||
ShieldOff,
|
ShieldOff,
|
||||||
|
ShieldQuestionMark,
|
||||||
Zap,
|
Zap,
|
||||||
} from "lucide-react-native";
|
} from "lucide-react-native";
|
||||||
import { getProviderIcon } from "@/components/provider-icons";
|
import { getProviderIcon } from "@/components/provider-icons";
|
||||||
@@ -180,6 +181,7 @@ const MODE_ICONS = {
|
|||||||
ShieldCheck,
|
ShieldCheck,
|
||||||
ShieldAlert,
|
ShieldAlert,
|
||||||
ShieldOff,
|
ShieldOff,
|
||||||
|
ShieldQuestionMark,
|
||||||
} as const;
|
} as const;
|
||||||
|
|
||||||
function alwaysTrue() {
|
function alwaysTrue() {
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ import { z } from "zod";
|
|||||||
import type { AgentMode } from "./agent-sdk-types.js";
|
import type { AgentMode } from "./agent-sdk-types.js";
|
||||||
|
|
||||||
export type AgentModeColorTier = "safe" | "moderate" | "dangerous" | "planning";
|
export type AgentModeColorTier = "safe" | "moderate" | "dangerous" | "planning";
|
||||||
export type AgentModeIcon = "ShieldCheck" | "ShieldAlert" | "ShieldOff";
|
export type AgentModeIcon = "ShieldCheck" | "ShieldAlert" | "ShieldOff" | "ShieldQuestionMark";
|
||||||
|
|
||||||
export interface AgentModeVisuals {
|
export interface AgentModeVisuals {
|
||||||
icon: AgentModeIcon;
|
icon: AgentModeIcon;
|
||||||
@@ -71,6 +71,14 @@ const CODEX_MODES: AgentProviderModeDefinition[] = [
|
|||||||
icon: "ShieldAlert",
|
icon: "ShieldAlert",
|
||||||
colorTier: "moderate",
|
colorTier: "moderate",
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
id: "auto-review",
|
||||||
|
label: "Auto-review",
|
||||||
|
description:
|
||||||
|
"Same workspace-write permissions as Default, but eligible `on-request` approvals are routed through the auto-reviewer subagent.",
|
||||||
|
icon: "ShieldQuestionMark",
|
||||||
|
colorTier: "moderate",
|
||||||
|
},
|
||||||
{
|
{
|
||||||
id: "full-access",
|
id: "full-access",
|
||||||
label: "Full Access",
|
label: "Full Access",
|
||||||
|
|||||||
@@ -73,7 +73,7 @@ function createConfig(overrides: Partial<AgentSessionConfig> = {}): AgentSession
|
|||||||
|
|
||||||
function createSession(
|
function createSession(
|
||||||
configOverrides: Partial<AgentSessionConfig> = {},
|
configOverrides: Partial<AgentSessionConfig> = {},
|
||||||
options: { goalsEnabled?: boolean } = {},
|
options: { goalsEnabled?: boolean; autoReviewEnabled?: boolean } = {},
|
||||||
): CodexTestSession {
|
): CodexTestSession {
|
||||||
const session = new __codexAppServerInternals.CodexAppServerAgentSession(
|
const session = new __codexAppServerInternals.CodexAppServerAgentSession(
|
||||||
createConfig(configOverrides),
|
createConfig(configOverrides),
|
||||||
@@ -85,6 +85,7 @@ function createSession(
|
|||||||
{},
|
{},
|
||||||
false,
|
false,
|
||||||
options.goalsEnabled === true,
|
options.goalsEnabled === true,
|
||||||
|
options.autoReviewEnabled === true,
|
||||||
) as CodexTestSession;
|
) as CodexTestSession;
|
||||||
session.connected = true;
|
session.connected = true;
|
||||||
session.currentThreadId = "test-thread";
|
session.currentThreadId = "test-thread";
|
||||||
@@ -199,6 +200,116 @@ function capturedThreadStartConfig(records: CapturedFakeCodexRecord[]): unknown
|
|||||||
}
|
}
|
||||||
|
|
||||||
describe("Codex app-server provider", () => {
|
describe("Codex app-server provider", () => {
|
||||||
|
test("getAvailableModes includes auto-review when the Codex version supports it", async () => {
|
||||||
|
const session = createSession({}, { autoReviewEnabled: true });
|
||||||
|
|
||||||
|
await expect(session.getAvailableModes()).resolves.toEqual(
|
||||||
|
expect.arrayContaining([
|
||||||
|
expect.objectContaining({
|
||||||
|
id: "auto-review",
|
||||||
|
label: "Auto-review",
|
||||||
|
}),
|
||||||
|
]),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("getAvailableModes excludes auto-review when the Codex version is too old", async () => {
|
||||||
|
const session = createSession({}, { autoReviewEnabled: false });
|
||||||
|
|
||||||
|
await expect(session.getAvailableModes()).resolves.not.toEqual(
|
||||||
|
expect.arrayContaining([expect.objectContaining({ id: "auto-review" })]),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("setMode auto-review sends approvalsReviewer to 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: "auto-review-thread" } };
|
||||||
|
}
|
||||||
|
if (method === "turn/start") {
|
||||||
|
return {};
|
||||||
|
}
|
||||||
|
throw new Error(`Unexpected request: ${method}`);
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
|
||||||
|
await session.setMode("auto-review");
|
||||||
|
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: "auto_review",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test.each(["auto_review", "guardian_subagent"])(
|
||||||
|
"parses %s thread/start response as auto-review mode",
|
||||||
|
async (approvalsReviewer) => {
|
||||||
|
const session = createSession(
|
||||||
|
{ modeId: "auto", thinkingOptionId: "medium" },
|
||||||
|
{ autoReviewEnabled: true },
|
||||||
|
);
|
||||||
|
session.currentThreadId = null;
|
||||||
|
session.activeForegroundTurnId = null;
|
||||||
|
session.client = {
|
||||||
|
request: vi.fn(async (method: string) => {
|
||||||
|
if (method === "thread/start") {
|
||||||
|
return {
|
||||||
|
thread: { id: "auto-review-thread" },
|
||||||
|
approvalPolicy: "on-request",
|
||||||
|
sandbox: { type: "workspaceWrite", networkAccess: false },
|
||||||
|
approvalsReviewer,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
if (method === "turn/start") {
|
||||||
|
return {};
|
||||||
|
}
|
||||||
|
throw new Error(`Unexpected request: ${method}`);
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
|
||||||
|
await session.startTurn("trigger thread creation");
|
||||||
|
|
||||||
|
await expect(session.getCurrentMode()).resolves.toBe("auto-review");
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
test("turn/start forwards approvalsReviewer while in auto-review mode", 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.startTurn("needs approval");
|
||||||
|
|
||||||
|
const turnStartCall = request.mock.calls.find(([method]) => method === "turn/start");
|
||||||
|
expect(turnStartCall?.[1]).toEqual(
|
||||||
|
expect.objectContaining({
|
||||||
|
approvalPolicy: "on-request",
|
||||||
|
approvalsReviewer: "auto_review",
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
test("passes ephemeral: true to thread/start when constructed as ephemeral", async () => {
|
test("passes ephemeral: true to thread/start when constructed as ephemeral", async () => {
|
||||||
const requests: Array<{ method: string; params: unknown }> = [];
|
const requests: Array<{ method: string; params: unknown }> = [];
|
||||||
const fakeClient: CodexClientLike = {
|
const fakeClient: CodexClientLike = {
|
||||||
|
|||||||
@@ -105,6 +105,7 @@ const CODEX_PLAN_IMPLEMENTATION_PROMPT_PREFIX =
|
|||||||
// `--enable goals` at launch, so we gate by version and silently skip the flag
|
// `--enable goals` at launch, so we gate by version and silently skip the flag
|
||||||
// (and the /goal slash command) when the binary is too old.
|
// (and the /goal slash command) when the binary is too old.
|
||||||
const CODEX_GOALS_MIN_VERSION: readonly [number, number, number] = [0, 128, 0];
|
const CODEX_GOALS_MIN_VERSION: readonly [number, number, number] = [0, 128, 0];
|
||||||
|
const CODEX_AUTO_REVIEW_MIN_VERSION: readonly [number, number, number] = [0, 115, 0];
|
||||||
|
|
||||||
function parseCodexVersion(versionOutput: string): [number, number, number] | null {
|
function parseCodexVersion(versionOutput: string): [number, number, number] | null {
|
||||||
const match = versionOutput.match(/(\d+)\.(\d+)\.(\d+)/);
|
const match = versionOutput.match(/(\d+)\.(\d+)\.(\d+)/);
|
||||||
@@ -161,6 +162,12 @@ const CODEX_MODES: AgentMode[] = [
|
|||||||
label: "Default Permissions",
|
label: "Default Permissions",
|
||||||
description: "Edit files and run commands with Codex's default approval flow.",
|
description: "Edit files and run commands with Codex's default approval flow.",
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
id: "auto-review",
|
||||||
|
label: "Auto-review",
|
||||||
|
description:
|
||||||
|
"Same workspace-write permissions as Default, but eligible `on-request` approvals are routed through the auto-reviewer subagent.",
|
||||||
|
},
|
||||||
{
|
{
|
||||||
id: "full-access",
|
id: "full-access",
|
||||||
label: "Full Access",
|
label: "Full Access",
|
||||||
@@ -191,10 +198,14 @@ interface CodexAppServerAgentDeps {
|
|||||||
) => CodexAppServerClientLike;
|
) => CodexAppServerClientLike;
|
||||||
}
|
}
|
||||||
|
|
||||||
const MODE_PRESETS: Record<
|
interface CodexModePreset {
|
||||||
string,
|
approvalPolicy: string;
|
||||||
{ approvalPolicy: string; sandbox: string; networkAccess?: boolean }
|
sandbox: string;
|
||||||
> = {
|
networkAccess?: boolean;
|
||||||
|
approvalsReviewer?: "auto_review";
|
||||||
|
}
|
||||||
|
|
||||||
|
const MODE_PRESETS: Record<string, CodexModePreset> = {
|
||||||
"read-only": {
|
"read-only": {
|
||||||
approvalPolicy: "on-request",
|
approvalPolicy: "on-request",
|
||||||
sandbox: "read-only",
|
sandbox: "read-only",
|
||||||
@@ -203,6 +214,11 @@ const MODE_PRESETS: Record<
|
|||||||
approvalPolicy: "on-request",
|
approvalPolicy: "on-request",
|
||||||
sandbox: "workspace-write",
|
sandbox: "workspace-write",
|
||||||
},
|
},
|
||||||
|
"auto-review": {
|
||||||
|
approvalPolicy: "on-request",
|
||||||
|
sandbox: "workspace-write",
|
||||||
|
approvalsReviewer: "auto_review",
|
||||||
|
},
|
||||||
"full-access": {
|
"full-access": {
|
||||||
approvalPolicy: "never",
|
approvalPolicy: "never",
|
||||||
sandbox: "danger-full-access",
|
sandbox: "danger-full-access",
|
||||||
@@ -210,6 +226,31 @@ const MODE_PRESETS: Record<
|
|||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|
||||||
|
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;
|
||||||
|
sandbox: string;
|
||||||
|
}): boolean {
|
||||||
|
return (
|
||||||
|
isAutoReviewReviewer(params.approvalsReviewer) &&
|
||||||
|
params.approvalPolicy === "on-request" &&
|
||||||
|
params.sandbox === "workspace-write"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
function validateCodexMode(modeId: string): void {
|
function validateCodexMode(modeId: string): void {
|
||||||
if (!(modeId in MODE_PRESETS)) {
|
if (!(modeId in MODE_PRESETS)) {
|
||||||
const validModes = Object.keys(MODE_PRESETS).join(", ");
|
const validModes = Object.keys(MODE_PRESETS).join(", ");
|
||||||
@@ -2658,6 +2699,7 @@ class CodexAppServerAgentSession implements AgentSession {
|
|||||||
private readonly deps: CodexAppServerAgentDeps = {},
|
private readonly deps: CodexAppServerAgentDeps = {},
|
||||||
private readonly ephemeral: boolean = false,
|
private readonly ephemeral: boolean = false,
|
||||||
private readonly goalsEnabled: boolean = false,
|
private readonly goalsEnabled: boolean = false,
|
||||||
|
private readonly autoReviewEnabled: boolean = false,
|
||||||
private readonly agentId?: string,
|
private readonly agentId?: string,
|
||||||
) {
|
) {
|
||||||
this.logger = logger.child({
|
this.logger = logger.child({
|
||||||
@@ -3126,6 +3168,7 @@ class CodexAppServerAgentSession implements AgentSession {
|
|||||||
: preset.networkAccess,
|
: preset.networkAccess,
|
||||||
),
|
),
|
||||||
};
|
};
|
||||||
|
applyApprovalsReviewerParam(params, preset);
|
||||||
|
|
||||||
if (this.config.model) {
|
if (this.config.model) {
|
||||||
params.model = this.config.model;
|
params.model = this.config.model;
|
||||||
@@ -3212,7 +3255,10 @@ class CodexAppServerAgentSession implements AgentSession {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async getAvailableModes(): Promise<AgentMode[]> {
|
async getAvailableModes(): Promise<AgentMode[]> {
|
||||||
return CODEX_MODES;
|
if (this.autoReviewEnabled) {
|
||||||
|
return CODEX_MODES;
|
||||||
|
}
|
||||||
|
return CODEX_MODES.filter((mode) => mode.id !== "auto-review");
|
||||||
}
|
}
|
||||||
|
|
||||||
async getCurrentMode(): Promise<string | null> {
|
async getCurrentMode(): Promise<string | null> {
|
||||||
@@ -3642,7 +3688,7 @@ class CodexAppServerAgentSession implements AgentSession {
|
|||||||
const approvalPolicy = this.config.approvalPolicy ?? preset.approvalPolicy;
|
const approvalPolicy = this.config.approvalPolicy ?? preset.approvalPolicy;
|
||||||
const sandbox = this.config.sandboxMode ?? preset.sandbox;
|
const sandbox = this.config.sandboxMode ?? preset.sandbox;
|
||||||
const innerConfig = this.buildCodexInnerConfig();
|
const innerConfig = this.buildCodexInnerConfig();
|
||||||
const rawResponse = await this.client.request("thread/start", {
|
const params: Record<string, unknown> = {
|
||||||
model,
|
model,
|
||||||
cwd: this.config.cwd ?? null,
|
cwd: this.config.cwd ?? null,
|
||||||
approvalPolicy,
|
approvalPolicy,
|
||||||
@@ -3652,13 +3698,27 @@ class CodexAppServerAgentSession implements AgentSession {
|
|||||||
: {}),
|
: {}),
|
||||||
...(innerConfig ? { config: innerConfig } : {}),
|
...(innerConfig ? { config: innerConfig } : {}),
|
||||||
...(this.ephemeral ? { ephemeral: true } : {}),
|
...(this.ephemeral ? { ephemeral: true } : {}),
|
||||||
});
|
};
|
||||||
|
applyApprovalsReviewerParam(params, preset);
|
||||||
|
const rawResponse = await this.client.request("thread/start", params);
|
||||||
const response = toObjectRecord(rawResponse);
|
const response = toObjectRecord(rawResponse);
|
||||||
const threadRecord = toObjectRecord(response?.thread);
|
const threadRecord = toObjectRecord(response?.thread);
|
||||||
const threadId = typeof threadRecord?.id === "string" ? threadRecord.id : undefined;
|
const threadId = typeof threadRecord?.id === "string" ? threadRecord.id : undefined;
|
||||||
if (!threadId) {
|
if (!threadId) {
|
||||||
throw new Error("Codex app-server did not return thread id");
|
throw new Error("Codex app-server did not return thread id");
|
||||||
}
|
}
|
||||||
|
const responseApprovalsReviewer =
|
||||||
|
typeof response?.approvalsReviewer === "string" ? response.approvalsReviewer : undefined;
|
||||||
|
if (
|
||||||
|
shouldPromoteThreadResponseToAutoReview({
|
||||||
|
approvalsReviewer: responseApprovalsReviewer,
|
||||||
|
approvalPolicy,
|
||||||
|
sandbox,
|
||||||
|
})
|
||||||
|
) {
|
||||||
|
this.currentMode = "auto-review";
|
||||||
|
this.cachedRuntimeInfo = null;
|
||||||
|
}
|
||||||
this.currentThreadId = threadId;
|
this.currentThreadId = threadId;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -4651,6 +4711,7 @@ export class CodexAppServerAgentClient implements AgentClient {
|
|||||||
readonly provider = CODEX_PROVIDER;
|
readonly provider = CODEX_PROVIDER;
|
||||||
readonly capabilities = CODEX_APP_SERVER_CAPABILITIES;
|
readonly capabilities = CODEX_APP_SERVER_CAPABILITIES;
|
||||||
private goalsEnabledPromise: Promise<boolean> | null = null;
|
private goalsEnabledPromise: Promise<boolean> | null = null;
|
||||||
|
private autoReviewEnabledPromise: Promise<boolean> | null = null;
|
||||||
|
|
||||||
constructor(
|
constructor(
|
||||||
private readonly logger: Logger,
|
private readonly logger: Logger,
|
||||||
@@ -4693,6 +4754,31 @@ export class CodexAppServerAgentClient implements AgentClient {
|
|||||||
return this.goalsEnabledPromise;
|
return this.goalsEnabledPromise;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private resolveAutoReviewEnabled(): Promise<boolean> {
|
||||||
|
if (!this.autoReviewEnabledPromise) {
|
||||||
|
this.autoReviewEnabledPromise = (async () => {
|
||||||
|
try {
|
||||||
|
const launchPrefix = await resolveCodexLaunchPrefix(this.runtimeSettings);
|
||||||
|
const versionOutput = await resolveBinaryVersion(launchPrefix.command);
|
||||||
|
const enabled = codexVersionAtLeast(versionOutput, CODEX_AUTO_REVIEW_MIN_VERSION);
|
||||||
|
this.logger.trace(
|
||||||
|
{
|
||||||
|
provider: CODEX_PROVIDER,
|
||||||
|
versionOutput,
|
||||||
|
enabled,
|
||||||
|
},
|
||||||
|
"provider.codex.config.auto_review_resolved",
|
||||||
|
);
|
||||||
|
return enabled;
|
||||||
|
} catch (error) {
|
||||||
|
this.logger.warn({ err: error }, "Failed to probe codex version for auto-review gate");
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
})();
|
||||||
|
}
|
||||||
|
return this.autoReviewEnabledPromise;
|
||||||
|
}
|
||||||
|
|
||||||
private async spawnAppServer(
|
private async spawnAppServer(
|
||||||
launchEnv?: Record<string, string>,
|
launchEnv?: Record<string, string>,
|
||||||
options?: { goalsEnabled?: boolean; agentId?: string },
|
options?: { goalsEnabled?: boolean; agentId?: string },
|
||||||
@@ -4737,6 +4823,7 @@ export class CodexAppServerAgentClient implements AgentClient {
|
|||||||
}
|
}
|
||||||
const sessionConfig: AgentSessionConfig = { ...config, provider: CODEX_PROVIDER };
|
const sessionConfig: AgentSessionConfig = { ...config, provider: CODEX_PROVIDER };
|
||||||
const goalsEnabled = await this.resolveGoalsEnabled();
|
const goalsEnabled = await this.resolveGoalsEnabled();
|
||||||
|
const autoReviewEnabled = await this.resolveAutoReviewEnabled();
|
||||||
const session = new CodexAppServerAgentSession(
|
const session = new CodexAppServerAgentSession(
|
||||||
sessionConfig,
|
sessionConfig,
|
||||||
null,
|
null,
|
||||||
@@ -4746,6 +4833,7 @@ export class CodexAppServerAgentClient implements AgentClient {
|
|||||||
this.sessionDeps(),
|
this.sessionDeps(),
|
||||||
options?.persistSession === false,
|
options?.persistSession === false,
|
||||||
goalsEnabled,
|
goalsEnabled,
|
||||||
|
autoReviewEnabled,
|
||||||
launchContext?.agentId,
|
launchContext?.agentId,
|
||||||
);
|
);
|
||||||
await session.connect();
|
await session.connect();
|
||||||
@@ -4765,6 +4853,7 @@ export class CodexAppServerAgentClient implements AgentClient {
|
|||||||
cwd: overrides?.cwd ?? storedConfig.cwd ?? process.cwd(),
|
cwd: overrides?.cwd ?? storedConfig.cwd ?? process.cwd(),
|
||||||
};
|
};
|
||||||
const goalsEnabled = await this.resolveGoalsEnabled();
|
const goalsEnabled = await this.resolveGoalsEnabled();
|
||||||
|
const autoReviewEnabled = await this.resolveAutoReviewEnabled();
|
||||||
const session = new CodexAppServerAgentSession(
|
const session = new CodexAppServerAgentSession(
|
||||||
merged,
|
merged,
|
||||||
handle,
|
handle,
|
||||||
@@ -4774,6 +4863,7 @@ export class CodexAppServerAgentClient implements AgentClient {
|
|||||||
this.sessionDeps(),
|
this.sessionDeps(),
|
||||||
false,
|
false,
|
||||||
goalsEnabled,
|
goalsEnabled,
|
||||||
|
autoReviewEnabled,
|
||||||
launchContext?.agentId,
|
launchContext?.agentId,
|
||||||
);
|
);
|
||||||
await session.connect();
|
await session.connect();
|
||||||
|
|||||||
@@ -234,6 +234,7 @@ export class CodexAppServerClient {
|
|||||||
|
|
||||||
if (isJsonRpcRequest(raw)) {
|
if (isJsonRpcRequest(raw)) {
|
||||||
const request = raw;
|
const request = raw;
|
||||||
|
this.traceRawEvent(request);
|
||||||
const handler = this.requestHandlers.get(request.method);
|
const handler = this.requestHandlers.get(request.method);
|
||||||
try {
|
try {
|
||||||
const result = handler ? await handler(request.params) : {};
|
const result = handler ? await handler(request.params) : {};
|
||||||
@@ -249,20 +250,24 @@ export class CodexAppServerClient {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (isJsonRpcNotification(raw)) {
|
if (isJsonRpcNotification(raw)) {
|
||||||
const traceContext = this.getTraceContext();
|
this.traceRawEvent(raw);
|
||||||
this.logger.trace(
|
|
||||||
{
|
|
||||||
provider: "codex",
|
|
||||||
agentId: traceContext.agentId,
|
|
||||||
sessionId: traceContext.sessionId ?? readProviderSessionId(raw.params),
|
|
||||||
turnId: traceContext.turnId ?? readProviderTurnId(raw.params),
|
|
||||||
method: raw.method,
|
|
||||||
params: raw.params,
|
|
||||||
rawEvent: raw,
|
|
||||||
},
|
|
||||||
"provider.codex.raw_event",
|
|
||||||
);
|
|
||||||
this.notificationHandler?.(raw.method, raw.params);
|
this.notificationHandler?.(raw.method, raw.params);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private traceRawEvent(raw: JsonRpcRequest | JsonRpcNotification): void {
|
||||||
|
const traceContext = this.getTraceContext();
|
||||||
|
this.logger.trace(
|
||||||
|
{
|
||||||
|
provider: "codex",
|
||||||
|
agentId: traceContext.agentId,
|
||||||
|
sessionId: traceContext.sessionId ?? readProviderSessionId(raw.params),
|
||||||
|
turnId: traceContext.turnId ?? readProviderTurnId(raw.params),
|
||||||
|
method: raw.method,
|
||||||
|
params: raw.params,
|
||||||
|
rawEvent: raw,
|
||||||
|
},
|
||||||
|
"provider.codex.raw_event",
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,382 @@
|
|||||||
|
import { execFileSync } from "node:child_process";
|
||||||
|
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||||
|
import { appendFile, writeFile } from "node:fs/promises";
|
||||||
|
import { createServer } from "node:http";
|
||||||
|
import os from "node:os";
|
||||||
|
import path from "node:path";
|
||||||
|
import { Writable } from "node:stream";
|
||||||
|
import { once } from "node:events";
|
||||||
|
|
||||||
|
import pino from "pino";
|
||||||
|
import { describe, expect, test } from "vitest";
|
||||||
|
|
||||||
|
import { CodexAppServerAgentClient } from "../agent/providers/codex-app-server-agent.js";
|
||||||
|
import { createMessageCollector } from "../test-utils/message-collector.js";
|
||||||
|
import { createTestPaseoDaemon } from "../test-utils/paseo-daemon.js";
|
||||||
|
import { DaemonClient } from "../test-utils/daemon-client.js";
|
||||||
|
|
||||||
|
const QA_REPORT_PATH = "/tmp/codex-auto-review-qa.md";
|
||||||
|
|
||||||
|
function parseCodexVersion(): [number, number, number] | null {
|
||||||
|
try {
|
||||||
|
const output = execFileSync("codex", ["--version"], { encoding: "utf8" });
|
||||||
|
const match = output.match(/(\d+)\.(\d+)\.(\d+)/);
|
||||||
|
if (!match) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
return [Number(match[1]), Number(match[2]), Number(match[3])];
|
||||||
|
} catch {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function codexVersionAtLeast(version: [number, number, number] | null): boolean {
|
||||||
|
if (!version) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
const min = [0, 115, 0] as const;
|
||||||
|
for (let index = 0; index < min.length; index += 1) {
|
||||||
|
if (version[index] > min[index]) return true;
|
||||||
|
if (version[index] < min[index]) return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
function sse(events: unknown[]): string {
|
||||||
|
return events
|
||||||
|
.map((event) => {
|
||||||
|
const type =
|
||||||
|
typeof event === "object" &&
|
||||||
|
event !== null &&
|
||||||
|
"type" in event &&
|
||||||
|
typeof (event as { type?: unknown }).type === "string"
|
||||||
|
? (event as { type: string }).type
|
||||||
|
: "message";
|
||||||
|
return `event: ${type}\ndata: ${JSON.stringify(event)}\n\n`;
|
||||||
|
})
|
||||||
|
.join("");
|
||||||
|
}
|
||||||
|
|
||||||
|
function responseCreated(id: string): Record<string, unknown> {
|
||||||
|
return { type: "response.created", response: { id } };
|
||||||
|
}
|
||||||
|
|
||||||
|
function responseCompleted(id: string): Record<string, unknown> {
|
||||||
|
return {
|
||||||
|
type: "response.completed",
|
||||||
|
response: {
|
||||||
|
id,
|
||||||
|
usage: {
|
||||||
|
input_tokens: 0,
|
||||||
|
input_tokens_details: null,
|
||||||
|
output_tokens: 0,
|
||||||
|
output_tokens_details: null,
|
||||||
|
total_tokens: 0,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function functionCallEvent(
|
||||||
|
callId: string,
|
||||||
|
name: string,
|
||||||
|
argumentsJson: string,
|
||||||
|
): Record<string, unknown> {
|
||||||
|
return {
|
||||||
|
type: "response.output_item.done",
|
||||||
|
item: {
|
||||||
|
type: "function_call",
|
||||||
|
call_id: callId,
|
||||||
|
name,
|
||||||
|
arguments: argumentsJson,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function assistantMessageEvent(id: string, text: string): Record<string, unknown> {
|
||||||
|
return {
|
||||||
|
type: "response.output_item.done",
|
||||||
|
item: {
|
||||||
|
type: "message",
|
||||||
|
role: "assistant",
|
||||||
|
id,
|
||||||
|
content: [{ type: "output_text", text }],
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function approvalToolCallSse(): string {
|
||||||
|
return sse([
|
||||||
|
responseCreated("resp-main-1"),
|
||||||
|
functionCallEvent(
|
||||||
|
"call-write-outside",
|
||||||
|
"exec_command",
|
||||||
|
JSON.stringify({
|
||||||
|
cmd: "python3 -c \"import urllib.request; urllib.request.urlopen('https://example.com', timeout=1)\"",
|
||||||
|
yield_time_ms: 1000,
|
||||||
|
sandbox_permissions: "require_escalated",
|
||||||
|
justification: "Probe auto-review approval routing.",
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
responseCompleted("resp-main-1"),
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
|
||||||
|
function guardianApprovalSse(): string {
|
||||||
|
return sse([
|
||||||
|
responseCreated("resp-guardian-1"),
|
||||||
|
assistantMessageEvent(
|
||||||
|
"msg-guardian-1",
|
||||||
|
JSON.stringify({
|
||||||
|
outcome: "allow",
|
||||||
|
risk_level: "low",
|
||||||
|
user_authorization: "high",
|
||||||
|
rationale: "Test harness approves the isolated write.",
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
responseCompleted("resp-guardian-1"),
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
|
||||||
|
function finalAnswerSse(): string {
|
||||||
|
return sse([
|
||||||
|
responseCreated("resp-main-2"),
|
||||||
|
assistantMessageEvent("msg-main-2", "done"),
|
||||||
|
responseCompleted("resp-main-2"),
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function startMockResponsesServer(sequence: string[]): Promise<{
|
||||||
|
url: string;
|
||||||
|
close: () => Promise<void>;
|
||||||
|
requestBodies: unknown[];
|
||||||
|
}> {
|
||||||
|
const requestBodies: unknown[] = [];
|
||||||
|
let index = 0;
|
||||||
|
const server = createServer((req, res) => {
|
||||||
|
const chunks: Buffer[] = [];
|
||||||
|
req.on("data", (chunk) => chunks.push(Buffer.from(chunk)));
|
||||||
|
req.on("end", () => {
|
||||||
|
const rawBody = Buffer.concat(chunks).toString("utf8");
|
||||||
|
requestBodies.push(JSON.parse(rawBody));
|
||||||
|
if (req.method !== "POST" || req.url !== "/v1/responses") {
|
||||||
|
res.statusCode = 404;
|
||||||
|
res.end("not found");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const body = sequence[index] ?? finalAnswerSse();
|
||||||
|
index += 1;
|
||||||
|
res.statusCode = 200;
|
||||||
|
res.setHeader("content-type", "text/event-stream");
|
||||||
|
res.setHeader("cache-control", "no-cache");
|
||||||
|
res.end(body);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
server.listen(0, "127.0.0.1");
|
||||||
|
await once(server, "listening");
|
||||||
|
const address = server.address();
|
||||||
|
if (!address || typeof address === "string") {
|
||||||
|
throw new Error("Expected TCP address for mock responses server");
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
url: `http://127.0.0.1:${address.port}`,
|
||||||
|
requestBodies,
|
||||||
|
close: () =>
|
||||||
|
new Promise<void>((resolve, reject) => {
|
||||||
|
server.close((error) => {
|
||||||
|
if (error) reject(error);
|
||||||
|
else resolve();
|
||||||
|
});
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function writeMockCodexConfig(codexHome: string, serverUrl: string): void {
|
||||||
|
writeFileSync(
|
||||||
|
path.join(codexHome, "config.toml"),
|
||||||
|
`
|
||||||
|
model = "mock-model"
|
||||||
|
model_provider = "mock_provider"
|
||||||
|
approval_policy = "never"
|
||||||
|
sandbox_mode = "read-only"
|
||||||
|
|
||||||
|
[features]
|
||||||
|
guardian_approval = true
|
||||||
|
|
||||||
|
[model_providers.mock_provider]
|
||||||
|
name = "Mock provider"
|
||||||
|
base_url = "${serverUrl}/v1"
|
||||||
|
wire_api = "responses"
|
||||||
|
request_max_retries = 0
|
||||||
|
stream_max_retries = 0
|
||||||
|
supports_websockets = false
|
||||||
|
`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
function createTraceLogger() {
|
||||||
|
const records: Array<Record<string, unknown>> = [];
|
||||||
|
const stream = new Writable({
|
||||||
|
write(chunk, _encoding, callback) {
|
||||||
|
try {
|
||||||
|
records.push(JSON.parse(chunk.toString()) as Record<string, unknown>);
|
||||||
|
} catch {
|
||||||
|
// Ignore non-JSON logger output.
|
||||||
|
}
|
||||||
|
callback();
|
||||||
|
},
|
||||||
|
});
|
||||||
|
return {
|
||||||
|
logger: pino({ level: "trace" }, stream),
|
||||||
|
records,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function rawCodexEvents(records: Array<Record<string, unknown>>): Array<Record<string, unknown>> {
|
||||||
|
return records
|
||||||
|
.filter((record) => record.msg === "provider.codex.raw_event")
|
||||||
|
.map((record) => ({
|
||||||
|
method: record.method,
|
||||||
|
params: record.params,
|
||||||
|
rawEvent: record.rawEvent,
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
|
||||||
|
function hasClientPermissionRequest(messages: unknown[]): boolean {
|
||||||
|
return messages.some(
|
||||||
|
(message) =>
|
||||||
|
typeof message === "object" &&
|
||||||
|
message !== null &&
|
||||||
|
"type" in message &&
|
||||||
|
(message as { type?: unknown }).type === "agent_permission_request",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function appendQaSection(title: string, payload: unknown): Promise<void> {
|
||||||
|
await appendFile(
|
||||||
|
QA_REPORT_PATH,
|
||||||
|
`\n## ${title}\n\n\`\`\`json\n${JSON.stringify(payload, null, 2)}\n\`\`\`\n`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function runScenario(modeId: "auto" | "auto-review") {
|
||||||
|
const cwd = mkdtempSync(path.join(os.tmpdir(), `codex-auto-review-${modeId}-cwd-`));
|
||||||
|
const codexHome = mkdtempSync(path.join(os.tmpdir(), `codex-auto-review-${modeId}-home-`));
|
||||||
|
const mockServer = await startMockResponsesServer(
|
||||||
|
modeId === "auto-review"
|
||||||
|
? [approvalToolCallSse(), guardianApprovalSse(), finalAnswerSse()]
|
||||||
|
: [approvalToolCallSse(), finalAnswerSse()],
|
||||||
|
);
|
||||||
|
const { logger, records } = createTraceLogger();
|
||||||
|
const daemon = await createTestPaseoDaemon({
|
||||||
|
agentClients: {
|
||||||
|
codex: new CodexAppServerAgentClient(logger, {
|
||||||
|
env: {
|
||||||
|
CODEX_HOME: codexHome,
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
logger,
|
||||||
|
});
|
||||||
|
const client = new DaemonClient({
|
||||||
|
url: `ws://127.0.0.1:${daemon.port}/ws`,
|
||||||
|
appVersion: "0.1.75",
|
||||||
|
});
|
||||||
|
const collector = createMessageCollector(client);
|
||||||
|
|
||||||
|
try {
|
||||||
|
writeMockCodexConfig(codexHome, mockServer.url);
|
||||||
|
await client.connect();
|
||||||
|
await client.fetchAgents({ subscribe: { subscriptionId: `codex-auto-review-${modeId}` } });
|
||||||
|
|
||||||
|
const agent = await client.createAgent({
|
||||||
|
provider: "codex",
|
||||||
|
cwd,
|
||||||
|
title: `codex-auto-review-${modeId}`,
|
||||||
|
modeId,
|
||||||
|
model: "mock-model",
|
||||||
|
thinkingOptionId: "medium",
|
||||||
|
});
|
||||||
|
|
||||||
|
collector.clear();
|
||||||
|
await client.sendAgentMessage(agent.id, "Run the scripted command.");
|
||||||
|
|
||||||
|
if (modeId === "auto") {
|
||||||
|
const permissionState = await client.waitForFinish(agent.id, 30_000);
|
||||||
|
expect(permissionState.status).toBe("permission");
|
||||||
|
const permission = permissionState.final?.pendingPermissions?.[0];
|
||||||
|
expect(permission?.kind).toBe("tool");
|
||||||
|
await client.respondToPermission(agent.id, permission!.id, { behavior: "deny" });
|
||||||
|
}
|
||||||
|
|
||||||
|
const finish = await client.waitForFinish(agent.id, 60_000);
|
||||||
|
expect(finish.status).toBe("idle");
|
||||||
|
|
||||||
|
return {
|
||||||
|
modeId,
|
||||||
|
websocketMessages: [...collector.messages],
|
||||||
|
rawCodexEvents: rawCodexEvents(records),
|
||||||
|
mockResponsesRequests: mockServer.requestBodies,
|
||||||
|
};
|
||||||
|
} finally {
|
||||||
|
collector.unsubscribe();
|
||||||
|
await client.close();
|
||||||
|
await daemon.close();
|
||||||
|
await mockServer.close();
|
||||||
|
rmSync(cwd, { recursive: true, force: true });
|
||||||
|
rmSync(codexHome, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const codexVersion = parseCodexVersion();
|
||||||
|
const canRun = codexVersionAtLeast(codexVersion);
|
||||||
|
|
||||||
|
describe.skipIf(!canRun)("daemon E2E (real codex) - Auto-review mode", () => {
|
||||||
|
test("Auto-review routes on-request approvals through the Codex auto-reviewer subagent", async () => {
|
||||||
|
await writeFile(
|
||||||
|
QA_REPORT_PATH,
|
||||||
|
`# Codex Auto-review QA\n\nE2E started with codex version ${JSON.stringify(codexVersion)}.\n`,
|
||||||
|
);
|
||||||
|
|
||||||
|
const result = await runScenario("auto-review");
|
||||||
|
const methods = result.rawCodexEvents.map((event) => event.method);
|
||||||
|
|
||||||
|
expect(methods).toContain("item/autoApprovalReview/started");
|
||||||
|
expect(methods).toContain("item/autoApprovalReview/completed");
|
||||||
|
expect(methods).not.toContain("item/commandExecution/requestApproval");
|
||||||
|
expect(hasClientPermissionRequest(result.websocketMessages)).toBe(false);
|
||||||
|
|
||||||
|
await appendQaSection("E2E Auto-review Wire Payloads", {
|
||||||
|
rawCodexEvents: result.rawCodexEvents.filter(
|
||||||
|
(event) =>
|
||||||
|
event.method === "item/autoApprovalReview/started" ||
|
||||||
|
event.method === "item/autoApprovalReview/completed",
|
||||||
|
),
|
||||||
|
requestApprovalEvents: result.rawCodexEvents.filter(
|
||||||
|
(event) => event.method === "item/commandExecution/requestApproval",
|
||||||
|
),
|
||||||
|
});
|
||||||
|
}, 90_000);
|
||||||
|
|
||||||
|
test("Default Permissions surfaces the same approval as a client permission request", async () => {
|
||||||
|
const result = await runScenario("auto");
|
||||||
|
const methods = result.rawCodexEvents.map((event) => event.method);
|
||||||
|
|
||||||
|
expect(methods).toContain("item/commandExecution/requestApproval");
|
||||||
|
expect(methods).not.toContain("item/autoApprovalReview/started");
|
||||||
|
expect(methods).not.toContain("item/autoApprovalReview/completed");
|
||||||
|
expect(hasClientPermissionRequest(result.websocketMessages)).toBe(true);
|
||||||
|
|
||||||
|
await appendQaSection("E2E Default Permissions Wire Payloads", {
|
||||||
|
rawCodexEvents: result.rawCodexEvents.filter(
|
||||||
|
(event) => event.method === "item/commandExecution/requestApproval",
|
||||||
|
),
|
||||||
|
autoApprovalReviewEvents: result.rawCodexEvents.filter(
|
||||||
|
(event) =>
|
||||||
|
event.method === "item/autoApprovalReview/started" ||
|
||||||
|
event.method === "item/autoApprovalReview/completed",
|
||||||
|
),
|
||||||
|
});
|
||||||
|
}, 90_000);
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user