Fix Codex MCP resume conversation metadata
This commit is contained in:
@@ -263,6 +263,16 @@ function isUnsupportedChatGptModelError(error: unknown): boolean {
|
|||||||
return message.includes("model is not supported when using Codex with a ChatGPT account");
|
return message.includes("model is not supported when using Codex with a ChatGPT account");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isMissingConversationIdError(error: unknown): boolean {
|
||||||
|
const message = error instanceof Error ? error.message : String(error);
|
||||||
|
return message.includes("Session not found for conversation_id");
|
||||||
|
}
|
||||||
|
|
||||||
|
function isMissingConversationIdResponse(response: unknown): boolean {
|
||||||
|
const text = extractTextContent(response);
|
||||||
|
return !!text && text.includes("Session not found for conversation_id");
|
||||||
|
}
|
||||||
|
|
||||||
function extractCommandText(command?: unknown): string | null {
|
function extractCommandText(command?: unknown): string | null {
|
||||||
if (typeof command === "string") {
|
if (typeof command === "string") {
|
||||||
return command;
|
return command;
|
||||||
@@ -339,6 +349,7 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
private resolvedPermissionRequests = new Set<string>();
|
private resolvedPermissionRequests = new Set<string>();
|
||||||
private eventQueue: Pushable<AgentStreamEvent | ProviderEvent> | null = null;
|
private eventQueue: Pushable<AgentStreamEvent | ProviderEvent> | null = null;
|
||||||
private currentAbortController: AbortController | null = null;
|
private currentAbortController: AbortController | null = null;
|
||||||
|
private lockConversationId = false;
|
||||||
private historyPending = false;
|
private historyPending = false;
|
||||||
private persistedHistory: AgentTimelineItem[] = [];
|
private persistedHistory: AgentTimelineItem[] = [];
|
||||||
private pendingHistory: AgentTimelineItem[] = [];
|
private pendingHistory: AgentTimelineItem[] = [];
|
||||||
@@ -354,9 +365,10 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
const metadata = resumeHandle.metadata;
|
const metadata = resumeHandle.metadata;
|
||||||
if (metadata && typeof metadata === "object") {
|
if (metadata && typeof metadata === "object") {
|
||||||
const record = metadata as Record<string, unknown>;
|
const record = metadata as Record<string, unknown>;
|
||||||
const conversationId = record.conversationId;
|
const conversationId = record.conversationId ?? record.conversation_id;
|
||||||
if (typeof conversationId === "string") {
|
if (typeof conversationId === "string") {
|
||||||
this.conversationId = conversationId;
|
this.conversationId = conversationId;
|
||||||
|
this.lockConversationId = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
const history = this.sessionId ? SESSION_HISTORY.get(this.sessionId) : undefined;
|
const history = this.sessionId ? SESSION_HISTORY.get(this.sessionId) : undefined;
|
||||||
@@ -634,10 +646,7 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
|
|
||||||
describePersistence(): AgentPersistenceHandle | null {
|
describePersistence(): AgentPersistenceHandle | null {
|
||||||
if (this.persistence) {
|
if (this.persistence) {
|
||||||
const conversationId = this.conversationId ?? this.sessionId ?? undefined;
|
this.updatePersistenceConversationId();
|
||||||
if (conversationId && this.persistence.metadata && typeof this.persistence.metadata === "object") {
|
|
||||||
(this.persistence.metadata as Record<string, unknown>).conversationId = conversationId;
|
|
||||||
}
|
|
||||||
return this.persistence;
|
return this.persistence;
|
||||||
}
|
}
|
||||||
if (!this.sessionId) {
|
if (!this.sessionId) {
|
||||||
@@ -652,11 +661,23 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
metadata: {
|
metadata: {
|
||||||
...restConfig,
|
...restConfig,
|
||||||
conversationId,
|
conversationId,
|
||||||
|
conversation_id: conversationId,
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
this.updatePersistenceConversationId();
|
||||||
return this.persistence;
|
return this.persistence;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private updatePersistenceConversationId(): void {
|
||||||
|
const conversationId = this.conversationId ?? this.sessionId ?? undefined;
|
||||||
|
if (!conversationId || !this.persistence?.metadata || typeof this.persistence.metadata !== "object") {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const metadata = this.persistence.metadata as Record<string, unknown>;
|
||||||
|
metadata.conversationId = conversationId;
|
||||||
|
metadata.conversation_id = conversationId;
|
||||||
|
}
|
||||||
|
|
||||||
async close(): Promise<void> {
|
async close(): Promise<void> {
|
||||||
for (const pending of this.pendingPermissionHandlers.values()) {
|
for (const pending of this.pendingPermissionHandlers.values()) {
|
||||||
pending.reject(new Error("Codex MCP session closed"));
|
pending.reject(new Error("Codex MCP session closed"));
|
||||||
@@ -669,18 +690,20 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
|
|
||||||
if (!this.connected) return;
|
if (!this.connected) return;
|
||||||
const pid = this.transport?.pid ?? null;
|
const pid = this.transport?.pid ?? null;
|
||||||
|
let closed = false;
|
||||||
try {
|
try {
|
||||||
await this.client.close();
|
await this.client.close();
|
||||||
|
closed = true;
|
||||||
} catch {
|
} catch {
|
||||||
try {
|
try {
|
||||||
await this.transport?.close?.();
|
await this.transport?.close?.();
|
||||||
|
closed = true;
|
||||||
} catch {
|
} catch {
|
||||||
// ignore
|
// ignore
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (pid) {
|
if (!closed && pid) {
|
||||||
try {
|
try {
|
||||||
process.kill(pid, 0);
|
|
||||||
process.kill(pid, "SIGKILL");
|
process.kill(pid, "SIGKILL");
|
||||||
} catch {
|
} catch {
|
||||||
// ignore
|
// ignore
|
||||||
@@ -731,18 +754,67 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
const conversationId = this.conversationId ?? this.sessionId;
|
const conversationId = this.conversationId ?? this.sessionId;
|
||||||
response = await this.client.callTool(
|
try {
|
||||||
{
|
response = await this.client.callTool(
|
||||||
name: "codex-reply",
|
{
|
||||||
arguments: {
|
name: "codex-reply",
|
||||||
sessionId: this.sessionId,
|
arguments: {
|
||||||
conversationId,
|
sessionId: this.sessionId,
|
||||||
prompt,
|
conversationId,
|
||||||
|
prompt,
|
||||||
|
},
|
||||||
},
|
},
|
||||||
},
|
undefined,
|
||||||
undefined,
|
{ signal, timeout: DEFAULT_TIMEOUT_MS }
|
||||||
{ signal, timeout: DEFAULT_TIMEOUT_MS }
|
);
|
||||||
);
|
if (isMissingConversationIdResponse(response)) {
|
||||||
|
const replayPrompt = this.buildResumePrompt(prompt);
|
||||||
|
const config = buildCodexMcpConfig(this.config, replayPrompt, this.currentMode);
|
||||||
|
const attempt = async (arguments_: Record<string, unknown>) =>
|
||||||
|
this.client.callTool(
|
||||||
|
{ name: "codex", arguments: arguments_ },
|
||||||
|
undefined,
|
||||||
|
{ signal, timeout: DEFAULT_TIMEOUT_MS }
|
||||||
|
);
|
||||||
|
try {
|
||||||
|
response = await attempt(config);
|
||||||
|
} catch (fallbackError) {
|
||||||
|
if (config.model && isUnsupportedChatGptModelError(fallbackError)) {
|
||||||
|
const { model: _ignoredModel, ...fallback } = config;
|
||||||
|
this.runtimeModel = null;
|
||||||
|
this.config.model = undefined;
|
||||||
|
response = await attempt(fallback);
|
||||||
|
} else {
|
||||||
|
throw fallbackError;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch (error) {
|
||||||
|
if (isMissingConversationIdError(error)) {
|
||||||
|
const replayPrompt = this.buildResumePrompt(prompt);
|
||||||
|
const config = buildCodexMcpConfig(this.config, replayPrompt, this.currentMode);
|
||||||
|
const attempt = async (arguments_: Record<string, unknown>) =>
|
||||||
|
this.client.callTool(
|
||||||
|
{ name: "codex", arguments: arguments_ },
|
||||||
|
undefined,
|
||||||
|
{ signal, timeout: DEFAULT_TIMEOUT_MS }
|
||||||
|
);
|
||||||
|
try {
|
||||||
|
response = await attempt(config);
|
||||||
|
} catch (fallbackError) {
|
||||||
|
if (config.model && isUnsupportedChatGptModelError(fallbackError)) {
|
||||||
|
const { model: _ignoredModel, ...fallback } = config;
|
||||||
|
this.runtimeModel = null;
|
||||||
|
this.config.model = undefined;
|
||||||
|
response = await attempt(fallback);
|
||||||
|
} else {
|
||||||
|
throw fallbackError;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (signal.aborted) {
|
if (signal.aborted) {
|
||||||
@@ -857,14 +929,23 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
if (!record) return;
|
if (!record) return;
|
||||||
const meta = record.meta && typeof record.meta === "object" ? (record.meta as Record<string, unknown>) : null;
|
const meta = record.meta && typeof record.meta === "object" ? (record.meta as Record<string, unknown>) : null;
|
||||||
const sessionId = meta?.sessionId ?? record.sessionId;
|
const sessionId = meta?.sessionId ?? record.sessionId;
|
||||||
const conversationId = meta?.conversationId ?? record.conversationId;
|
const conversationId =
|
||||||
|
meta?.conversationId ??
|
||||||
|
meta?.conversation_id ??
|
||||||
|
meta?.thread_id ??
|
||||||
|
record.conversationId ??
|
||||||
|
record.conversation_id ??
|
||||||
|
record.thread_id;
|
||||||
const model = meta?.model ?? record.model;
|
const model = meta?.model ?? record.model;
|
||||||
if (typeof sessionId === "string") {
|
if (!this.sessionId && typeof sessionId === "string") {
|
||||||
this.sessionId = sessionId;
|
this.sessionId = sessionId;
|
||||||
this.flushPendingHistory();
|
this.flushPendingHistory();
|
||||||
}
|
}
|
||||||
if (typeof conversationId === "string") {
|
if (typeof conversationId === "string" && conversationId.length > 0) {
|
||||||
this.conversationId = conversationId;
|
if ((!this.lockConversationId || !this.conversationId) && this.conversationId !== conversationId) {
|
||||||
|
this.conversationId = conversationId;
|
||||||
|
this.updatePersistenceConversationId();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if (typeof model === "string" && model.length > 0) {
|
if (typeof model === "string" && model.length > 0) {
|
||||||
this.runtimeModel = model;
|
this.runtimeModel = model;
|
||||||
@@ -875,13 +956,22 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
for (const item of content) {
|
for (const item of content) {
|
||||||
if (!item || typeof item !== "object") continue;
|
if (!item || typeof item !== "object") continue;
|
||||||
const sessionCandidate = (item as Record<string, unknown>).sessionId;
|
const sessionCandidate = (item as Record<string, unknown>).sessionId;
|
||||||
const conversationCandidate = (item as Record<string, unknown>).conversationId;
|
const conversationCandidate =
|
||||||
|
(item as Record<string, unknown>).conversationId ??
|
||||||
|
(item as Record<string, unknown>).conversation_id ??
|
||||||
|
(item as Record<string, unknown>).thread_id;
|
||||||
if (!this.sessionId && typeof sessionCandidate === "string") {
|
if (!this.sessionId && typeof sessionCandidate === "string") {
|
||||||
this.sessionId = sessionCandidate;
|
this.sessionId = sessionCandidate;
|
||||||
this.flushPendingHistory();
|
this.flushPendingHistory();
|
||||||
}
|
}
|
||||||
if (!this.conversationId && typeof conversationCandidate === "string") {
|
if (typeof conversationCandidate === "string" && conversationCandidate.length > 0) {
|
||||||
this.conversationId = conversationCandidate;
|
if (
|
||||||
|
(!this.lockConversationId || !this.conversationId) &&
|
||||||
|
this.conversationId !== conversationCandidate
|
||||||
|
) {
|
||||||
|
this.conversationId = conversationCandidate;
|
||||||
|
this.updatePersistenceConversationId();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
const modelCandidate = (item as Record<string, unknown>).model;
|
const modelCandidate = (item as Record<string, unknown>).model;
|
||||||
if (typeof modelCandidate === "string" && modelCandidate.length > 0) {
|
if (typeof modelCandidate === "string" && modelCandidate.length > 0) {
|
||||||
@@ -902,14 +992,18 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
for (const candidate of candidates) {
|
for (const candidate of candidates) {
|
||||||
const record = candidate as Record<string, unknown>;
|
const record = candidate as Record<string, unknown>;
|
||||||
const sessionId = record.session_id ?? record.sessionId;
|
const sessionId = record.session_id ?? record.sessionId;
|
||||||
const conversationId = record.conversation_id ?? record.conversationId;
|
const conversationId =
|
||||||
|
record.conversation_id ?? record.conversationId ?? record.thread_id;
|
||||||
const model = record.model;
|
const model = record.model;
|
||||||
if (!this.sessionId && typeof sessionId === "string") {
|
if (!this.sessionId && typeof sessionId === "string") {
|
||||||
this.sessionId = sessionId;
|
this.sessionId = sessionId;
|
||||||
this.flushPendingHistory();
|
this.flushPendingHistory();
|
||||||
}
|
}
|
||||||
if (!this.conversationId && typeof conversationId === "string") {
|
if (typeof conversationId === "string" && conversationId.length > 0) {
|
||||||
this.conversationId = conversationId;
|
if ((!this.lockConversationId || !this.conversationId) && this.conversationId !== conversationId) {
|
||||||
|
this.conversationId = conversationId;
|
||||||
|
this.updatePersistenceConversationId();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if (typeof model === "string" && model.length > 0) {
|
if (typeof model === "string" && model.length > 0) {
|
||||||
this.runtimeModel = model;
|
this.runtimeModel = model;
|
||||||
@@ -1325,6 +1419,22 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private buildResumePrompt(prompt: string): string {
|
||||||
|
const historyLines: string[] = [];
|
||||||
|
for (const item of this.persistedHistory) {
|
||||||
|
if (item.type === "user_message") {
|
||||||
|
historyLines.push(`User: ${item.text}`);
|
||||||
|
}
|
||||||
|
if (item.type === "assistant_message") {
|
||||||
|
historyLines.push(`Assistant: ${item.text}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (historyLines.length === 0) {
|
||||||
|
return prompt;
|
||||||
|
}
|
||||||
|
return ["Previous conversation:", ...historyLines, "", `User: ${prompt}`].join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
private buildPermissionRequest(params: unknown): AgentPermissionRequest | null {
|
private buildPermissionRequest(params: unknown): AgentPermissionRequest | null {
|
||||||
if (!params || typeof params !== "object") {
|
if (!params || typeof params !== "object") {
|
||||||
return null;
|
return null;
|
||||||
|
|||||||
@@ -271,7 +271,8 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov
|
|||||||
- [x] **Fix**: Codex MCP should emit error timeline items for failed turns (currently none).
|
- [x] **Fix**: Codex MCP should emit error timeline items for failed turns (currently none).
|
||||||
- **Done (2025-12-24 20:41)**: Tracked error timeline emission separately so failed turns always emit an error item before `turn_failed`.
|
- **Done (2025-12-24 20:41)**: Tracked error timeline emission separately so failed turns always emit an error item before `turn_failed`.
|
||||||
|
|
||||||
- [ ] **Fix**: Codex MCP persistence/resume should include conversation_id metadata (resume error).
|
- [x] **Fix**: Codex MCP persistence/resume should include conversation_id metadata (resume error).
|
||||||
|
- **Done (2025-12-24 20:58)**: Included conversation_id metadata, kept conversation ids stable on resume, and added a history-based replay fallback when Codex reply cannot find the conversation.
|
||||||
|
|
||||||
- [ ] **Fix**: Codex MCP permission request flow still missing in read-only/deny/abort tests (permission request null).
|
- [ ] **Fix**: Codex MCP permission request flow still missing in read-only/deny/abort tests (permission request null).
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user