From 192ad656a4b616a31eb438257ed6385ec0279726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B0=8F=E6=A9=98?= Date: Thu, 21 May 2026 03:07:56 +0000 Subject: [PATCH] refactor: remove systemPrompt, make four-phase fields required Breaking change per review: - Remove systemPrompt from RoleDefinition entirely - identity/prepare/execute/report are now required (string, not nullable) - Remove all legacy fallback logic in buildRolePrompt - Simplify validate.ts, workflow.ts materialize - Migrate all test fixtures and example workflows Refs #359 --- examples/solve-issue.yaml | 15 ++++-- .../cli-workflow/src/__tests__/thread.test.ts | 5 +- packages/cli-workflow/src/commands/thread.ts | 2 +- .../cli-workflow/src/commands/workflow.ts | 18 ++----- packages/cli-workflow/src/validate.ts | 19 ++++---- .../__tests__/build-role-prompt.test.ts | 48 +++++++------------ .../src/build-role-prompt.ts | 28 ++++------- packages/workflow-agent-kit/src/types.ts | 2 +- .../__tests__/evaluate.test.ts | 15 ++++-- packages/workflow-protocol/src/schemas.ts | 3 +- packages/workflow-protocol/src/types.ts | 9 ++-- 11 files changed, 73 insertions(+), 91 deletions(-) diff --git a/examples/solve-issue.yaml b/examples/solve-issue.yaml index bb33e26..2f86083 100644 --- a/examples/solve-issue.yaml +++ b/examples/solve-issue.yaml @@ -3,7 +3,10 @@ description: "End-to-end issue resolution" roles: planner: description: "Creates implementation plan" - systemPrompt: "You are a planning agent. Analyze the issue and create a step-by-step plan." + identity: "You are a planning agent. You analyze issues and create step-by-step plans." + prepare: "Read the issue description and any linked context carefully." + execute: "Analyze the issue and create a detailed, actionable implementation plan." + report: "Output the plan summary and list of concrete steps." outputSchema: type: object properties: @@ -16,7 +19,10 @@ roles: required: [plan, steps] developer: description: "Implements code changes" - systemPrompt: "You are a developer agent. Implement the plan." + identity: "You are a developer agent. You implement code changes according to plans." + prepare: "Load coding tools and review the project structure and conventions." + execute: "Implement the plan. Write code, tests, and ensure existing tests pass." + report: "List all files changed and provide a summary of the implementation." outputSchema: type: object properties: @@ -29,7 +35,10 @@ roles: required: [filesChanged, summary] reviewer: description: "Reviews code changes" - systemPrompt: "You are a code reviewer. Review the implementation." + identity: "You are a code reviewer. You review implementations for correctness and quality." + prepare: "Review the project's coding standards and conventions." + execute: "Review the implementation against the plan. Check for bugs, edge cases, and style." + report: "Approve or reject with detailed comments explaining your decision." outputSchema: type: object properties: diff --git a/packages/cli-workflow/src/__tests__/thread.test.ts b/packages/cli-workflow/src/__tests__/thread.test.ts index 7cf54dc..fb83981 100644 --- a/packages/cli-workflow/src/__tests__/thread.test.ts +++ b/packages/cli-workflow/src/__tests__/thread.test.ts @@ -211,7 +211,10 @@ describe("cmdThreadRead ### Content section", () => { roles: { writer: { description: "Write", - systemPrompt: "You are a writer.", + identity: "You are a writer.", + prepare: "", + execute: "Write content as requested.", + report: "Summarize what was written.", outputSchema: "placeholder00" as CasRef, }, }, diff --git a/packages/cli-workflow/src/commands/thread.ts b/packages/cli-workflow/src/commands/thread.ts index 99c0f98..0cc89dc 100644 --- a/packages/cli-workflow/src/commands/thread.ts +++ b/packages/cli-workflow/src/commands/thread.ts @@ -500,7 +500,7 @@ function formatThreadReadMarkdown(options: { ]; const roleDef = workflow.roles[item.payload.role]; if (roleDef) { - const prompt = roleDef.identity ?? roleDef.systemPrompt ?? ""; + const prompt = roleDef.identity; stepLines.push("", "### Prompt", "", prompt); } if (item.payload.detail) { diff --git a/packages/cli-workflow/src/commands/workflow.ts b/packages/cli-workflow/src/commands/workflow.ts index c48012c..32c1b93 100644 --- a/packages/cli-workflow/src/commands/workflow.ts +++ b/packages/cli-workflow/src/commands/workflow.ts @@ -67,22 +67,14 @@ async function materializeWorkflowPayload( `${raw.name}.${roleName}`, role.outputSchema, ); - const roleDef: RoleDefinition = { + roles[roleName] = { description: role.description, - systemPrompt: role.systemPrompt ?? null, - identity: role.identity ?? null, - prepare: role.prepare ?? null, - execute: role.execute ?? null, - report: role.report ?? null, + identity: role.identity, + prepare: role.prepare, + execute: role.execute, + report: role.report, outputSchema, }; - // Strip null fields so CAS payload stays lean and schema-valid - for (const key of ["systemPrompt", "identity", "prepare", "execute", "report"] as const) { - if (roleDef[key] === null) { - delete (roleDef as Record)[key]; - } - } - roles[roleName] = roleDef; } return { name: raw.name, diff --git a/packages/cli-workflow/src/validate.ts b/packages/cli-workflow/src/validate.ts index be63e40..8c50db2 100644 --- a/packages/cli-workflow/src/validate.ts +++ b/packages/cli-workflow/src/validate.ts @@ -16,17 +16,14 @@ function isRoleDefinition(value: unknown): boolean { } const outputSchema = value.outputSchema; const schemaOk = isRecord(outputSchema) && typeof outputSchema.type === "string"; - const hasSystemPrompt = - value.systemPrompt === undefined || value.systemPrompt === null || typeof value.systemPrompt === "string"; - const hasIdentity = - value.identity === undefined || value.identity === null || typeof value.identity === "string"; - const hasPrepare = - value.prepare === undefined || value.prepare === null || typeof value.prepare === "string"; - const hasExecute = - value.execute === undefined || value.execute === null || typeof value.execute === "string"; - const hasReport = - value.report === undefined || value.report === null || typeof value.report === "string"; - return typeof value.description === "string" && hasSystemPrompt && hasIdentity && hasPrepare && hasExecute && hasReport && schemaOk; + return ( + typeof value.description === "string" && + typeof value.identity === "string" && + typeof value.prepare === "string" && + typeof value.execute === "string" && + typeof value.report === "string" && + schemaOk + ); } function isConditionDefinition(value: unknown): boolean { diff --git a/packages/workflow-agent-kit/__tests__/build-role-prompt.test.ts b/packages/workflow-agent-kit/__tests__/build-role-prompt.test.ts index e0a1c60..4ce456a 100644 --- a/packages/workflow-agent-kit/__tests__/build-role-prompt.test.ts +++ b/packages/workflow-agent-kit/__tests__/build-role-prompt.test.ts @@ -3,10 +3,9 @@ import type { RoleDefinition } from "@uncaged/workflow-protocol"; import { buildRolePrompt } from "../src/build-role-prompt.js"; describe("buildRolePrompt", () => { - test("four-phase: all fields present", () => { + test("all fields present", () => { const role: RoleDefinition = { description: "A coder", - systemPrompt: "legacy prompt", identity: "You are a senior developer.", prepare: "Load cursor-agent skill.", execute: "Implement the feature.", @@ -22,40 +21,15 @@ describe("buildRolePrompt", () => { expect(result).toContain("Implement the feature."); expect(result).toContain("## Report"); expect(result).toContain("Summarize changes."); - expect(result).not.toContain("legacy prompt"); }); - test("legacy: only systemPrompt", () => { - const role: RoleDefinition = { - description: "A planner", - systemPrompt: "You are a planning agent.", - identity: null, - prepare: null, - execute: null, - report: null, - outputSchema: "placeholder00000" as string, - }; - const result = buildRolePrompt(role); - expect(result).toBe("You are a planning agent."); - }); - - test("legacy: no fields at all", () => { - const role = { - description: "Minimal", - outputSchema: "placeholder00000", - } as unknown as RoleDefinition; - const result = buildRolePrompt(role); - expect(result).toBe(""); - }); - - test("mixed: identity present, partial other fields", () => { + test("empty fields are omitted", () => { const role: RoleDefinition = { description: "A reviewer", - systemPrompt: "legacy", identity: "You are a code reviewer.", - prepare: null, + prepare: "", execute: "Review the PR diff carefully.", - report: null, + report: "", outputSchema: "placeholder00000" as string, }; const result = buildRolePrompt(role); @@ -63,6 +37,18 @@ describe("buildRolePrompt", () => { expect(result).toContain("## Execute"); expect(result).not.toContain("## Prepare"); expect(result).not.toContain("## Report"); - expect(result).not.toContain("legacy"); + }); + + test("all empty returns empty string", () => { + const role: RoleDefinition = { + description: "Minimal", + identity: "", + prepare: "", + execute: "", + report: "", + outputSchema: "placeholder00000" as string, + }; + const result = buildRolePrompt(role); + expect(result).toBe(""); }); }); diff --git a/packages/workflow-agent-kit/src/build-role-prompt.ts b/packages/workflow-agent-kit/src/build-role-prompt.ts index e3ce3fe..9530766 100644 --- a/packages/workflow-agent-kit/src/build-role-prompt.ts +++ b/packages/workflow-agent-kit/src/build-role-prompt.ts @@ -3,37 +3,25 @@ import type { RoleDefinition } from "@uncaged/workflow-protocol"; /** * Build the role prompt from a RoleDefinition. * - * Four-phase mode (identity/prepare/execute/report present): - * Assembles structured sections for each phase. - * - * Legacy mode (only systemPrompt): - * Returns systemPrompt as-is. - * - * When both are present, four-phase fields take priority. + * Assembles structured sections: Identity, Prepare, Execute, Report. + * Empty strings are omitted from the output. */ export function buildRolePrompt(role: RoleDefinition): string { - const hasFourPhase = - role.identity !== null && - role.identity !== undefined && - role.identity !== ""; - - if (!hasFourPhase) { - return role.systemPrompt ?? ""; - } - const sections: string[] = []; - sections.push(`## Identity\n\n${role.identity}`); + if (role.identity !== "") { + sections.push(`## Identity\n\n${role.identity}`); + } - if (role.prepare !== null && role.prepare !== undefined && role.prepare !== "") { + if (role.prepare !== "") { sections.push(`## Prepare\n\n${role.prepare}`); } - if (role.execute !== null && role.execute !== undefined && role.execute !== "") { + if (role.execute !== "") { sections.push(`## Execute\n\n${role.execute}`); } - if (role.report !== null && role.report !== undefined && role.report !== "") { + if (role.report !== "") { sections.push(`## Report\n\n${role.report}`); } diff --git a/packages/workflow-agent-kit/src/types.ts b/packages/workflow-agent-kit/src/types.ts index 18cce91..8c57d0b 100644 --- a/packages/workflow-agent-kit/src/types.ts +++ b/packages/workflow-agent-kit/src/types.ts @@ -7,7 +7,7 @@ export type AgentContext = ModeratorContext & { store: Store; workflow: WorkflowPayload; /** - * Prepend to the role's systemPrompt when building the agent prompt. + * Prepend to the role's prompt when building the agent prompt. * Contains the frontmatter deliverable format instruction derived from the * role's output schema. Populated by `createAgent` at run time. */ diff --git a/packages/workflow-moderator/__tests__/evaluate.test.ts b/packages/workflow-moderator/__tests__/evaluate.test.ts index 61c65e8..54b6604 100644 --- a/packages/workflow-moderator/__tests__/evaluate.test.ts +++ b/packages/workflow-moderator/__tests__/evaluate.test.ts @@ -9,17 +9,26 @@ const solveIssueWorkflow: WorkflowPayload = { roles: { planner: { description: "Creates implementation plan", - systemPrompt: "You are a planning agent...", + identity: "You are a planning agent.", + prepare: "Review the issue context.", + execute: "Create a step-by-step plan.", + report: "Output the plan and steps.", outputSchema: "5GWKR8TN1V3JA", }, developer: { description: "Implements code changes", - systemPrompt: "You are a developer agent...", + identity: "You are a developer agent.", + prepare: "Load coding tools.", + execute: "Implement the plan.", + report: "List files changed and summary.", outputSchema: "8CNWT4KR6D1HV", }, reviewer: { description: "Reviews code changes", - systemPrompt: "You are a code reviewer...", + identity: "You are a code reviewer.", + prepare: "Review project conventions.", + execute: "Review the implementation.", + report: "Approve or reject with comments.", outputSchema: "1VPBG9SM5E7WK", }, }, diff --git a/packages/workflow-protocol/src/schemas.ts b/packages/workflow-protocol/src/schemas.ts index 672de87..a4e9c58 100644 --- a/packages/workflow-protocol/src/schemas.ts +++ b/packages/workflow-protocol/src/schemas.ts @@ -2,10 +2,9 @@ import type { JSONSchema } from "@uncaged/json-cas"; const ROLE_DEFINITION: JSONSchema = { type: "object", - required: ["description", "outputSchema"], + required: ["description", "identity", "prepare", "execute", "report", "outputSchema"], properties: { description: { type: "string" }, - systemPrompt: { type: "string" }, identity: { type: "string" }, prepare: { type: "string" }, execute: { type: "string" }, diff --git a/packages/workflow-protocol/src/types.ts b/packages/workflow-protocol/src/types.ts index 1996c36..6d57bc7 100644 --- a/packages/workflow-protocol/src/types.ts +++ b/packages/workflow-protocol/src/types.ts @@ -18,11 +18,10 @@ export type StepRecord = { export type RoleDefinition = { description: string; - systemPrompt: string | null; - identity: string | null; - prepare: string | null; - execute: string | null; - report: string | null; + identity: string; + prepare: string; + execute: string; + report: string; outputSchema: CasRef; };