mirror of
https://github.com/getpaseo/paseo.git
synced 2026-07-29 12:01:31 +00:00
Keep plan approval focused on the latest proposal (#2534)
* fix(server): replace stale plan approvals Synthetic plan approvals accumulated across planning turns because each proposal used a new permission ID. Dismiss the current proposal only after a follow-up prompt is accepted, and enforce one pending proposal when a newer plan arrives. * fix(server): close plan approval submission race Deactivate the existing proposal before starting a revision so it cannot be implemented concurrently. Restore it when submission fails, while preserving any newer approval emitted during the request. * fix(server): deactivate plans before prompt setup Start the plan-dismissal transaction before any asynchronous prompt setup so a stale proposal cannot be implemented from another client while the revision is preparing. * fix(server): make prompt dismissal final Treat prompt submission as the dismissal event instead of compensating after failures. This avoids resurrecting stale plans across ambiguous provider outcomes and manager failure cleanup.
This commit is contained in:
@@ -3914,8 +3914,8 @@ describe("Codex app-server provider", () => {
|
|||||||
},
|
},
|
||||||
actions: [
|
actions: [
|
||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
id: "reject",
|
id: "dismiss",
|
||||||
label: "Reject",
|
label: "Dismiss",
|
||||||
behavior: "deny",
|
behavior: "deny",
|
||||||
}),
|
}),
|
||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
@@ -3979,6 +3979,216 @@ describe("Codex app-server provider", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("replaces a pending synthetic plan approval when a later plan completes", () => {
|
||||||
|
const session = createSession({
|
||||||
|
featureValues: { plan_mode: true },
|
||||||
|
});
|
||||||
|
|
||||||
|
asInternals(session).handleNotification("turn/started", {
|
||||||
|
turn: { id: "turn-plan-first" },
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/plan/updated", {
|
||||||
|
plan: [{ step: "Implement the first plan", status: "pending" }],
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/completed", {
|
||||||
|
turn: { status: "completed", error: null },
|
||||||
|
});
|
||||||
|
|
||||||
|
asInternals(session).handleNotification("turn/started", {
|
||||||
|
turn: { id: "turn-plan-second" },
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/plan/updated", {
|
||||||
|
plan: [{ step: "Implement the revised plan", status: "pending" }],
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/completed", {
|
||||||
|
turn: { status: "completed", error: null },
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(session.getPendingPermissions()).toEqual([
|
||||||
|
expect.objectContaining({
|
||||||
|
kind: "plan",
|
||||||
|
input: { plan: "- Implement the revised plan" },
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("dismisses a pending synthetic plan approval after a new prompt is accepted", async () => {
|
||||||
|
const session = createSession({
|
||||||
|
featureValues: { plan_mode: true },
|
||||||
|
});
|
||||||
|
const events: AgentStreamEvent[] = [];
|
||||||
|
session.subscribe((event) => events.push(event));
|
||||||
|
|
||||||
|
asInternals(session).handleNotification("turn/started", {
|
||||||
|
turn: { id: "turn-plan-pending" },
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/plan/updated", {
|
||||||
|
plan: [{ step: "Implement the original plan", status: "pending" }],
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/completed", {
|
||||||
|
turn: { status: "completed", error: null },
|
||||||
|
});
|
||||||
|
const pendingPlan = session.getPendingPermissions()[0];
|
||||||
|
expect(pendingPlan).toBeDefined();
|
||||||
|
|
||||||
|
session.activeForegroundTurnId = null;
|
||||||
|
session.client = createStub<CodexClientLike>({
|
||||||
|
request: async (method) => {
|
||||||
|
if (method === "thread/loaded/list") return { data: ["test-thread"] };
|
||||||
|
if (method === "turn/start") return {};
|
||||||
|
throw new Error(`Unexpected request: ${method}`);
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await session.startTurn("Revise the plan to include tests");
|
||||||
|
|
||||||
|
expect(session.getPendingPermissions()).toEqual([]);
|
||||||
|
expect(events).toContainEqual({
|
||||||
|
type: "permission_resolved",
|
||||||
|
provider: "codex",
|
||||||
|
requestId: pendingPlan!.id,
|
||||||
|
resolution: {
|
||||||
|
behavior: "deny",
|
||||||
|
message: "Dismissed by a new prompt",
|
||||||
|
},
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test("makes the old plan non-actionable while a new prompt is being prepared", async () => {
|
||||||
|
const session = createSession({
|
||||||
|
featureValues: { plan_mode: true },
|
||||||
|
});
|
||||||
|
|
||||||
|
asInternals(session).handleNotification("turn/started", {
|
||||||
|
turn: { id: "turn-plan-pending" },
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/plan/updated", {
|
||||||
|
plan: [{ step: "Implement the original plan", status: "pending" }],
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/completed", {
|
||||||
|
turn: { status: "completed", error: null },
|
||||||
|
});
|
||||||
|
const pendingPlan = session.getPendingPermissions()[0];
|
||||||
|
expect(pendingPlan).toBeDefined();
|
||||||
|
|
||||||
|
let continuePromptSetup: (() => void) | undefined;
|
||||||
|
let markPromptSetupStarted: (() => void) | undefined;
|
||||||
|
const promptSetupStarted = new Promise<void>((resolve) => {
|
||||||
|
markPromptSetupStarted = resolve;
|
||||||
|
});
|
||||||
|
session.activeForegroundTurnId = null;
|
||||||
|
session.client = createStub<CodexClientLike>({
|
||||||
|
request: async (method) => {
|
||||||
|
if (method === "thread/loaded/list") {
|
||||||
|
markPromptSetupStarted?.();
|
||||||
|
await new Promise<void>((resolve) => {
|
||||||
|
continuePromptSetup = resolve;
|
||||||
|
});
|
||||||
|
return { data: ["test-thread"] };
|
||||||
|
}
|
||||||
|
if (method === "turn/start") return {};
|
||||||
|
throw new Error(`Unexpected request: ${method}`);
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const startTurn = session.startTurn("Revise the plan");
|
||||||
|
await promptSetupStarted;
|
||||||
|
|
||||||
|
expect(session.getPendingPermissions()).toEqual([]);
|
||||||
|
await expect(
|
||||||
|
session.respondToPermission(pendingPlan!.id, {
|
||||||
|
behavior: "allow",
|
||||||
|
selectedActionId: "implement",
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(
|
||||||
|
`No pending Codex app-server permission request with id '${pendingPlan!.id}'`,
|
||||||
|
);
|
||||||
|
|
||||||
|
continuePromptSetup?.();
|
||||||
|
await startTurn;
|
||||||
|
});
|
||||||
|
|
||||||
|
test("does not dismiss a new plan approval emitted while a prompt is being accepted", async () => {
|
||||||
|
const session = createSession({
|
||||||
|
featureValues: { plan_mode: true },
|
||||||
|
});
|
||||||
|
|
||||||
|
asInternals(session).handleNotification("turn/started", {
|
||||||
|
turn: { id: "turn-plan-pending" },
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/plan/updated", {
|
||||||
|
plan: [{ step: "Implement the original plan", status: "pending" }],
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/completed", {
|
||||||
|
turn: { status: "completed", error: null },
|
||||||
|
});
|
||||||
|
|
||||||
|
let acceptPrompt: (() => void) | undefined;
|
||||||
|
let markPromptRequested: (() => void) | undefined;
|
||||||
|
const promptRequested = new Promise<void>((resolve) => {
|
||||||
|
markPromptRequested = resolve;
|
||||||
|
});
|
||||||
|
session.activeForegroundTurnId = null;
|
||||||
|
session.client = createStub<CodexClientLike>({
|
||||||
|
request: async (method) => {
|
||||||
|
if (method === "thread/loaded/list") return { data: ["test-thread"] };
|
||||||
|
if (method === "turn/start") {
|
||||||
|
markPromptRequested?.();
|
||||||
|
return await new Promise<void>((resolve) => {
|
||||||
|
acceptPrompt = resolve;
|
||||||
|
});
|
||||||
|
}
|
||||||
|
throw new Error(`Unexpected request: ${method}`);
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const startTurn = session.startTurn("Revise the plan");
|
||||||
|
await promptRequested;
|
||||||
|
asInternals(session).handleNotification("turn/plan/updated", {
|
||||||
|
plan: [{ step: "Implement the newer plan", status: "pending" }],
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/completed", {
|
||||||
|
turn: { status: "completed", error: null },
|
||||||
|
});
|
||||||
|
acceptPrompt?.();
|
||||||
|
await startTurn;
|
||||||
|
|
||||||
|
expect(session.getPendingPermissions()).toEqual([
|
||||||
|
expect.objectContaining({ input: { plan: "- Implement the newer plan" } }),
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("keeps a synthetic plan dismissed when a new prompt is rejected", async () => {
|
||||||
|
const session = createSession({
|
||||||
|
featureValues: { plan_mode: true },
|
||||||
|
});
|
||||||
|
|
||||||
|
asInternals(session).handleNotification("turn/started", {
|
||||||
|
turn: { id: "turn-plan-pending" },
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/plan/updated", {
|
||||||
|
plan: [{ step: "Implement the original plan", status: "pending" }],
|
||||||
|
});
|
||||||
|
asInternals(session).handleNotification("turn/completed", {
|
||||||
|
turn: { status: "completed", error: null },
|
||||||
|
});
|
||||||
|
const pendingPlan = session.getPendingPermissions()[0];
|
||||||
|
expect(pendingPlan).toBeDefined();
|
||||||
|
|
||||||
|
session.activeForegroundTurnId = null;
|
||||||
|
session.client = createStub<CodexClientLike>({
|
||||||
|
request: async (method) => {
|
||||||
|
if (method === "thread/loaded/list") return { data: ["test-thread"] };
|
||||||
|
if (method === "turn/start") throw new Error("Prompt rejected");
|
||||||
|
throw new Error(`Unexpected request: ${method}`);
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(session.startTurn("Revise the plan")).rejects.toThrow("Prompt rejected");
|
||||||
|
|
||||||
|
expect(session.getPendingPermissions()).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
test("emits imageView paths with spaces as valid assistant markdown images", () => {
|
test("emits imageView paths with spaces as valid assistant markdown images", () => {
|
||||||
const session = createSession();
|
const session = createSession();
|
||||||
const events: AgentStreamEvent[] = [];
|
const events: AgentStreamEvent[] = [];
|
||||||
|
|||||||
@@ -963,8 +963,8 @@ function buildPlanPermissionActions(options?: {
|
|||||||
}): AgentPermissionAction[] {
|
}): AgentPermissionAction[] {
|
||||||
const actions: AgentPermissionAction[] = [
|
const actions: AgentPermissionAction[] = [
|
||||||
{
|
{
|
||||||
id: "reject",
|
id: "dismiss",
|
||||||
label: "Reject",
|
label: "Dismiss",
|
||||||
behavior: "deny",
|
behavior: "deny",
|
||||||
variant: "danger",
|
variant: "danger",
|
||||||
intent: "dismiss",
|
intent: "dismiss",
|
||||||
@@ -3092,6 +3092,13 @@ interface CodexSubAgentCallState {
|
|||||||
childThreadIds: Set<string>;
|
childThreadIds: Set<string>;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
interface CodexPendingPermissionHandler {
|
||||||
|
resolve: (value: unknown) => void;
|
||||||
|
kind: "command" | "file" | "question" | "mcp_elicitation" | "plan";
|
||||||
|
questions?: CodexQuestionPrompt[];
|
||||||
|
planText?: string;
|
||||||
|
}
|
||||||
|
|
||||||
export class CodexAppServerAgentSession implements AgentSession {
|
export class CodexAppServerAgentSession implements AgentSession {
|
||||||
readonly provider = CODEX_PROVIDER;
|
readonly provider = CODEX_PROVIDER;
|
||||||
readonly capabilities = CODEX_APP_SERVER_CAPABILITIES;
|
readonly capabilities = CODEX_APP_SERVER_CAPABILITIES;
|
||||||
@@ -3115,15 +3122,7 @@ export class CodexAppServerAgentSession implements AgentSession {
|
|||||||
private persistedProviderSubagentEvents: AgentStreamEvent[] = [];
|
private persistedProviderSubagentEvents: AgentStreamEvent[] = [];
|
||||||
private pendingPermissions = new Map<string, AgentPermissionRequest>();
|
private pendingPermissions = new Map<string, AgentPermissionRequest>();
|
||||||
private mcpElicitationPermissionIds = new Map<number, string>();
|
private mcpElicitationPermissionIds = new Map<number, string>();
|
||||||
private pendingPermissionHandlers = new Map<
|
private pendingPermissionHandlers = new Map<string, CodexPendingPermissionHandler>();
|
||||||
string,
|
|
||||||
{
|
|
||||||
resolve: (value: unknown) => void;
|
|
||||||
kind: "command" | "file" | "question" | "mcp_elicitation" | "plan";
|
|
||||||
questions?: CodexQuestionPrompt[];
|
|
||||||
planText?: string;
|
|
||||||
}
|
|
||||||
>();
|
|
||||||
private resolvedPermissionRequests = new Set<string>();
|
private resolvedPermissionRequests = new Set<string>();
|
||||||
private pendingAgentMessages = new Map<string, string>();
|
private pendingAgentMessages = new Map<string, string>();
|
||||||
private pendingReasoning = new Map<string, string[]>();
|
private pendingReasoning = new Map<string, string[]>();
|
||||||
@@ -3430,6 +3429,8 @@ export class CodexAppServerAgentSession implements AgentSession {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private emitSyntheticPlanApprovalRequest(planText: string): void {
|
private emitSyntheticPlanApprovalRequest(planText: string): void {
|
||||||
|
this.dismissPendingPlanApprovals("Superseded by a newer plan");
|
||||||
|
|
||||||
const requestId = `permission-${randomUUID()}`;
|
const requestId = `permission-${randomUUID()}`;
|
||||||
const request: AgentPermissionRequest = {
|
const request: AgentPermissionRequest = {
|
||||||
id: requestId,
|
id: requestId,
|
||||||
@@ -3837,30 +3838,31 @@ export class CodexAppServerAgentSession implements AgentSession {
|
|||||||
throw new Error("A foreground turn is already active");
|
throw new Error("A foreground turn is already active");
|
||||||
}
|
}
|
||||||
|
|
||||||
await this.connect();
|
this.dismissPendingPlanApprovals("Dismissed by a new prompt");
|
||||||
if (!this.client) {
|
|
||||||
throw new Error("Codex client not initialized");
|
|
||||||
}
|
|
||||||
|
|
||||||
const slashCommand = await this.resolveSlashCommandInvocation(prompt);
|
|
||||||
const effectivePrompt = slashCommand
|
|
||||||
? await this.buildCommandPromptInput(slashCommand.commandName, slashCommand.args)
|
|
||||||
: prompt;
|
|
||||||
|
|
||||||
if (this.currentThreadId) {
|
|
||||||
await this.ensureThreadLoaded();
|
|
||||||
} else {
|
|
||||||
await this.ensureThread();
|
|
||||||
}
|
|
||||||
|
|
||||||
const turnStart = await this.buildTurnStartParams(effectivePrompt, options);
|
|
||||||
|
|
||||||
const turnId = this.createTurnId();
|
|
||||||
this.activeForegroundTurnId = turnId;
|
|
||||||
this.activeClientMessageId = options?.clientMessageId ?? null;
|
|
||||||
this.currentTurnId = null;
|
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
await this.connect();
|
||||||
|
if (!this.client) {
|
||||||
|
throw new Error("Codex client not initialized");
|
||||||
|
}
|
||||||
|
|
||||||
|
const slashCommand = await this.resolveSlashCommandInvocation(prompt);
|
||||||
|
const effectivePrompt = slashCommand
|
||||||
|
? await this.buildCommandPromptInput(slashCommand.commandName, slashCommand.args)
|
||||||
|
: prompt;
|
||||||
|
|
||||||
|
if (this.currentThreadId) {
|
||||||
|
await this.ensureThreadLoaded();
|
||||||
|
} else {
|
||||||
|
await this.ensureThread();
|
||||||
|
}
|
||||||
|
|
||||||
|
const turnStart = await this.buildTurnStartParams(effectivePrompt, options);
|
||||||
|
const turnId = this.createTurnId();
|
||||||
|
this.activeForegroundTurnId = turnId;
|
||||||
|
this.activeClientMessageId = options?.clientMessageId ?? null;
|
||||||
|
this.currentTurnId = null;
|
||||||
|
|
||||||
this.logTurnStartSummary({
|
this.logTurnStartSummary({
|
||||||
turnId,
|
turnId,
|
||||||
thinkingOptionId: turnStart.thinkingOptionId,
|
thinkingOptionId: turnStart.thinkingOptionId,
|
||||||
@@ -3871,13 +3873,12 @@ export class CodexAppServerAgentSession implements AgentSession {
|
|||||||
hasCodexConfig: turnStart.hasCodexConfig,
|
hasCodexConfig: turnStart.hasCodexConfig,
|
||||||
});
|
});
|
||||||
await this.client.request("turn/start", turnStart.params, TURN_START_TIMEOUT_MS);
|
await this.client.request("turn/start", turnStart.params, TURN_START_TIMEOUT_MS);
|
||||||
|
return { turnId };
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
this.activeForegroundTurnId = null;
|
this.activeForegroundTurnId = null;
|
||||||
this.activeClientMessageId = null;
|
this.activeClientMessageId = null;
|
||||||
throw error;
|
throw error;
|
||||||
}
|
}
|
||||||
|
|
||||||
return { turnId };
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private rememberCodexUserMessageTurn(messageId: string | null | undefined): boolean {
|
private rememberCodexUserMessageTurn(messageId: string | null | undefined): boolean {
|
||||||
@@ -4128,12 +4129,7 @@ export class CodexAppServerAgentSession implements AgentSession {
|
|||||||
private handlePlanPermissionResponse(params: {
|
private handlePlanPermissionResponse(params: {
|
||||||
requestId: string;
|
requestId: string;
|
||||||
response: AgentPermissionResponse;
|
response: AgentPermissionResponse;
|
||||||
pending: {
|
pending: CodexPendingPermissionHandler;
|
||||||
resolve: (value: unknown) => void;
|
|
||||||
kind: "command" | "file" | "question" | "mcp_elicitation" | "plan";
|
|
||||||
questions?: CodexQuestionPrompt[];
|
|
||||||
planText?: string;
|
|
||||||
};
|
|
||||||
pendingRequest: AgentPermissionRequest | null;
|
pendingRequest: AgentPermissionRequest | null;
|
||||||
}): AgentPermissionResult | void {
|
}): AgentPermissionResult | void {
|
||||||
const { requestId, response, pending, pendingRequest } = params;
|
const { requestId, response, pending, pendingRequest } = params;
|
||||||
@@ -4144,6 +4140,23 @@ export class CodexAppServerAgentSession implements AgentSession {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
this.resolvePlanPermission(requestId, response);
|
||||||
|
if (followUpPrompt) {
|
||||||
|
return { followUpPrompt };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private dismissPendingPlanApprovals(message: string): void {
|
||||||
|
const requestIds = Array.from(this.pendingPermissionHandlers)
|
||||||
|
.filter(([, pending]) => pending.kind === "plan")
|
||||||
|
.map(([requestId]) => requestId);
|
||||||
|
|
||||||
|
for (const requestId of requestIds) {
|
||||||
|
this.resolvePlanPermission(requestId, { behavior: "deny", message });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private resolvePlanPermission(requestId: string, resolution: AgentPermissionResponse): void {
|
||||||
this.pendingPermissionHandlers.delete(requestId);
|
this.pendingPermissionHandlers.delete(requestId);
|
||||||
this.pendingPermissions.delete(requestId);
|
this.pendingPermissions.delete(requestId);
|
||||||
this.resolvedPermissionRequests.add(requestId);
|
this.resolvedPermissionRequests.add(requestId);
|
||||||
@@ -4151,11 +4164,8 @@ export class CodexAppServerAgentSession implements AgentSession {
|
|||||||
type: "permission_resolved",
|
type: "permission_resolved",
|
||||||
provider: CODEX_PROVIDER,
|
provider: CODEX_PROVIDER,
|
||||||
requestId,
|
requestId,
|
||||||
resolution: response,
|
resolution,
|
||||||
});
|
});
|
||||||
if (followUpPrompt) {
|
|
||||||
return { followUpPrompt };
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private emitDeniedToolCallTimelineEvent(params: {
|
private emitDeniedToolCallTimelineEvent(params: {
|
||||||
|
|||||||
Reference in New Issue
Block a user