Skip to content

Commit 77cc1b0

Browse files
Harden plan and goal session state
Restore collaboration mode when sessions are loaded and publish authoritative goal snapshots without allowing stale reads to overwrite live updates. Keep goal control responses typed and preserve stable goal identity for UI replay.
1 parent cd13da3 commit 77cc1b0

19 files changed

Lines changed: 348 additions & 62 deletions

src/CodexAcpClient.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,7 @@ export class CodexAcpClient {
336336
sessionId: request.sessionId,
337337
currentModelId: currentModelId,
338338
models: codexModels,
339+
collaborationMode: this.getCollaborationMode(response.thread.id),
339340
modelProvider: response.modelProvider,
340341
currentServiceTier: response.serviceTier as ServiceTier ?? null,
341342
additionalDirectories,
@@ -363,6 +364,7 @@ export class CodexAcpClient {
363364
sessionId: request.sessionId,
364365
currentModelId: currentModelId,
365366
models: codexModels,
367+
collaborationMode: this.getCollaborationMode(response.thread.id),
366368
modelProvider: response.modelProvider,
367369
currentServiceTier: response.serviceTier as ServiceTier ?? null,
368370
thread: historyResponse.thread,
@@ -389,6 +391,7 @@ export class CodexAcpClient {
389391
sessionId: response.thread.id,
390392
currentModelId: currentModelId,
391393
models: codexModels,
394+
collaborationMode: this.getCollaborationMode(response.thread.id),
392395
modelProvider: response.modelProvider,
393396
currentServiceTier: response.serviceTier as ServiceTier ?? null,
394397
additionalDirectories,
@@ -440,11 +443,18 @@ export class CodexAcpClient {
440443
}, onTurnStarted);
441444
}
442445

443-
async setGoalStatus(sessionId: string, status: ThreadGoalStatus): Promise<void> {
446+
async setGoalStatus(sessionId: string, status: ThreadGoalStatus): Promise<ThreadGoal> {
447+
let updatedGoal: ThreadGoal | null = null;
444448
await this.codexClient.runGoalSet({
445449
threadId: sessionId,
446450
status,
451+
}, undefined, undefined, (goal) => {
452+
updatedGoal = goal;
447453
});
454+
if (updatedGoal === null) {
455+
throw new Error(`Goal update for session ${sessionId} returned no goal`);
456+
}
457+
return updatedGoal;
448458
}
449459

450460
async resumeGoal(
@@ -697,6 +707,10 @@ export class CodexAcpClient {
697707
});
698708
}
699709

710+
private getCollaborationMode(sessionId: string): ModeKind {
711+
return this.codexClient.getThreadSettings(sessionId)?.collaborationMode.mode ?? "default";
712+
}
713+
700714
resolveTurnInterrupted(params: { threadId: string, turnId: string }): void {
701715
this.codexClient.resolveTurnInterrupted(params.threadId, params.turnId);
702716
}
@@ -863,6 +877,7 @@ export type SessionMetadata = {
863877
sessionId: string,
864878
currentModelId: string,
865879
models: Model[],
880+
collaborationMode: ModeKind,
866881
modelProvider?: string | null,
867882
currentServiceTier?: ServiceTier | null,
868883
additionalDirectories: string[],

src/CodexAcpServer.ts

Lines changed: 66 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import type {
1313
Model,
1414
ReasoningEffortOption,
1515
Thread,
16-
ThreadGoalStatus,
1716
ThreadItem,
1817
UserInput
1918
} from "./app-server/v2";
@@ -23,7 +22,6 @@ import {AgentMode, MODE_CONFIG_ID} from "./AgentMode";
2322
import {
2423
COLLABORATION_MODE_CONFIG_ID,
2524
createCollaborationModeConfigOption,
26-
DEFAULT_COLLABORATION_MODE,
2725
parseCollaborationMode,
2826
} from "./CollaborationModeConfig";
2927
import type {ModeKind} from "./app-server/ModeKind";
@@ -82,14 +80,11 @@ import {
8280
createAgentTextThoughtChunk,
8381
createUserMessageChunk,
8482
} from "./ContentChunks";
85-
86-
export interface ThreadGoalSnapshot {
87-
objective: string;
88-
status: ThreadGoalStatus;
89-
tokenBudget: number | null;
90-
timeUsedSeconds: number;
91-
controlMethod: typeof GOAL_CONTROL_METHOD;
92-
}
83+
import {
84+
sameThreadGoalSnapshot,
85+
type ThreadGoalSnapshot,
86+
toThreadGoalSnapshot,
87+
} from "./ThreadGoalSnapshot";
9388

9489
export interface SessionState {
9590
sessionId: string,
@@ -114,6 +109,7 @@ export interface SessionState {
114109
sessionMcpServers?: Array<string>;
115110
terminalOutputMode: TerminalOutputMode;
116111
currentGoal?: ThreadGoalSnapshot | null;
112+
goalRevision: number;
117113
sessionTitle: string | null;
118114
sessionTitleSource: "unset" | "fallback" | "explicit" | "unknown";
119115
}
@@ -263,10 +259,17 @@ export class CodexAcpServer {
263259
if (!sessionState) {
264260
throw RequestError.invalidParams(undefined, `Unknown session: ${methodRequest.params.sessionId}`);
265261
}
262+
const sessionGeneration = this.getSessionGeneration(sessionState.sessionId);
266263
if (methodRequest.params.action === "pause") {
267-
await this.runWithProcessCheck(() => this.codexAcpClient.setGoalStatus(sessionState.sessionId, "paused"));
268-
} else {
264+
const goal = await this.runWithProcessCheck(() => this.codexAcpClient.setGoalStatus(sessionState.sessionId, "paused"));
265+
if (this.goalPublishIsCurrent(sessionState, sessionGeneration)) {
266+
await this.publishGoalSnapshot(sessionState, toThreadGoalSnapshot(goal), false);
267+
}
268+
} else if (methodRequest.params.action === "clear") {
269269
await this.runWithProcessCheck(() => this.codexAcpClient.clearGoal(sessionState.sessionId));
270+
if (this.goalPublishIsCurrent(sessionState, sessionGeneration)) {
271+
await this.publishGoalSnapshot(sessionState, null, false);
272+
}
270273
}
271274
return {};
272275
}
@@ -428,7 +431,7 @@ export class CodexAcpServer {
428431
supportedReasoningEfforts: currentModel?.supportedReasoningEfforts ?? [],
429432
supportedInputModalities: currentModel?.inputModalities ?? ["text", "image"],
430433
agentMode: AgentMode.getInitialAgentMode(),
431-
collaborationMode: DEFAULT_COLLABORATION_MODE,
434+
collaborationMode: sessionMetadata.collaborationMode,
432435
currentTurnId: null,
433436
lastTokenUsage: null,
434437
totalTokenUsage: null,
@@ -443,6 +446,7 @@ export class CodexAcpServer {
443446
currentModelSupportsFast: currentModelSupportsFast,
444447
sessionMcpServers: sessionMcpServers,
445448
terminalOutputMode: this.terminalOutputMode,
449+
goalRevision: 0,
446450
sessionTitle: null,
447451
sessionTitleSource: "sessionId" in request ? "unknown" : "unset",
448452
};
@@ -459,7 +463,7 @@ export class CodexAcpServer {
459463

460464
this.publishAvailableCommandsAsync(sessionState);
461465
if ("sessionId" in request) {
462-
this.publishCurrentGoalAsync(sessionState);
466+
this.publishCurrentGoalAsync(sessionState, sessionGeneration);
463467
}
464468
const sessionModelState: LegacySessionModelState = this.createModelState(models, currentModelId);
465469
const sessionModeState: SessionModeState = sessionState.agentMode.toSessionModeState();
@@ -897,21 +901,55 @@ export class CodexAcpServer {
897901
void this.availableCommands.publish(sessionState);
898902
}
899903

900-
private publishCurrentGoalAsync(sessionState: SessionState): void {
901-
void this.publishCurrentGoal(sessionState).catch((err) => {
904+
private publishCurrentGoalAsync(sessionState: SessionState, sessionGeneration: number): void {
905+
void this.publishCurrentGoalBestEffort(sessionState, sessionGeneration, true);
906+
}
907+
908+
private async publishCurrentGoalBestEffort(
909+
sessionState: SessionState,
910+
sessionGeneration: number,
911+
force: boolean,
912+
): Promise<void> {
913+
try {
914+
await this.publishCurrentGoal(sessionState, sessionGeneration, force);
915+
} catch (err) {
902916
logger.error(`Failed to publish current goal for session ${sessionState.sessionId}`, err);
903-
});
917+
}
904918
}
905919

906-
private async publishCurrentGoal(sessionState: SessionState): Promise<void> {
920+
private async publishCurrentGoal(
921+
sessionState: SessionState,
922+
sessionGeneration: number,
923+
force: boolean,
924+
): Promise<void> {
925+
const requestRevision = ++sessionState.goalRevision;
907926
const goal = await this.runWithProcessCheck(() => this.codexAcpClient.getGoal(sessionState.sessionId));
908-
const snapshot: ThreadGoalSnapshot | null = goal === null ? null : {
909-
objective: goal.objective.trim(),
910-
status: goal.status,
911-
tokenBudget: goal.tokenBudget,
912-
timeUsedSeconds: goal.timeUsedSeconds,
913-
controlMethod: GOAL_CONTROL_METHOD,
914-
};
927+
const snapshot = goal === null ? null : toThreadGoalSnapshot(goal);
928+
if (!this.goalPublishIsCurrent(sessionState, sessionGeneration)
929+
|| sessionState.goalRevision !== requestRevision) {
930+
return;
931+
}
932+
await this.publishGoalSnapshot(sessionState, snapshot, force, false);
933+
}
934+
935+
private goalPublishIsCurrent(sessionState: SessionState, sessionGeneration: number): boolean {
936+
return this.sessions.get(sessionState.sessionId) === sessionState
937+
&& this.getSessionGeneration(sessionState.sessionId) === sessionGeneration
938+
&& !this.sessionIsClosing(sessionState.sessionId);
939+
}
940+
941+
private async publishGoalSnapshot(
942+
sessionState: SessionState,
943+
snapshot: ThreadGoalSnapshot | null,
944+
force: boolean,
945+
incrementRevision = true,
946+
): Promise<void> {
947+
if (incrementRevision) {
948+
sessionState.goalRevision += 1;
949+
}
950+
if (!force && sameThreadGoalSnapshot(sessionState.currentGoal, snapshot)) {
951+
return;
952+
}
915953
sessionState.currentGoal = snapshot;
916954
const session = new ACPSessionConnection(this.connection, sessionState.sessionId);
917955
await session.update({
@@ -1007,7 +1045,7 @@ export class CodexAcpServer {
10071045
supportedReasoningEfforts: currentModel?.supportedReasoningEfforts ?? [],
10081046
supportedInputModalities: currentModel?.inputModalities ?? ["text", "image"],
10091047
agentMode: AgentMode.getInitialAgentMode(),
1010-
collaborationMode: DEFAULT_COLLABORATION_MODE,
1048+
collaborationMode: sessionMetadata.collaborationMode,
10111049
currentTurnId: null,
10121050
lastTokenUsage: null,
10131051
totalTokenUsage: null,
@@ -1022,6 +1060,7 @@ export class CodexAcpServer {
10221060
currentModelSupportsFast: currentModelSupportsFast,
10231061
sessionMcpServers: sessionMcpServers,
10241062
terminalOutputMode: this.terminalOutputMode,
1063+
goalRevision: 0,
10251064
sessionTitle: null,
10261065
sessionTitleSource: "unset",
10271066
};
@@ -1037,7 +1076,7 @@ export class CodexAcpServer {
10371076
}
10381077

10391078
await this.availableCommands.publish(sessionState);
1040-
await this.publishCurrentGoal(sessionState);
1079+
await this.publishCurrentGoalBestEffort(sessionState, requestedSessionGeneration, true);
10411080
const sessionModelState: LegacySessionModelState = this.createModelState(models, currentModelId);
10421081
const sessionModeState: SessionModeState = sessionState.agentMode.toSessionModeState();
10431082

src/CodexAppServerClient.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import type {
4949
ThreadReadResponse,
5050
ThreadResumeParams,
5151
ThreadResumeResponse,
52+
ThreadSettings,
5253
ThreadStartParams,
5354
ThreadStartResponse,
5455
ThreadUnsubscribeParams,
@@ -141,6 +142,7 @@ export class CodexAppServerClient {
141142
private readonly threadStatusCaptures = new Map<string, Set<(status: ThreadStatus) => void>>();
142143
private readonly threadGoalUpdateCaptures = new Map<string, Set<(event: ThreadGoalUpdatedNotification) => void>>();
143144
private readonly threadGoalClearedCaptures = new Map<string, Set<() => void>>();
145+
private readonly threadSettings = new Map<string, ThreadSettings>();
144146
private readonly staleTurnIds = new Map<string, Set<string>>();
145147

146148
constructor(connection: MessageConnection) {
@@ -171,6 +173,9 @@ export class CodexAppServerClient {
171173
if (isThreadGoalClearedNotification(serverNotification)) {
172174
this.recordThreadGoalCleared(serverNotification.params);
173175
}
176+
if (serverNotification.method === "thread/settings/updated") {
177+
this.threadSettings.set(serverNotification.params.threadId, serverNotification.params.threadSettings);
178+
}
174179
const routing = extractTurnRouting(serverNotification);
175180
if (this.handleStaleTurnNotification(serverNotification, routing)) {
176181
return;
@@ -312,6 +317,7 @@ export class CodexAppServerClient {
312317
params: ThreadGoalSetParams,
313318
onTurnStarted?: (turnId: string) => void,
314319
runtimeEffectsGraceMs = GOAL_RUNTIME_EFFECTS_GRACE_MS,
320+
onGoalSet?: (goal: ThreadGoal) => void,
315321
): Promise<TurnCompletedNotification | null> {
316322
let goalTurnId: string | null = null;
317323
const capturedCompletions: Array<TurnCompletedNotification> = [];
@@ -363,6 +369,7 @@ export class CodexAppServerClient {
363369
try {
364370
const goalSetResponse = await this.threadGoalSet(params);
365371
expectedGoal = goalSetResponse.goal;
372+
onGoalSet?.(expectedGoal);
366373
if (capturedGoalUpdates.some(event => goalsMatch(event.goal, expectedGoal!))) {
367374
goalUpdateHandled = true;
368375
resolveGoalUpdateHandled();
@@ -515,6 +522,10 @@ export class CodexAppServerClient {
515522
return await this.sendRequest({ method: "thread/resume", params: params });
516523
}
517524

525+
getThreadSettings(threadId: string): ThreadSettings | undefined {
526+
return this.threadSettings.get(threadId);
527+
}
528+
518529
async threadSettingsUpdate(params: ExperimentalThreadSettingsUpdateParams): Promise<void> {
519530
await this.connection.sendRequest("thread/settings/update", params);
520531
}

src/CodexCommands.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,10 @@ export class CodexCommands {
204204
await this.sendCommandUsageMessage(commandName, "no arguments", sessionId);
205205
return { handled: true };
206206
}
207-
await options.setConfigOption?.(COLLABORATION_MODE_CONFIG_ID, PLAN_COLLABORATION_MODE);
207+
const mode = sessionState.collaborationMode === PLAN_COLLABORATION_MODE
208+
? DEFAULT_COLLABORATION_MODE
209+
: PLAN_COLLABORATION_MODE;
210+
await options.setConfigOption?.(COLLABORATION_MODE_CONFIG_ID, mode);
208211
return { handled: options.setConfigOption !== undefined };
209212
}
210213
case "compact": {

src/CodexEventHandler.ts

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ import type {
33
FuzzyFileSearchSessionUpdatedNotification,
44
ServerNotification
55
} from "./app-server";
6-
import type {SessionState, ThreadGoalSnapshot} from "./CodexAcpServer";
7-
import {GOAL_CONTROL_METHOD} from "./AcpExtensions";
6+
import type {SessionState} from "./CodexAcpServer";
87
import {type PlanEntry, RequestError} from "@agentclientprotocol/sdk";
98
import {ACPSessionConnection, type AcpClientConnection, type UpdateSessionEvent} from "./ACPSessionConnection";
109
import type {
@@ -63,6 +62,7 @@ import {
6362
createAgentTextMessageChunk,
6463
createAgentTextThoughtChunk,
6564
} from "./ContentChunks";
65+
import {sameThreadGoalSnapshot, toThreadGoalSnapshot} from "./ThreadGoalSnapshot";
6666

6767
export { stripShellPrefix };
6868

@@ -257,8 +257,9 @@ export class CodexEventHandler {
257257
}
258258

259259
private createThreadGoalUpdatedEvent(event: ThreadGoalUpdatedNotification): UpdateSessionEvent | null {
260-
const goalSnapshot = this.createThreadGoalSnapshot(event);
261-
if (this.sameThreadGoalSnapshot(this.sessionState.currentGoal, goalSnapshot)) {
260+
this.sessionState.goalRevision += 1;
261+
const goalSnapshot = toThreadGoalSnapshot(event.goal);
262+
if (sameThreadGoalSnapshot(this.sessionState.currentGoal, goalSnapshot)) {
262263
return null;
263264
}
264265
this.sessionState.currentGoal = goalSnapshot;
@@ -269,6 +270,7 @@ export class CodexEventHandler {
269270
}
270271

271272
private createThreadGoalClearedEvent(_event: ThreadGoalClearedNotification): UpdateSessionEvent | null {
273+
this.sessionState.goalRevision += 1;
272274
if (this.sessionState.currentGoal === null) {
273275
return null;
274276
}
@@ -279,27 +281,6 @@ export class CodexEventHandler {
279281
});
280282
}
281283

282-
private createThreadGoalSnapshot(event: ThreadGoalUpdatedNotification): ThreadGoalSnapshot {
283-
return {
284-
objective: event.goal.objective.trim(),
285-
status: event.goal.status,
286-
tokenBudget: event.goal.tokenBudget,
287-
timeUsedSeconds: event.goal.timeUsedSeconds,
288-
controlMethod: GOAL_CONTROL_METHOD,
289-
};
290-
}
291-
292-
private sameThreadGoalSnapshot(
293-
left: ThreadGoalSnapshot | null | undefined,
294-
right: ThreadGoalSnapshot
295-
): boolean {
296-
return left !== null
297-
&& left !== undefined
298-
&& left.objective === right.objective
299-
&& left.status === right.status
300-
&& left.tokenBudget === right.tokenBudget;
301-
}
302-
303284
private createReasoningDeltaEvent(
304285
event: ReasoningSummaryTextDeltaNotification | ReasoningTextDeltaNotification
305286
): UpdateSessionEvent {

0 commit comments

Comments
 (0)