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
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ describe("CodexAppServerAgent", () => {
).toBe("untrusted");
});

it("falls back to auto for a non-codex initial permissionMode", async () => {
it("maps bypassPermissions to Codex full-access", async () => {
const stub = makeStubRpc({
"thread/start": { thread: { id: "t" } },
"turn/start": { turn: { id: "turn_1" } },
Expand All @@ -874,7 +874,6 @@ describe("CodexAppServerAgent", () => {
processOptions: { binaryPath: "/x/codex" },
rpcFactory: stub.factory,
});
// "bypassPermissions" is a Claude mode, not a codex mode → default "auto".
await agent.newSession({
cwd: "/r",
_meta: { permissionMode: "bypassPermissions" },
Expand All @@ -889,7 +888,10 @@ describe("CodexAppServerAgent", () => {
const turnStart = stub.requests.find((r) => r.method === "turn/start");
expect(
(turnStart?.params as { approvalPolicy?: string }).approvalPolicy,
).toBe("on-request");
).toBe("never");
expect(
(turnStart?.params as { sandboxPolicy?: unknown }).sandboxPolicy,
).toEqual({ type: "dangerFullAccess" });
});

it("applies a read-only sandboxPolicy + approvalPolicy when the picker is Plan", async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
collaborationModeFor,
DEFAULT_EFFORTS,
modeApprovalPolicy,
resolveCodexMode,
SessionConfigState,
sandboxPolicyFor,
} from "./session-config";

Expand Down Expand Up @@ -106,6 +108,34 @@ describe("collaborationModeFor", () => {
});
});

describe("resolveCodexMode", () => {
it.each([
["read-only", "read-only"],
["auto", "auto"],
["full-access", "full-access"],
["bypassPermissions", "full-access"],
["default", "auto"],
[undefined, "auto"],
])("maps host mode %s to codex mode %s", (mode, expected) => {
expect(resolveCodexMode(mode)).toBe(expected);
});
});

describe("SessionConfigState", () => {
it("canonicalizes bypassPermissions during a live mode update", () => {
const config = new SessionConfigState("gpt-5.5");

config.setOption("mode", "bypassPermissions");

expect(config.mode).toBe("full-access");
expect(config.approvalPolicy()).toBe("never");
expect(config.sandboxPolicy()).toEqual({ type: "dangerFullAccess" });
expect(
config.options.find((option) => option.category === "mode")?.currentValue,
).toBe("full-access");
});
});

describe("buildConfigOptions", () => {
const byCategory = (
opts: ReturnType<typeof buildConfigOptions>,
Expand Down
23 changes: 14 additions & 9 deletions packages/agent/src/adapters/codex-app-server/session-config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import type { SessionConfigOption } from "@agentclientprotocol/sdk";
import { CODEX_MODE_PRESETS, type CodexModePreset } from "@posthog/shared";
import {
CODEX_MODE_PRESETS,
type CodexModePreset,
type ExecutionMode,
resolveCloudInitialPermissionMode,
} from "@posthog/shared";
import { type GatewayModel, isOpenAIModel } from "../../gateway-models";
import { getReasoningEffortOptions } from "./models";

Expand Down Expand Up @@ -121,13 +126,13 @@ export function collaborationModeFor(
}

/**
* Resolve the host's initial `_meta.permissionMode` to a codex mode. A recognized
* mode is honored; anything else (e.g. "bypassPermissions") falls back to default.
* Resolve a host permission mode or live picker value to a codex mode. A
* recognized mode is honored; Claude's bypass mode maps to Codex full access.
* Other unknown modes fall back to default.
*/
export function resolveInitialMode(permissionMode: string | undefined): string {
return permissionMode && CODEX_MODES.some((m) => m.id === permissionMode)
? permissionMode
: DEFAULT_MODE;
export function resolveCodexMode(mode: string | undefined): string {
if (!mode) return DEFAULT_MODE;
return resolveCloudInitialPermissionMode("codex", mode as ExecutionMode);
}

/** Codex's standard reasoning efforts; used when model/list doesn't expose them. */
Expand Down Expand Up @@ -246,7 +251,7 @@ export class SessionConfigState {

/** Apply the host's initial approval mode (from `_meta.permissionMode`). */
setInitialMode(permissionMode: string | undefined): void {
this._mode = resolveInitialMode(permissionMode);
this._mode = resolveCodexMode(permissionMode);
this.rebuild();
}

Expand All @@ -260,7 +265,7 @@ export class SessionConfigState {
if (configId === "model") this._model = value;
else if (configId === "effort") this._effort = value;
else if (configId === "mode") {
this._mode = value;
this._mode = resolveCodexMode(value);
modeChanged = true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ function createHarness() {
taskViewedApi: { markActivity },
getPersistedConfigOptions: () => undefined,
setPersistedConfigOptions: vi.fn(),
adapterStore: {
getAdapter: () => undefined,
setAdapter: vi.fn(),
removeAdapter: vi.fn(),
},
trpc: {
agent: {
onSessionIdleKilled: {
Expand Down
83 changes: 53 additions & 30 deletions packages/core/src/sessions/sessionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,6 @@ export interface SessionServiceDeps {
options: SessionConfigOption[],
) => void;
removePersistedConfigOptions: (taskRunId: string) => void;
updatePersistedConfigOptionValue: (...args: any[]) => any;
adapterStore: {
getAdapter(taskRunId: string): Adapter | undefined;
setAdapter(taskRunId: string, adapter: Adapter): void;
Expand Down Expand Up @@ -4029,7 +4028,7 @@ export class SessionService {
this.d.store.updateSession(session.taskRunId, {
configOptions: updatedOptions,
});
this.d.updatePersistedConfigOptionValue(session.taskRunId, configId, value);
this.d.setPersistedConfigOptions(session.taskRunId, updatedOptions);

if (
!session.isCloud &&
Expand All @@ -4054,20 +4053,25 @@ export class SessionService {
});
}
} catch (error) {
// Rollback on error
const rolledBackOptions = configOptions.map((opt) =>
opt.id === configId
? ({ ...opt, currentValue: previousValue } as SessionConfigOption)
: opt,
);
this.d.store.updateSession(session.taskRunId, {
configOptions: rolledBackOptions,
});
this.d.updatePersistedConfigOptionValue(
session.taskRunId,
configId,
String(previousValue),
const latestConfigOptions =
this.d.store.getSessionByTaskId(taskId)?.configOptions ?? [];
const latestOption = latestConfigOptions.find(
(option) => option.id === configId,
);
if (latestOption?.currentValue === value) {
const rolledBackOptions = latestConfigOptions.map((option) =>
option.id === configId
? ({
...option,
currentValue: previousValue,
} as SessionConfigOption)
: option,
);
this.d.store.updateSession(session.taskRunId, {
configOptions: rolledBackOptions,
});
this.d.setPersistedConfigOptions(session.taskRunId, rolledBackOptions);
}
this.d.log.error("Failed to set session config option", {
taskId,
configId,
Expand Down Expand Up @@ -4483,8 +4487,33 @@ export class SessionService {
runState?: Record<string, unknown>,
): () => void {
const taskRunId = runId;
const persistedConfigOptions = this.d.getPersistedConfigOptions(taskRunId);
const persistedAdapter = this.d.adapterStore.getAdapter(taskRunId);
const buildInitialConfigOptions = (
mode: string | undefined,
configAdapter: Adapter | undefined = persistedAdapter,
): SessionConfigOption[] => {
const defaults = addMissingCloudRuntimeConfigOptions(
buildCloudDefaultConfigOptions(mode, adapter),
adapter,
initialModel,
initialReasoningEffort,
);
if (!persistedConfigOptions?.length) return defaults;
if (configAdapter && configAdapter !== adapter) return defaults;

const defaultIds = new Set(defaults.map((option) => option.id));
const completeOptions = [
...defaults,
...persistedConfigOptions.filter(
(option) => !defaultIds.has(option.id),
),
];
return mergeConfigOptions(completeOptions, persistedConfigOptions);
Comment thread
richardsolomou marked this conversation as resolved.
};

if (this.supersededRunIds.has(runId)) return () => {};
this.d.adapterStore.setAdapter(taskRunId, adapter);

const existingWatcher = this.cloudTaskWatchers.get(taskId);

Expand Down Expand Up @@ -4512,11 +4541,9 @@ export class SessionService {
if (shouldRefreshConfigOptions) {
this.d.store.updateSession(existing.taskRunId, {
adapter,
configOptions: addMissingCloudRuntimeConfigOptions(
buildCloudDefaultConfigOptions(currentMode, adapter),
adapter,
initialModel,
initialReasoningEffort,
configOptions: buildInitialConfigOptions(
currentMode,
existing.adapter,
),
});
} else {
Expand Down Expand Up @@ -4613,11 +4640,9 @@ export class SessionService {
session.status = "disconnected";
session.isCloud = true;
session.adapter = adapter;
session.configOptions = addMissingCloudRuntimeConfigOptions(
buildCloudDefaultConfigOptions(initialMode, adapter),
adapter,
initialModel,
initialReasoningEffort,
session.configOptions = buildInitialConfigOptions(
initialMode,
existing?.taskRunId === taskRunId ? existing.adapter : persistedAdapter,
);
this.d.store.setSession(session);
// Optimistic seeding for the initial task description is deferred
Expand All @@ -4636,11 +4661,9 @@ export class SessionService {
)?.currentValue;
const currentMode =
typeof existingMode === "string" ? existingMode : initialMode;
updates.configOptions = addMissingCloudRuntimeConfigOptions(
buildCloudDefaultConfigOptions(currentMode, adapter),
adapter,
initialModel,
initialReasoningEffort,
updates.configOptions = buildInitialConfigOptions(
currentMode,
existing.adapter,
);
} else {
const configOptions = addMissingCloudRuntimeConfigOptions(
Expand Down
33 changes: 0 additions & 33 deletions packages/ui/src/features/sessions/sessionConfigStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ interface SessionConfigActions {
getConfigOptions: (taskRunId: string) => SessionConfigOption[] | undefined;
/** Remove config options for a task run */
removeConfigOptions: (taskRunId: string) => void;
/** Update a single config option value */
updateConfigOptionValue: (
taskRunId: string,
configId: string,
value: string,
) => void;
}

type SessionConfigStore = SessionConfigState & SessionConfigActions;
Expand All @@ -42,22 +36,6 @@ export const useSessionConfigStore = create<SessionConfigStore>()(
const { [taskRunId]: _removed, ...rest } = state.configsByRunId;
return { configsByRunId: rest };
}),

updateConfigOptionValue: (taskRunId, configId, value) =>
set((state) => {
const existing = state.configsByRunId[taskRunId];
if (!existing) return state;

const updated = existing.map((opt) =>
opt.id === configId
? ({ ...opt, currentValue: value } as SessionConfigOption)
: opt,
);

return {
configsByRunId: { ...state.configsByRunId, [taskRunId]: updated },
};
}),
}),
{
name: "session-config-storage",
Expand Down Expand Up @@ -86,14 +64,3 @@ export function setPersistedConfigOptions(
export function removePersistedConfigOptions(taskRunId: string): void {
useSessionConfigStore.getState().removeConfigOptions(taskRunId);
}

/** Non-hook accessor for updating a single config option value */
export function updatePersistedConfigOptionValue(
taskRunId: string,
configId: string,
value: string,
): void {
useSessionConfigStore
.getState()
.updateConfigOptionValue(taskRunId, configId, value);
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ const mockSessionConfigStore = vi.hoisted(() => ({
getPersistedConfigOptions: vi.fn(() => undefined),
setPersistedConfigOptions: vi.fn(),
removePersistedConfigOptions: vi.fn(),
updatePersistedConfigOptionValue: vi.fn(),
}));

vi.mock(
Expand Down
Loading
Loading