From c783b7b09334dabe3f93344a23164284db343df8 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Thu, 30 Apr 2026 22:36:26 +0700 Subject: [PATCH] Fix Codex skill prompt turn startup --- .../src/server/agent/agent-manager.test.ts | 65 +++++++++++++++++ .../server/src/server/agent/agent-manager.ts | 3 +- .../providers/codex-app-server-agent.test.ts | 73 ++++++++++++++++++- .../agent/providers/codex-app-server-agent.ts | 72 +++++++++++++----- 4 files changed, 193 insertions(+), 20 deletions(-) diff --git a/packages/server/src/server/agent/agent-manager.test.ts b/packages/server/src/server/agent/agent-manager.test.ts index 33174a883..3aacd2ade 100644 --- a/packages/server/src/server/agent/agent-manager.test.ts +++ b/packages/server/src/server/agent/agent-manager.test.ts @@ -3606,6 +3606,71 @@ test("clearAgentAttention on errored agent stays cleared until a new error trans expect(persistedAfterSecondFailure?.attentionReason).toBe("error"); }); +test("streamAgent clears pending run when startTurn fails before a turn id exists", async () => { + const workdir = mkdtempSync(join(tmpdir(), "agent-manager-start-turn-failure-")); + const storagePath = join(workdir, "agents"); + const storage = new AgentStorage(storagePath, logger); + + class FailsOnceBeforeTurnSession extends TestAgentSession { + private attempt = 0; + + override async startTurn(): Promise<{ turnId: string }> { + this.attempt += 1; + if (this.attempt === 1) { + throw new Error("Invalid request: missing field `text`"); + } + return super.startTurn(); + } + } + + class FailsOnceClient implements AgentClient { + readonly provider = "codex" as const; + readonly capabilities = TEST_CAPABILITIES; + readonly session = new FailsOnceBeforeTurnSession({ + provider: "codex", + cwd: workdir, + }); + + async isAvailable(): Promise { + return true; + } + + async createSession(): Promise { + return this.session; + } + + async resumeSession(): Promise { + return this.session; + } + } + + const manager = new AgentManager({ + clients: { + codex: new FailsOnceClient(), + }, + registry: storage, + logger, + idFactory: () => "00000000-0000-4000-8000-000000000131", + }); + + const agent = await manager.createAgent({ + provider: "codex", + cwd: workdir, + title: "Start turn failure cleanup", + }); + + await expect(manager.runAgent(agent.id, "fail before turn id")).rejects.toThrow( + "Invalid request: missing field `text`", + ); + + await expect(manager.runAgent(agent.id, "second turn")).resolves.toEqual( + expect.objectContaining({ + sessionId: expect.any(String), + canceled: false, + }), + ); +}); + test("archiveAgent persists archivedAt and updatedAt before emitting closed state", async () => { const workdir = mkdtempSync(join(tmpdir(), "agent-manager-archive-")); const storagePath = join(workdir, "agents"); diff --git a/packages/server/src/server/agent/agent-manager.ts b/packages/server/src/server/agent/agent-manager.ts index 37a3817d1..105e63a98 100644 --- a/packages/server/src/server/agent/agent-manager.ts +++ b/packages/server/src/server/agent/agent-manager.ts @@ -1324,12 +1324,13 @@ export class AgentManager { turnId = result.turnId; } catch (error) { const errorMsg = error instanceof Error ? error.message : "Failed to start turn"; - this.handleStreamEvent(agent, { + await this.handleStreamEvent(agent, { type: "turn_failed", provider: agent.provider, error: errorMsg, }); this.finalizeForegroundTurn(agent); + this.settlePendingForegroundRun(agentId, pendingRun.token); throw error; } diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts index 0580b2b0d..2b7eaf3f1 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts @@ -314,6 +314,59 @@ describe("Codex app-server provider", () => { ); }); + test("resolves Codex skill slash commands into app-server skill input", async () => { + const session = createSession(); + const request = vi.fn(async (method: string) => { + if (method === "skills/list") { + return { + data: [ + { + cwd: "/tmp/codex-question-test", + skills: [ + { + name: "paseo-implement", + description: "Execute an existing Paseo plan.", + path: "/tmp/skills/paseo-implement/SKILL.md", + }, + ], + errors: [], + }, + ], + }; + } + if (method === "thread/loaded/list") { + return { data: ["test-thread"] }; + } + if (method === "turn/start") { + return {}; + } + throw new Error(`Unexpected request: ${method}`); + }); + + session.activeForegroundTurnId = null; + session.client = { request } as unknown as CodexClientLike; + + await session.startTurn("/paseo-implement in a worktree, remember to use Claude for the UI"); + + const turnStartCall = request.mock.calls.find(([method]) => method === "turn/start"); + expect(turnStartCall?.[1]).toEqual( + expect.objectContaining({ + input: [ + { + type: "skill", + name: "paseo-implement", + path: "/tmp/skills/paseo-implement/SKILL.md", + }, + { + type: "text", + text: "in a worktree, remember to use Claude for the UI", + text_elements: [], + }, + ], + }), + ); + }); + test("maps image prompt blocks to Codex localImage input", async () => { const input = await codexAppServerTurnInputFromPrompt( [ @@ -352,11 +405,27 @@ describe("Codex app-server provider", () => { expect(input).toEqual([ { type: "text", + text_elements: [], text: expect.stringContaining("GitHub PR #123: Fix race in worktree setup"), }, ]); }); + test("passes Codex skill prompt blocks through to Codex app-server input", async () => { + const input = await codexAppServerTurnInputFromPrompt( + [ + { type: "skill", name: "fix-build", path: "/tmp/skills/fix-build/SKILL.md" }, + { type: "text", text: "keep this build moving" }, + ], + logger, + ); + + expect(input).toEqual([ + { type: "skill", name: "fix-build", path: "/tmp/skills/fix-build/SKILL.md" }, + { type: "text", text: "keep this build moving", text_elements: [] }, + ]); + }); + test("separates Codex text prompts from rendered attachment text", async () => { const input = await codexAppServerTurnInputFromPrompt( [ @@ -373,10 +442,11 @@ describe("Codex app-server provider", () => { ); expect(input).toEqual([ - { type: "text", text: "Please review this" }, + { type: "text", text: "Please review this", text_elements: [] }, { type: "text", text: expect.stringMatching(/^\n\nGitHub Issue #456: Attachment spacing/), + text_elements: [], }, ]); }); @@ -399,6 +469,7 @@ describe("Codex app-server provider", () => { { type: "text", text: expect.stringMatching(/^GitHub Issue #456: Attachment spacing/), + text_elements: [], }, ]); }); diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index 1c64d705d..ed203dc50 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -2402,22 +2402,55 @@ async function readCodexConfiguredDefaults( return mergeCodexConfiguredDefaults(savedConfigDefaults, configReadDefaults); } +interface CodexSkillPromptBlock { + type: "skill"; + name: string; + path: string; +} + +type CodexPromptContentBlock = AgentPromptContentBlock | CodexSkillPromptBlock; +type CodexPromptInput = string | CodexPromptContentBlock[]; +interface CodexTextElement { + byteRange: { + start: number; + end: number; + }; + placeholder: string | null; +} + +type CodexAppServerUserInput = + | { + type: "text"; + text: string; + text_elements: CodexTextElement[]; + } + | { + type: "localImage"; + path: string; + } + | CodexSkillPromptBlock; + export async function codexAppServerTurnInputFromPrompt( - prompt: AgentPromptInput, + prompt: CodexPromptInput, logger: Logger, -): Promise { +): Promise { if (typeof prompt === "string") { - return [{ type: "text", text: prompt }]; + return [toCodexTextInput(prompt)]; } - const output: unknown[] = []; + const output: CodexAppServerUserInput[] = []; let previousTextBlock = false; for (const block of prompt) { if (block.type === "text") { - output.push(block); + output.push(toCodexTextInput(block.text)); previousTextBlock = block.text.length > 0; continue; } + if (block.type === "skill") { + output.push(block); + previousTextBlock = false; + continue; + } if (block.type === "image") { try { const filePath = await writeImageAttachment(block.mimeType, block.data); @@ -2426,23 +2459,27 @@ export async function codexAppServerTurnInputFromPrompt( const message = error instanceof Error ? error.message : String(error); logger.warn({ message }, "Failed to write Codex image attachment"); output.push({ - type: "text", - text: `User attached image (failed to write temp file): ${message}`, + ...toCodexTextInput(`User attached image (failed to write temp file): ${message}`), }); } previousTextBlock = false; continue; } const attachmentText = renderPromptAttachmentAsText(block); - output.push({ - type: "text", - text: previousTextBlock ? `\n\n${attachmentText}` : attachmentText, - }); + output.push(toCodexTextInput(previousTextBlock ? `\n\n${attachmentText}` : attachmentText)); previousTextBlock = true; } return output; } +function toCodexTextInput(text: string): Extract { + return { + type: "text", + text, + text_elements: [], + }; +} + function buildCodexAppServerEnv( runtimeSettings?: ProviderRuntimeSettings, launchEnv?: Record, @@ -2891,7 +2928,7 @@ class CodexAppServerAgentSession implements AgentSession { private async buildCommandPromptInput( commandName: string, args?: string, - ): Promise { + ): Promise { if (commandName.startsWith("prompts:")) { const promptName = commandName.slice("prompts:".length); const codexHome = resolveCodexHomeDir(); @@ -2908,9 +2945,9 @@ class CodexAppServerAgentSession implements AgentSession { } const skill = this.cachedSkills.find((entry) => entry.name === commandName); if (skill) { - const input = [ + const input: CodexPromptContentBlock[] = [ { type: "skill", name: skill.name, path: skill.path }, - ] as unknown as AgentPromptContentBlock[]; + ]; if (args && args.trim().length > 0) { input.push({ type: "text", text: args.trim() }); } else { @@ -3496,12 +3533,11 @@ class CodexAppServerAgentSession implements AgentSession { return Object.keys(innerConfig).length > 0 ? innerConfig : null; } - private async buildUserInput(prompt: AgentPromptInput): Promise { + private async buildUserInput(prompt: CodexPromptInput): Promise { if (typeof prompt === "string") { - return [{ type: "text", text: prompt }]; + return [toCodexTextInput(prompt)]; } - const blocks = prompt as AgentPromptContentBlock[]; - return await codexAppServerTurnInputFromPrompt(blocks, this.logger); + return await codexAppServerTurnInputFromPrompt(prompt, this.logger); } private emitEvent(event: AgentStreamEvent): void {