Skip to content

Commit 33783a0

Browse files
fix(agent): address permission review feedback
Generated-By: PostHog Code Task-Id: 1f1fe07c-6384-4d1e-8ab9-bad3700caa2e
1 parent 1821651 commit 33783a0

4 files changed

Lines changed: 114 additions & 4 deletions

File tree

packages/agent/src/adapters/codex-app-server/session-config.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
DEFAULT_EFFORTS,
88
modeApprovalPolicy,
99
resolveInitialMode,
10+
SessionConfigState,
1011
sandboxPolicyFor,
1112
} from "./session-config";
1213

@@ -120,6 +121,21 @@ describe("resolveInitialMode", () => {
120121
});
121122
});
122123

124+
describe("SessionConfigState", () => {
125+
it("canonicalizes bypassPermissions during a live mode update", () => {
126+
const config = new SessionConfigState("gpt-5.5");
127+
128+
config.setOption("mode", "bypassPermissions");
129+
130+
expect(config.mode).toBe("full-access");
131+
expect(config.approvalPolicy()).toBe("never");
132+
expect(config.sandboxPolicy()).toEqual({ type: "dangerFullAccess" });
133+
expect(
134+
config.options.find((option) => option.category === "mode")?.currentValue,
135+
).toBe("full-access");
136+
});
137+
});
138+
123139
describe("buildConfigOptions", () => {
124140
const byCategory = (
125141
opts: ReturnType<typeof buildConfigOptions>,

packages/agent/src/adapters/codex-app-server/session-config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ export class SessionConfigState {
268268
if (configId === "model") this._model = value;
269269
else if (configId === "effort") this._effort = value;
270270
else if (configId === "mode") {
271-
this._mode = value;
271+
this._mode = resolveInitialMode(value);
272272
modeChanged = true;
273273
}
274274
}

packages/core/src/sessions/sessionService.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4488,8 +4488,10 @@ export class SessionService {
44884488
): () => void {
44894489
const taskRunId = runId;
44904490
const persistedConfigOptions = this.d.getPersistedConfigOptions(taskRunId);
4491+
const persistedAdapter = this.d.adapterStore.getAdapter(taskRunId);
44914492
const buildInitialConfigOptions = (
44924493
mode: string | undefined,
4494+
configAdapter: Adapter | undefined = persistedAdapter,
44934495
): SessionConfigOption[] => {
44944496
const defaults = addMissingCloudRuntimeConfigOptions(
44954497
buildCloudDefaultConfigOptions(mode, adapter),
@@ -4498,6 +4500,7 @@ export class SessionService {
44984500
initialReasoningEffort,
44994501
);
45004502
if (!persistedConfigOptions?.length) return defaults;
4503+
if (configAdapter && configAdapter !== adapter) return defaults;
45014504

45024505
const defaultIds = new Set(defaults.map((option) => option.id));
45034506
const completeOptions = [
@@ -4510,6 +4513,7 @@ export class SessionService {
45104513
};
45114514

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

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

@@ -4537,7 +4541,10 @@ export class SessionService {
45374541
if (shouldRefreshConfigOptions) {
45384542
this.d.store.updateSession(existing.taskRunId, {
45394543
adapter,
4540-
configOptions: buildInitialConfigOptions(currentMode),
4544+
configOptions: buildInitialConfigOptions(
4545+
currentMode,
4546+
existing.adapter,
4547+
),
45414548
});
45424549
} else {
45434550
const configOptions = addMissingCloudRuntimeConfigOptions(
@@ -4633,7 +4640,10 @@ export class SessionService {
46334640
session.status = "disconnected";
46344641
session.isCloud = true;
46354642
session.adapter = adapter;
4636-
session.configOptions = buildInitialConfigOptions(initialMode);
4643+
session.configOptions = buildInitialConfigOptions(
4644+
initialMode,
4645+
existing?.taskRunId === taskRunId ? existing.adapter : persistedAdapter,
4646+
);
46374647
this.d.store.setSession(session);
46384648
// Optimistic seeding for the initial task description is deferred
46394649
// until `hydrateCloudTaskSessionFromLogs` confirms there's no prior
@@ -4651,7 +4661,10 @@ export class SessionService {
46514661
)?.currentValue;
46524662
const currentMode =
46534663
typeof existingMode === "string" ? existingMode : initialMode;
4654-
updates.configOptions = buildInitialConfigOptions(currentMode);
4664+
updates.configOptions = buildInitialConfigOptions(
4665+
currentMode,
4666+
existing.adapter,
4667+
);
46554668
} else {
46564669
const configOptions = addMissingCloudRuntimeConfigOptions(
46574670
existing.configOptions,

packages/ui/src/features/sessions/sessionServiceHost.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,8 @@ describe("SessionService", () => {
432432
mockGetConfigOptionByCategory.mockReturnValue(undefined);
433433
mockBuildAuthenticatedClient.mockReturnValue(mockAuthenticatedClient);
434434
mockAuthenticatedClient.getTaskRunSessionLogs.mockResolvedValue([]);
435+
mockSessionConfigStore.getPersistedConfigOptions.mockReturnValue(undefined);
436+
mockAdapterFns.getAdapter.mockReturnValue(undefined);
435437
mockSessionStoreSetters.getSessionByTaskId.mockReturnValue(undefined);
436438
mockSessionStoreSetters.getSessions.mockReturnValue({});
437439
mockAuth.fetchAuthState.mockResolvedValue({
@@ -1072,6 +1074,85 @@ describe("SessionService", () => {
10721074
mockSessionConfigStore.setPersistedConfigOptions.mockReset();
10731075
});
10741076

1077+
it("drops persisted options when the cloud adapter changes", () => {
1078+
const service = getSessionService();
1079+
mockAdapterFns.getAdapter.mockReturnValue("claude");
1080+
mockSessionConfigStore.getPersistedConfigOptions.mockReturnValue([
1081+
{
1082+
id: "mode",
1083+
name: "Mode",
1084+
type: "select",
1085+
category: "mode",
1086+
currentValue: "acceptEdits",
1087+
options: [],
1088+
},
1089+
{
1090+
id: "model",
1091+
name: "Model",
1092+
type: "select",
1093+
category: "model",
1094+
currentValue: "claude-opus-4-8",
1095+
options: [],
1096+
},
1097+
{
1098+
id: "effort",
1099+
name: "Effort",
1100+
type: "select",
1101+
category: "thought_level",
1102+
currentValue: "high",
1103+
options: [],
1104+
},
1105+
]);
1106+
1107+
service.watchCloudTask(
1108+
"task-adapter-change",
1109+
"run-adapter-change",
1110+
"https://api.example.com",
1111+
123,
1112+
undefined,
1113+
undefined,
1114+
"auto",
1115+
"codex",
1116+
"gpt-5.5",
1117+
undefined,
1118+
undefined,
1119+
undefined,
1120+
"max",
1121+
);
1122+
1123+
expect(mockSessionStoreSetters.setSession).toHaveBeenCalledWith(
1124+
expect.objectContaining({
1125+
adapter: "codex",
1126+
configOptions: expect.arrayContaining([
1127+
expect.objectContaining({
1128+
category: "mode",
1129+
currentValue: "auto",
1130+
}),
1131+
expect.objectContaining({
1132+
category: "model",
1133+
currentValue: "gpt-5.5",
1134+
}),
1135+
expect.objectContaining({
1136+
category: "thought_level",
1137+
currentValue: "max",
1138+
}),
1139+
]),
1140+
}),
1141+
);
1142+
const session = mockSessionStoreSetters.setSession.mock.calls.at(-1)?.[0];
1143+
expect(session?.configOptions).not.toEqual(
1144+
expect.arrayContaining([
1145+
expect.objectContaining({ currentValue: "acceptEdits" }),
1146+
expect.objectContaining({ currentValue: "claude-opus-4-8" }),
1147+
expect.objectContaining({ id: "effort" }),
1148+
]),
1149+
);
1150+
expect(mockAdapterFns.setAdapter).toHaveBeenCalledWith(
1151+
"run-adapter-change",
1152+
"codex",
1153+
);
1154+
});
1155+
10751156
it("shows the selected cloud model and reasoning before preview config loads", () => {
10761157
const service = getSessionService();
10771158

0 commit comments

Comments
 (0)