Skip to content

Commit a56b832

Browse files
authored
feat: Harden CodexAcpAgent session state and permission handling (#1487)
## Changes 1. Initialize sessionState and codexConnection eagerly in the constructor instead of using ! assertions 2. Pass permissionMode, taskRunId and taskId from meta into createSessionState for all session creation paths (load, new, fork) 3. Fix double-counting of token usage by relying on sessionUpdate notifications instead of also adding response.usage 4. Remove redundant cancel() override, use inherited interrupt() and base class cancellation state 5. Use POSTHOG_NOTIFICATIONS.USAGE_UPDATE constant instead of hardcoded string 6. Escape newlines in system instructions passed to codex CLI args 7. Abort the session's AbortController on closeSession for proper async cleanup ## How did you test this? Manually
1 parent 8afb366 commit a56b832

6 files changed

Lines changed: 82 additions & 98 deletions

File tree

packages/agent/src/acp-extensions.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,7 @@ export const POSTHOG_NOTIFICATIONS = {
6464

6565
/** Marks a boundary for log compaction */
6666
COMPACT_BOUNDARY: "_posthog/compact_boundary",
67+
68+
/** Token usage update for a session turn */
69+
USAGE_UPDATE: "_posthog/usage_update",
6770
} as const;

packages/agent/src/adapters/claude/claude-agent.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import {
4444
} from "@anthropic-ai/claude-agent-sdk";
4545
import { v7 as uuidv7 } from "uuid";
4646
import packageJson from "../../../package.json" with { type: "json" };
47+
import { POSTHOG_NOTIFICATIONS } from "../../acp-extensions";
4748
import { unreachable, withTimeout } from "../../utils/common";
4849
import { Logger } from "../../utils/logger";
4950
import { Pushable } from "../../utils/streams";
@@ -442,16 +443,19 @@ export class ClaudeAcpAgent extends BaseAcpAgent {
442443
});
443444
}
444445

445-
await this.client.extNotification("_posthog/usage_update", {
446-
sessionId: params.sessionId,
447-
used: {
448-
inputTokens: message.usage.input_tokens,
449-
outputTokens: message.usage.output_tokens,
450-
cachedReadTokens: message.usage.cache_read_input_tokens,
451-
cachedWriteTokens: message.usage.cache_creation_input_tokens,
446+
await this.client.extNotification(
447+
POSTHOG_NOTIFICATIONS.USAGE_UPDATE,
448+
{
449+
sessionId: params.sessionId,
450+
used: {
451+
inputTokens: message.usage.input_tokens,
452+
outputTokens: message.usage.output_tokens,
453+
cachedReadTokens: message.usage.cache_read_input_tokens,
454+
cachedWriteTokens: message.usage.cache_creation_input_tokens,
455+
},
456+
cost: message.total_cost_usd,
452457
},
453-
cost: message.total_cost_usd,
454-
});
458+
);
455459

456460
const usage: Usage = {
457461
inputTokens: this.session.accumulatedUsage.inputTokens,

packages/agent/src/adapters/codex/codex-agent.ts

Lines changed: 51 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import {
1313
type AgentSideConnection,
1414
type AuthenticateRequest,
15-
type CancelNotification,
1615
ClientSideConnection,
1716
type ForkSessionRequest,
1817
type ForkSessionResponse,
@@ -37,9 +36,8 @@ import {
3736
import packageJson from "../../../package.json" with { type: "json" };
3837
import { POSTHOG_NOTIFICATIONS } from "../../acp-extensions";
3938
import {
40-
CODEX_NATIVE_MODES,
41-
type CodexNativeMode,
42-
type PermissionMode,
39+
CODE_EXECUTION_MODES,
40+
type CodeExecutionMode,
4341
} from "../../execution-mode";
4442
import type { ProcessSpawnedCallback } from "../../types";
4543
import { Logger } from "../../utils/logger";
@@ -85,19 +83,26 @@ type CodexSession = BaseSession & {
8583
settingsManager: CodexSettingsManager;
8684
};
8785

88-
function toPermissionMode(mode?: string): PermissionMode {
89-
if (mode && (CODEX_NATIVE_MODES as readonly string[]).includes(mode)) {
90-
return mode as CodexNativeMode;
86+
function toCodeExecutionMode(mode?: string): CodeExecutionMode {
87+
if (mode && (CODE_EXECUTION_MODES as readonly string[]).includes(mode)) {
88+
return mode as CodeExecutionMode;
9189
}
92-
return "auto";
90+
return "default";
9391
}
9492

93+
const CODEX_NATIVE_MODE: Record<CodeExecutionMode, string> = {
94+
default: "default",
95+
acceptEdits: "default",
96+
plan: "plan",
97+
bypassPermissions: "default",
98+
};
99+
95100
export class CodexAcpAgent extends BaseAcpAgent {
96101
readonly adapterName = "codex";
97102
declare session: CodexSession;
98103
private codexProcess: CodexProcess;
99-
private codexConnection!: ClientSideConnection;
100-
private sessionState!: CodexSessionState;
104+
private codexConnection: ClientSideConnection;
105+
private sessionState: CodexSessionState;
101106

102107
constructor(client: AgentSideConnection, options: CodexAcpAgentOptions) {
103108
super(client);
@@ -126,29 +131,14 @@ export class CodexAcpAgent extends BaseAcpAgent {
126131
cancelled: false,
127132
};
128133

134+
this.sessionState = createSessionState("", cwd);
135+
129136
// Create the ClientSideConnection to codex-acp.
130137
// The Client handler delegates all requests from codex-acp to the upstream
131138
// PostHog Code client via our AgentSideConnection.
132139
this.codexConnection = new ClientSideConnection(
133140
(_agent) =>
134-
createCodexClient(
135-
this.client,
136-
this.logger,
137-
this.sessionState ?? {
138-
sessionId: "",
139-
cwd: "",
140-
modeId: "auto",
141-
configOptions: [],
142-
accumulatedUsage: {
143-
inputTokens: 0,
144-
outputTokens: 0,
145-
cachedReadTokens: 0,
146-
cachedWriteTokens: 0,
147-
},
148-
permissionMode: "auto",
149-
cancelled: false,
150-
},
151-
),
141+
createCodexClient(this.client, this.logger, this.sessionState),
152142
codexStream,
153143
);
154144
}
@@ -195,7 +185,7 @@ export class CodexAcpAgent extends BaseAcpAgent {
195185
taskId: meta?.taskId ?? meta?.persistence?.taskId,
196186
modeId: response.modes?.currentModeId ?? "default",
197187
modelId: response.models?.currentModelId,
198-
permissionMode: toPermissionMode(meta?.permissionMode),
188+
permissionMode: toCodeExecutionMode(meta?.permissionMode),
199189
});
200190
this.sessionId = response.sessionId;
201191
this.sessionState.configOptions = response.configOptions ?? [];
@@ -219,9 +209,11 @@ export class CodexAcpAgent extends BaseAcpAgent {
219209

220210
async loadSession(params: LoadSessionRequest): Promise<LoadSessionResponse> {
221211
const response = await this.codexConnection.loadSession(params);
212+
const meta = params._meta as NewSessionMeta | undefined;
222213

223-
// Update session state
224-
this.sessionState = createSessionState(params.sessionId, params.cwd);
214+
this.sessionState = createSessionState(params.sessionId, params.cwd, {
215+
permissionMode: toCodeExecutionMode(meta?.permissionMode),
216+
});
225217
this.sessionId = params.sessionId;
226218
this.sessionState.configOptions = response.configOptions ?? [];
227219

@@ -238,11 +230,15 @@ export class CodexAcpAgent extends BaseAcpAgent {
238230
mcpServers: params.mcpServers ?? [],
239231
});
240232

241-
this.sessionState = createSessionState(params.sessionId, params.cwd);
233+
const meta = params._meta as NewSessionMeta | undefined;
234+
this.sessionState = createSessionState(params.sessionId, params.cwd, {
235+
taskRunId: meta?.taskRunId,
236+
taskId: meta?.taskId ?? meta?.persistence?.taskId,
237+
permissionMode: toCodeExecutionMode(meta?.permissionMode),
238+
});
242239
this.sessionId = params.sessionId;
243240
this.sessionState.configOptions = loadResponse.configOptions ?? [];
244241

245-
const meta = params._meta as NewSessionMeta | undefined;
246242
if (meta?.taskRunId) {
247243
await this.client.extNotification(POSTHOG_NOTIFICATIONS.SDK_SESSION, {
248244
taskRunId: meta.taskRunId,
@@ -268,7 +264,12 @@ export class CodexAcpAgent extends BaseAcpAgent {
268264
_meta: params._meta,
269265
});
270266

271-
this.sessionState = createSessionState(newResponse.sessionId, params.cwd);
267+
const meta = params._meta as NewSessionMeta | undefined;
268+
this.sessionState = createSessionState(newResponse.sessionId, params.cwd, {
269+
taskRunId: meta?.taskRunId,
270+
taskId: meta?.taskId ?? meta?.persistence?.taskId,
271+
permissionMode: toCodeExecutionMode(meta?.permissionMode),
272+
});
272273
this.sessionId = newResponse.sessionId;
273274
this.sessionState.configOptions = newResponse.configOptions ?? [];
274275

@@ -284,31 +285,21 @@ export class CodexAcpAgent extends BaseAcpAgent {
284285
async unstable_listSessions(
285286
params: ListSessionsRequest,
286287
): Promise<ListSessionsResponse> {
287-
return this.codexConnection.listSessions(params);
288+
return this.listSessions(params);
288289
}
289290

290291
async prompt(params: PromptRequest): Promise<PromptResponse> {
291-
if (this.sessionState) {
292-
this.sessionState.cancelled = false;
293-
this.sessionState.interruptReason = undefined;
294-
resetUsage(this.sessionState);
295-
}
292+
this.session.cancelled = false;
293+
this.session.interruptReason = undefined;
294+
resetUsage(this.sessionState);
296295

297296
const response = await this.codexConnection.prompt(params);
298297

299-
if (this.sessionState && response.usage) {
300-
// Accumulate token usage from the prompt response
301-
this.sessionState.accumulatedUsage.inputTokens +=
302-
response.usage.inputTokens ?? 0;
303-
this.sessionState.accumulatedUsage.outputTokens +=
304-
response.usage.outputTokens ?? 0;
305-
this.sessionState.accumulatedUsage.cachedReadTokens +=
306-
response.usage.cachedReadTokens ?? 0;
307-
this.sessionState.accumulatedUsage.cachedWriteTokens +=
308-
response.usage.cachedWriteTokens ?? 0;
309-
}
298+
// Usage is already accumulated via sessionUpdate notifications in
299+
// codex-client.ts. Do NOT also add response.usage here or tokens
300+
// get double-counted.
310301

311-
if (this.sessionState?.taskRunId) {
302+
if (this.sessionState.taskRunId) {
312303
const { accumulatedUsage } = this.sessionState;
313304

314305
await this.client.extNotification(POSTHOG_NOTIFICATIONS.TURN_COMPLETE, {
@@ -328,7 +319,7 @@ export class CodexAcpAgent extends BaseAcpAgent {
328319
});
329320

330321
if (response.usage) {
331-
await this.client.extNotification("_posthog/usage_update", {
322+
await this.client.extNotification(POSTHOG_NOTIFICATIONS.USAGE_UPDATE, {
332323
sessionId: params.sessionId,
333324
used: {
334325
inputTokens: response.usage.inputTokens ?? 0,
@@ -345,47 +336,32 @@ export class CodexAcpAgent extends BaseAcpAgent {
345336
}
346337

347338
protected async interrupt(): Promise<void> {
348-
if (this.sessionState) {
349-
this.sessionState.cancelled = true;
350-
}
351339
await this.codexConnection.cancel({
352340
sessionId: this.sessionId,
353341
});
354342
}
355343

356-
async cancel(params: CancelNotification): Promise<void> {
357-
if (this.sessionState) {
358-
this.sessionState.cancelled = true;
359-
const meta = params._meta as { interruptReason?: string } | undefined;
360-
if (meta?.interruptReason) {
361-
this.sessionState.interruptReason = meta.interruptReason;
362-
}
363-
}
364-
await this.codexConnection.cancel(params);
365-
}
366-
367344
async setSessionMode(
368345
params: SetSessionModeRequest,
369346
): Promise<SetSessionModeResponse> {
370-
const permissionMode = toPermissionMode(params.modeId);
347+
const requestedMode = toCodeExecutionMode(params.modeId);
348+
const nativeMode = CODEX_NATIVE_MODE[requestedMode];
371349

372350
const response = await this.codexConnection.setSessionMode({
373351
...params,
374-
modeId: permissionMode,
352+
modeId: nativeMode,
375353
});
376354

377-
if (this.sessionState) {
378-
this.sessionState.modeId = permissionMode;
379-
this.sessionState.permissionMode = permissionMode;
380-
}
355+
this.sessionState.modeId = nativeMode;
356+
this.sessionState.permissionMode = requestedMode;
381357
return response ?? {};
382358
}
383359

384360
async setSessionConfigOption(
385361
params: SetSessionConfigOptionRequest,
386362
): Promise<SetSessionConfigOptionResponse> {
387363
const response = await this.codexConnection.setSessionConfigOption(params);
388-
if (this.sessionState && response.configOptions) {
364+
if (response.configOptions) {
389365
this.sessionState.configOptions = response.configOptions;
390366
}
391367
return response;
@@ -397,6 +373,7 @@ export class CodexAcpAgent extends BaseAcpAgent {
397373

398374
async closeSession(): Promise<void> {
399375
this.logger.info("Closing Codex session", { sessionId: this.sessionId });
376+
this.session.abortController.abort();
400377
this.session.settingsManager.dispose();
401378
try {
402379
this.codexProcess.kill();

packages/agent/src/adapters/codex/codex-client.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import type {
2929
WriteTextFileRequest,
3030
WriteTextFileResponse,
3131
} from "@agentclientprotocol/sdk";
32-
import type { PermissionMode } from "../../execution-mode";
32+
import type { CodeExecutionMode } from "../../execution-mode";
3333
import type { Logger } from "../../utils/logger";
3434
import type { CodexSessionState } from "./session-state";
3535

@@ -38,10 +38,11 @@ export interface CodexClientCallbacks {
3838
onUsageUpdate?: (update: Record<string, unknown>) => void;
3939
}
4040

41-
const AUTO_APPROVED_KINDS: Partial<Record<PermissionMode, Set<ToolKind>>> = {
42-
auto: new Set(["read", "search", "fetch", "think"]),
43-
"read-only": new Set(["read", "search", "fetch", "think"]),
44-
"full-access": new Set([
41+
const AUTO_APPROVED_KINDS: Record<CodeExecutionMode, Set<ToolKind>> = {
42+
default: new Set(["read", "search", "fetch", "think"]),
43+
acceptEdits: new Set(["read", "edit", "search", "fetch", "think"]),
44+
plan: new Set(["read", "search", "fetch", "think"]),
45+
bypassPermissions: new Set([
4546
"read",
4647
"edit",
4748
"delete",
@@ -56,10 +57,10 @@ const AUTO_APPROVED_KINDS: Partial<Record<PermissionMode, Set<ToolKind>>> = {
5657
};
5758

5859
function shouldAutoApprove(
59-
mode: PermissionMode,
60+
mode: CodeExecutionMode,
6061
kind: ToolKind | null | undefined,
6162
): boolean {
62-
if (mode === "full-access") return true;
63+
if (mode === "bypassPermissions") return true;
6364
if (!kind) return false;
6465
return AUTO_APPROVED_KINDS[mode]?.has(kind) ?? false;
6566
}

packages/agent/src/adapters/codex/session-state.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55

66
import type { SessionConfigOption } from "@agentclientprotocol/sdk";
7-
import type { PermissionMode } from "../../execution-mode";
7+
import type { CodeExecutionMode } from "../../execution-mode";
88

99
export interface CodexUsage {
1010
inputTokens: number;
@@ -22,9 +22,7 @@ export interface CodexSessionState {
2222
accumulatedUsage: CodexUsage;
2323
contextSize?: number;
2424
contextUsed?: number;
25-
permissionMode: PermissionMode;
26-
cancelled: boolean;
27-
interruptReason?: string;
25+
permissionMode: CodeExecutionMode;
2826
taskRunId?: string;
2927
taskId?: string;
3028
}
@@ -37,13 +35,13 @@ export function createSessionState(
3735
taskId?: string;
3836
modeId?: string;
3937
modelId?: string;
40-
permissionMode?: PermissionMode;
38+
permissionMode?: CodeExecutionMode;
4139
},
4240
): CodexSessionState {
4341
return {
4442
sessionId,
4543
cwd,
46-
modeId: opts?.modeId ?? "auto",
44+
modeId: opts?.modeId ?? "default",
4745
modelId: opts?.modelId,
4846
configOptions: [],
4947
accumulatedUsage: {
@@ -52,8 +50,7 @@ export function createSessionState(
5250
cachedReadTokens: 0,
5351
cachedWriteTokens: 0,
5452
},
55-
permissionMode: opts?.permissionMode ?? "auto",
56-
cancelled: false,
53+
permissionMode: opts?.permissionMode ?? "default",
5754
taskRunId: opts?.taskRunId,
5855
taskId: opts?.taskId,
5956
};

packages/agent/src/adapters/codex/spawn.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ function buildConfigArgs(options: CodexProcessOptions): string[] {
4646
if (options.instructions) {
4747
const escaped = options.instructions
4848
.replace(/\\/g, "\\\\")
49+
.replace(/\n/g, "\\n")
50+
.replace(/\r/g, "\\r")
4951
.replace(/"/g, '\\"');
5052
args.push("-c", `instructions="${escaped}"`);
5153
}

0 commit comments

Comments
 (0)