diff --git a/packages/coding-agent/src/prompts/system/subagent-submit-reminder.md b/packages/coding-agent/src/prompts/system/subagent-submit-reminder.md index 2a31f1494..8a6727444 100644 --- a/packages/coding-agent/src/prompts/system/subagent-submit-reminder.md +++ b/packages/coding-agent/src/prompts/system/subagent-submit-reminder.md @@ -2,10 +2,10 @@ You stopped without calling submit_result. This is reminder {{retryCount}} of {{maxRetries}}. You **MUST** call submit_result as your only action now. Choose one: -- If task is complete: you **MUST** call submit_result with your result data -- If task failed or was interrupted: you **MUST** call submit_result with status="aborted" and describe what happened +- If task is complete: call submit_result with your result in the `data` field +- If task failed: call submit_result with an `error` field describing what happened -You **MUST NOT** choose aborted if you can still complete the task through exploration (using available tools or repo context). If you abort, you **MUST** include what you tried and the exact blocker. +You **MUST NOT** give up if you can still complete the task through exploration (using available tools or repo context). If you submit an error, you **MUST** include what you tried and the exact blocker. You **MUST NOT** output text without a tool call. You **MUST** call submit_result to finish. \ No newline at end of file diff --git a/packages/coding-agent/src/prompts/system/subagent-system-prompt.md b/packages/coding-agent/src/prompts/system/subagent-system-prompt.md index 7ecf69b41..e7b8ae1a0 100644 --- a/packages/coding-agent/src/prompts/system/subagent-system-prompt.md +++ b/packages/coding-agent/src/prompts/system/subagent-system-prompt.md @@ -29,11 +29,11 @@ Your result **MUST** match this TypeScript interface: {{/if}} {{SECTION_SEPERATOR "Giving Up"}} -If you cannot complete the assignment, you **MUST** call `submit_result` exactly once with `status="aborted"` and an error message describing what you tried and the exact blocker. +If you cannot complete the assignment, you **MUST** call `submit_result` exactly once with an `error` message describing what you tried and the exact blocker. -Aborting is a last resort. -You **MUST NOT** abort due to uncertainty or missing information obtainable via tools or repo context. -You **MUST NOT** abort due to requiring a design, you can derive that yourself, more than capable of that. +Giving up is a last resort. +You **MUST NOT** give up due to uncertainty or missing information obtainable via tools or repo context. +You **MUST NOT** give up due to requiring a design, you can derive that yourself, more than capable of that. Proceed with the best approach using the most reasonable option. diff --git a/packages/coding-agent/src/tools/submit-result.ts b/packages/coding-agent/src/tools/submit-result.ts index 2e9fdfd30..43c6dc281 100644 --- a/packages/coding-agent/src/tools/submit-result.ts +++ b/packages/coding-agent/src/tools/submit-result.ts @@ -4,8 +4,7 @@ * Subagents must call this tool to finish and return structured JSON output. */ import type { AgentTool, AgentToolContext, AgentToolResult, AgentToolUpdateCallback } from "@oh-my-pi/pi-agent-core"; -import { StringEnum } from "@oh-my-pi/pi-ai"; -import type { Static, TObject } from "@sinclair/typebox"; +import type { Static, TSchema } from "@sinclair/typebox"; import { Type } from "@sinclair/typebox"; import Ajv, { type ErrorObject, type ValidateFunction } from "ajv"; import { subprocessToolRegistry } from "../task/subprocess-tool-registry"; @@ -52,13 +51,13 @@ function formatAjvErrors(errors: ErrorObject[] | null | undefined): string { .join("; "); } -export class SubmitResultTool implements AgentTool { +export class SubmitResultTool implements AgentTool { readonly name = "submit_result"; readonly label = "Submit Result"; readonly description = "Finish the task with structured JSON output. Call exactly once at the end of the task.\n\n" + - "If you cannot complete the task, call with status='aborted' and an error message."; - readonly parameters: TObject; + "If you cannot complete the task, call with an error message payload."; + readonly parameters: TSchema; readonly strict = true; readonly #validate?: ValidateFunction; @@ -92,45 +91,45 @@ export class SubmitResultTool implements AgentTool }) : Type.Object({}, { additionalProperties: true, description: "Structured JSON output (no schema specified)" }); - this.parameters = Type.Object({ - data: dataSchema, - status: Type.Optional( - StringEnum(["success", "aborted"], { - description: "Use 'aborted' if the task cannot be completed, defaults to 'success'", - }), - ), - error: Type.Optional(Type.String({ description: "Error message when status is 'aborted'" })), - }); + this.parameters = Type.Union([ + Type.Object({ + data: dataSchema, + }), + Type.Object({ + error: Type.String({ description: "Error message when the task cannot be completed" }), + }), + ]); } async execute( _toolCallId: string, - params: Static, + params: Static, _signal?: AbortSignal, _onUpdate?: AgentToolUpdateCallback, _context?: AgentToolContext, ): Promise> { - const status = (params.status ?? "success") as "success" | "aborted"; + const raw = params as Record; + const errorMessage = typeof raw.error === "string" ? raw.error : undefined; + const status = errorMessage !== undefined ? "aborted" : "success"; + const data = raw.data; - // Skip validation when aborting - data is optional for aborts if (status === "success") { - if (params.data === undefined || params.data === null) { - throw new Error("data is required when status is 'success' (got null/undefined)"); + if (data === undefined || data === null) { + throw new Error("data is required when submit_result indicates success"); } if (this.#schemaError) { throw new Error(`Invalid output schema: ${this.#schemaError}`); } - if (this.#validate && !this.#validate(params.data)) { + if (this.#validate && !this.#validate(data)) { throw new Error(`Output does not match schema: ${formatAjvErrors(this.#validate.errors)}`); } } - const responseText = - status === "aborted" ? `Task aborted: ${params.error || "No reason provided"}` : "Result submitted."; + const responseText = status === "aborted" ? `Task aborted: ${errorMessage}` : "Result submitted."; return { content: [{ type: "text", text: responseText }], - details: { data: params.data, status, error: params.error as string | undefined }, + details: { data, status, error: errorMessage }, }; } } diff --git a/packages/coding-agent/test/tools/submit-result.test.ts b/packages/coding-agent/test/tools/submit-result.test.ts new file mode 100644 index 000000000..cce05819c --- /dev/null +++ b/packages/coding-agent/test/tools/submit-result.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from "bun:test"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; +import { SubmitResultTool } from "@oh-my-pi/pi-coding-agent/tools/submit-result"; + +function createSession(overrides: Partial = {}): ToolSession { + return { + cwd: "/tmp", + hasUI: false, + getSessionFile: () => null, + getSessionSpawns: () => "*", + settings: Settings.isolated(), + ...overrides, + }; +} + +describe("SubmitResultTool", () => { + it("exposes `{ data } | { error }` parameter union", () => { + const tool = new SubmitResultTool(createSession()); + const schema = tool.parameters as { anyOf?: Array<{ required?: string[] }> }; + expect(schema.anyOf).toHaveLength(2); + expect(schema.anyOf?.[0]?.required).toEqual(["data"]); + expect(schema.anyOf?.[1]?.required).toEqual(["error"]); + }); + + it("accepts success payload with data", async () => { + const tool = new SubmitResultTool(createSession()); + const result = await tool.execute("call-1", { data: { ok: true } } as never); + expect(result.details).toEqual({ data: { ok: true }, status: "success", error: undefined }); + }); + + it("accepts aborted payload with error only", async () => { + const tool = new SubmitResultTool(createSession()); + const result = await tool.execute("call-2", { error: "blocked" } as never); + expect(result.details).toEqual({ data: undefined, status: "aborted", error: "blocked" }); + }); + + it("rejects successful submissions without data", async () => { + const tool = new SubmitResultTool(createSession()); + await expect(tool.execute("call-3", {} as never)).rejects.toThrow( + "data is required when submit_result indicates success", + ); + }); +});