Fix Codex skill prompt turn startup

2026-04-30 22:36:26 +07:00
parent 4d840dafd4
commit c783b7b093
4 changed files with 193 additions and 20 deletions
@@ -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<boolean> {
+      return true;
+    }
+
+    async createSession(): Promise<AgentSession> {
+      return this.session;
+    }
+
+    async resumeSession(): Promise<AgentSession> {
+      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");
@@ -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;
      }

@@ -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: [],
      },
    ]);
  });
@@ -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<unknown[]> {
+): Promise<CodexAppServerUserInput[]> {
  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<CodexAppServerUserInput, { type: "text" }> {
+  return {
+    type: "text",
+    text,
+    text_elements: [],
+  };
+}
+
 function buildCodexAppServerEnv(
  runtimeSettings?: ProviderRuntimeSettings,
  launchEnv?: Record<string, string>,
@@ -2891,7 +2928,7 @@ class CodexAppServerAgentSession implements AgentSession {
  private async buildCommandPromptInput(
    commandName: string,
    args?: string,
-  ): Promise<AgentPromptInput> {
+  ): Promise<CodexPromptInput> {
    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<unknown[]> {
+  private async buildUserInput(prompt: CodexPromptInput): Promise<CodexAppServerUserInput[]> {
    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 {