From daf07b5746182ed732bc16016f916e3e4ba4092b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B0=8F=E6=A9=98?= Date: Tue, 28 Apr 2026 13:56:37 +0000 Subject: [PATCH] feat: add reviewer role to all three workflows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - workflow-generator, sense-generator, gitea-issue-solver all now have: planner → coder → reviewer → tester → committer → END - Reviewer uses createHermesRole with git diff/status for static analysis - Checks: garbage files, secrets, debug code, unrelated changes - Planner prompt now requires Role Behavior sections for every role - Coder prompt now emphasizes reading initial user prompt for specifics --- workflows/gitea-issue-solver/build.ts | 6 ++- workflows/gitea-issue-solver/index.ts | 5 +- workflows/gitea-issue-solver/moderator.ts | 10 +++- .../roles/reviewer/index.ts | 21 +++++++++ .../roles/reviewer/prompt.ts | 46 +++++++++++++++++++ workflows/sense-generator/build.ts | 2 + workflows/sense-generator/moderator.ts | 8 +++- .../sense-generator/roles/reviewer/index.ts | 21 +++++++++ .../sense-generator/roles/reviewer/prompt.ts | 46 +++++++++++++++++++ workflows/workflow-generator/build.ts | 2 +- .../workflow-generator/roles/coder/prompt.ts | 5 ++ .../roles/planner/prompt.ts | 6 +++ .../roles/reviewer/index.ts | 14 +++--- .../roles/reviewer/prompt.ts | 45 +++++++++++++----- 14 files changed, 212 insertions(+), 25 deletions(-) create mode 100644 workflows/gitea-issue-solver/roles/reviewer/index.ts create mode 100644 workflows/gitea-issue-solver/roles/reviewer/prompt.ts create mode 100644 workflows/sense-generator/roles/reviewer/index.ts create mode 100644 workflows/sense-generator/roles/reviewer/prompt.ts diff --git a/workflows/gitea-issue-solver/build.ts b/workflows/gitea-issue-solver/build.ts index 72f0ae5..c102767 100644 --- a/workflows/gitea-issue-solver/build.ts +++ b/workflows/gitea-issue-solver/build.ts @@ -1,18 +1,21 @@ import type { WorkflowDefinition } from "@uncaged/nerve-core"; +import type { LlmProvider } from "@uncaged/nerve-workflow-utils"; import { moderator } from "./moderator.js"; import type { WorkflowMeta } from "./moderator.js"; import { buildIntakeRole } from "./roles/intake/index.js"; import { buildIssueReaderRole } from "./roles/issue-reader/index.js"; import { buildPlannerRole } from "./roles/planner/index.js"; import { buildImplementerRole } from "./roles/implementer/index.js"; +import { buildReviewerRole } from "./roles/reviewer/index.js"; import { buildTesterRole } from "./roles/tester/index.js"; import { buildPrPublisherRole } from "./roles/pr-publisher/index.js"; export type BuildGiteaIssueSolverDeps = { nerveRoot: string; + provider: LlmProvider | null; }; -export function buildGiteaIssueSolver({ nerveRoot }: BuildGiteaIssueSolverDeps): WorkflowDefinition { +export function buildGiteaIssueSolver({ nerveRoot, provider }: BuildGiteaIssueSolverDeps): WorkflowDefinition { return { name: "gitea-issue-solver", roles: { @@ -20,6 +23,7 @@ export function buildGiteaIssueSolver({ nerveRoot }: BuildGiteaIssueSolverDeps): "issue-reader": buildIssueReaderRole({ nerveRoot }), planner: buildPlannerRole({ nerveRoot }), implementer: buildImplementerRole({ nerveRoot }), + reviewer: provider ? buildReviewerRole({ provider, nerveRoot }) : buildReviewerRole({ provider: { apiKey: "", baseUrl: "", model: "" }, nerveRoot }), tester: buildTesterRole({ nerveRoot }), "pr-publisher": buildPrPublisherRole({ nerveRoot }), }, diff --git a/workflows/gitea-issue-solver/index.ts b/workflows/gitea-issue-solver/index.ts index d254701..bd1edd6 100644 --- a/workflows/gitea-issue-solver/index.ts +++ b/workflows/gitea-issue-solver/index.ts @@ -1,9 +1,12 @@ import { join } from "node:path"; import { buildGiteaIssueSolver } from "./build.js"; +import { resolveDashScopeProvider } from "./lib/provider.js"; const HOME = process.env.HOME ?? "/home/azureuser"; const NERVE_ROOT = join(HOME, ".uncaged-nerve"); -const workflow = buildGiteaIssueSolver({ nerveRoot: NERVE_ROOT }); +const provider = await resolveDashScopeProvider(NERVE_ROOT); + +const workflow = buildGiteaIssueSolver({ nerveRoot: NERVE_ROOT, provider }); export default workflow; diff --git a/workflows/gitea-issue-solver/moderator.ts b/workflows/gitea-issue-solver/moderator.ts index fc5ab51..10b9afd 100644 --- a/workflows/gitea-issue-solver/moderator.ts +++ b/workflows/gitea-issue-solver/moderator.ts @@ -5,6 +5,7 @@ import type { IntakeMeta } from "./roles/intake/index.js"; import type { IssueReaderMeta } from "./roles/issue-reader/index.js"; import type { PlannerMeta } from "./roles/planner/index.js"; import type { ImplementerMeta } from "./roles/implementer/index.js"; +import type { ReviewerMeta } from "./roles/reviewer/index.js"; import type { TesterMeta } from "./roles/tester/index.js"; import type { PrPublisherMeta } from "./roles/pr-publisher/index.js"; @@ -13,6 +14,7 @@ export type WorkflowMeta = { "issue-reader": IssueReaderMeta; planner: PlannerMeta; implementer: ImplementerMeta; + reviewer: ReviewerMeta; tester: TesterMeta; "pr-publisher": PrPublisherMeta; }; @@ -42,7 +44,13 @@ export const moderator: Moderator = (context) => { } if (last.role === "implementer") { - return "tester"; + return "reviewer"; + } + + if (last.role === "reviewer") { + const meta = last.meta as WorkflowMeta["reviewer"]; + if (meta.approved) return "tester"; + return "implementer"; } if (last.role === "tester") { diff --git a/workflows/gitea-issue-solver/roles/reviewer/index.ts b/workflows/gitea-issue-solver/roles/reviewer/index.ts new file mode 100644 index 0000000..9148076 --- /dev/null +++ b/workflows/gitea-issue-solver/roles/reviewer/index.ts @@ -0,0 +1,21 @@ +import type { LlmProvider } from "@uncaged/nerve-workflow-utils"; +import { createHermesRole } from "@uncaged/nerve-workflow-utils"; +import { reviewerPrompt } from "./prompt.js"; +import { z } from "zod"; + +export const reviewerMetaSchema = z.object({ + approved: z.boolean().describe("true if the diff is clean and ready for tester validation"), +}); +export type ReviewerMeta = z.infer; + +export type BuildReviewerDeps = { + provider: LlmProvider; + nerveRoot: string; +}; + +export function buildReviewerRole({ provider, nerveRoot }: BuildReviewerDeps) { + return createHermesRole({ + prompt: async (threadId) => reviewerPrompt({ threadId, nerveRoot }), + extract: { provider, schema: reviewerMetaSchema }, + }); +} diff --git a/workflows/gitea-issue-solver/roles/reviewer/prompt.ts b/workflows/gitea-issue-solver/roles/reviewer/prompt.ts new file mode 100644 index 0000000..6532961 --- /dev/null +++ b/workflows/gitea-issue-solver/roles/reviewer/prompt.ts @@ -0,0 +1,46 @@ +export function reviewerPrompt({ threadId, nerveRoot }: { threadId: string; nerveRoot: string }): string { + return `You are a **code reviewer** for Nerve workflow changes. You run after the coder and before the tester. + +**IMPORTANT: The Nerve workspace is at \`${nerveRoot}\`. Always \`cd ${nerveRoot}\` first.** + +Read the workflow thread for context: \`nerve thread ${threadId}\` + +## Your job — static analysis of the git diff + +Run these commands and analyze the output: + +1. **\`cd ${nerveRoot} && git diff --stat\`** — see what files changed +2. **\`cd ${nerveRoot} && git diff\`** — read the actual diff +3. **\`cd ${nerveRoot} && git status --short\`** — check for untracked files + +## Checklist + +### 🔴 Must reject (approved: false) +- **Garbage files**: node_modules/, .DS_Store, pnpm cache artifacts (e.g. \`false/\` directory), build outputs (dist/) that shouldn't be committed +- **Secrets/credentials**: API keys, tokens, passwords hardcoded in the diff +- **Unrelated changes**: files modified outside the scope of the planner's design + +### ⚠️ Flag but may still approve +- Debug code left behind (console.log, debugger, TODO/FIXME without context) +- Hardcoded paths that should be configurable +- Missing type exports that other files might need + +### ✅ Verify +- All files from the planner's design are created/modified +- Code follows existing patterns (compare with sibling workflows) +- Meta types are simple routing signals (single boolean per role) +- No dynamic import() + +## Output + +Summarize what you found. If garbage files or secrets are present, list them explicitly so the coder knows what to clean up. + +End with: +\`\`\`json +{ "approved": true } +\`\`\` +or +\`\`\`json +{ "approved": false } +\`\`\``; +} diff --git a/workflows/sense-generator/build.ts b/workflows/sense-generator/build.ts index 4754db0..55b5917 100644 --- a/workflows/sense-generator/build.ts +++ b/workflows/sense-generator/build.ts @@ -2,6 +2,7 @@ import type { WorkflowDefinition } from "@uncaged/nerve-core"; import type { LlmProvider } from "@uncaged/nerve-workflow-utils"; import { buildPlannerRole } from "./roles/planner/index.js"; import { buildCoderRole } from "./roles/coder/index.js"; +import { buildReviewerRole } from "./roles/reviewer/index.js"; import { buildTesterRole } from "./roles/tester/index.js"; import { buildCommitterRole } from "./roles/committer/index.js"; import { moderator } from "./moderator.js"; @@ -21,6 +22,7 @@ export function buildSenseGenerator({ roles: { planner: buildPlannerRole({ provider, cwd }), coder: buildCoderRole({ provider, cwd }), + reviewer: buildReviewerRole({ provider, nerveRoot: cwd }), tester: buildTesterRole({ provider, nerveRoot: cwd }), committer: buildCommitterRole({ nerveRoot: cwd }), }, diff --git a/workflows/sense-generator/moderator.ts b/workflows/sense-generator/moderator.ts index c3e6e47..3dd09de 100644 --- a/workflows/sense-generator/moderator.ts +++ b/workflows/sense-generator/moderator.ts @@ -2,12 +2,14 @@ import { END } from "@uncaged/nerve-core"; import type { Moderator } from "@uncaged/nerve-core"; import type { PlannerMeta } from "./roles/planner/index.js"; import type { CoderMeta } from "./roles/coder/index.js"; +import type { ReviewerMeta } from "./roles/reviewer/index.js"; import type { TesterMeta } from "./roles/tester/index.js"; import type { CommitterMeta } from "./roles/committer/index.js"; export type SenseMeta = { planner: PlannerMeta; coder: CoderMeta; + reviewer: ReviewerMeta; tester: TesterMeta; committer: CommitterMeta; }; @@ -24,7 +26,11 @@ export const moderator: Moderator = (context) => { const coderCount = context.steps.filter((s) => s.role === "coder").length; if (last.role === "planner") return "coder"; - if (last.role === "coder") return "tester"; + if (last.role === "coder") return "reviewer"; + if (last.role === "reviewer") { + if (last.meta.approved) return "tester"; + return coderCount < MAX_CODER_ITERATIONS ? "coder" : END; + } if (last.role === "tester") { if (last.meta.passed) return "committer"; const testerCount = countRole(context.steps, "tester"); diff --git a/workflows/sense-generator/roles/reviewer/index.ts b/workflows/sense-generator/roles/reviewer/index.ts new file mode 100644 index 0000000..9148076 --- /dev/null +++ b/workflows/sense-generator/roles/reviewer/index.ts @@ -0,0 +1,21 @@ +import type { LlmProvider } from "@uncaged/nerve-workflow-utils"; +import { createHermesRole } from "@uncaged/nerve-workflow-utils"; +import { reviewerPrompt } from "./prompt.js"; +import { z } from "zod"; + +export const reviewerMetaSchema = z.object({ + approved: z.boolean().describe("true if the diff is clean and ready for tester validation"), +}); +export type ReviewerMeta = z.infer; + +export type BuildReviewerDeps = { + provider: LlmProvider; + nerveRoot: string; +}; + +export function buildReviewerRole({ provider, nerveRoot }: BuildReviewerDeps) { + return createHermesRole({ + prompt: async (threadId) => reviewerPrompt({ threadId, nerveRoot }), + extract: { provider, schema: reviewerMetaSchema }, + }); +} diff --git a/workflows/sense-generator/roles/reviewer/prompt.ts b/workflows/sense-generator/roles/reviewer/prompt.ts new file mode 100644 index 0000000..6532961 --- /dev/null +++ b/workflows/sense-generator/roles/reviewer/prompt.ts @@ -0,0 +1,46 @@ +export function reviewerPrompt({ threadId, nerveRoot }: { threadId: string; nerveRoot: string }): string { + return `You are a **code reviewer** for Nerve workflow changes. You run after the coder and before the tester. + +**IMPORTANT: The Nerve workspace is at \`${nerveRoot}\`. Always \`cd ${nerveRoot}\` first.** + +Read the workflow thread for context: \`nerve thread ${threadId}\` + +## Your job — static analysis of the git diff + +Run these commands and analyze the output: + +1. **\`cd ${nerveRoot} && git diff --stat\`** — see what files changed +2. **\`cd ${nerveRoot} && git diff\`** — read the actual diff +3. **\`cd ${nerveRoot} && git status --short\`** — check for untracked files + +## Checklist + +### 🔴 Must reject (approved: false) +- **Garbage files**: node_modules/, .DS_Store, pnpm cache artifacts (e.g. \`false/\` directory), build outputs (dist/) that shouldn't be committed +- **Secrets/credentials**: API keys, tokens, passwords hardcoded in the diff +- **Unrelated changes**: files modified outside the scope of the planner's design + +### ⚠️ Flag but may still approve +- Debug code left behind (console.log, debugger, TODO/FIXME without context) +- Hardcoded paths that should be configurable +- Missing type exports that other files might need + +### ✅ Verify +- All files from the planner's design are created/modified +- Code follows existing patterns (compare with sibling workflows) +- Meta types are simple routing signals (single boolean per role) +- No dynamic import() + +## Output + +Summarize what you found. If garbage files or secrets are present, list them explicitly so the coder knows what to clean up. + +End with: +\`\`\`json +{ "approved": true } +\`\`\` +or +\`\`\`json +{ "approved": false } +\`\`\``; +} diff --git a/workflows/workflow-generator/build.ts b/workflows/workflow-generator/build.ts index 320db26..06b46e4 100644 --- a/workflows/workflow-generator/build.ts +++ b/workflows/workflow-generator/build.ts @@ -23,7 +23,7 @@ export function buildWorkflowGenerator({ roles: { planner: buildPlannerRole({ provider, cwd: nerveRoot }), coder: buildCoderRole({ provider, cwd: nerveRoot }), - reviewer: buildReviewerRole({ provider, cwd: nerveRoot }), + reviewer: buildReviewerRole({ provider, nerveRoot }), tester: buildTesterRole({ provider, nerveRoot }), committer: buildCommitterRole({ nerveRoot }), }, diff --git a/workflows/workflow-generator/roles/coder/prompt.ts b/workflows/workflow-generator/roles/coder/prompt.ts index 527abf1..85ce857 100644 --- a/workflows/workflow-generator/roles/coder/prompt.ts +++ b/workflows/workflow-generator/roles/coder/prompt.ts @@ -7,6 +7,11 @@ Also look at existing workflows in the \`workflows/\` directory for patterns. Implement the planner's design. This may be **creating a new workflow** or **modifying an existing one**. If there is reviewer, tester, or committer feedback in the thread, fix the issues they identified. +**IMPORTANT:** The thread contains both the **initial user prompt** (the first message) and the **planner's design**. Read both carefully: +- The **initial prompt** contains the user's specific requirements for role behavior, tools to use, and acceptance criteria +- The **planner's design** contains the architecture, file structure, and routing logic +- When writing role prompts, follow the user's behavioral requirements from the initial prompt — do not invent your own interpretation + ## Multi-step approach You do NOT need to finish everything in one pass. You may return \`done: false\` to continue in the next iteration. For example: diff --git a/workflows/workflow-generator/roles/planner/prompt.ts b/workflows/workflow-generator/roles/planner/prompt.ts index 13c6480..77e1e23 100644 --- a/workflows/workflow-generator/roles/planner/prompt.ts +++ b/workflows/workflow-generator/roles/planner/prompt.ts @@ -27,6 +27,12 @@ For **modifications to existing workflows**: - Impact on moderator routing logic (this workflow's typical order is planner → coder → reviewer → tester → committer) - Backward compatibility considerations (if any) +**For every role (new or modified)**, include a **Role Behavior** section that describes: +- What the role should do, check, or produce +- What tools or commands it should use +- What criteria determine its meta output (e.g. approved/passed/done) +- Preserve the user's specific requirements verbatim — do NOT summarize away details + If requirements are NOT clear, describe what is missing or ambiguous. End your response with a JSON block: diff --git a/workflows/workflow-generator/roles/reviewer/index.ts b/workflows/workflow-generator/roles/reviewer/index.ts index e6fee58..9148076 100644 --- a/workflows/workflow-generator/roles/reviewer/index.ts +++ b/workflows/workflow-generator/roles/reviewer/index.ts @@ -1,23 +1,21 @@ import type { LlmProvider } from "@uncaged/nerve-workflow-utils"; -import { createCursorRole } from "@uncaged/nerve-workflow-utils"; +import { createHermesRole } from "@uncaged/nerve-workflow-utils"; import { reviewerPrompt } from "./prompt.js"; import { z } from "zod"; export const reviewerMetaSchema = z.object({ - approved: z.boolean().describe("true if the workflow matches the plan and is ready for tester validation"), + approved: z.boolean().describe("true if the diff is clean and ready for tester validation"), }); export type ReviewerMeta = z.infer; export type BuildReviewerDeps = { provider: LlmProvider; - cwd: string; + nerveRoot: string; }; -export function buildReviewerRole({ provider, cwd }: BuildReviewerDeps) { - return createCursorRole({ - cwd, - mode: "ask", - prompt: async (threadId) => reviewerPrompt({ threadId }), +export function buildReviewerRole({ provider, nerveRoot }: BuildReviewerDeps) { + return createHermesRole({ + prompt: async (threadId) => reviewerPrompt({ threadId, nerveRoot }), extract: { provider, schema: reviewerMetaSchema }, }); } diff --git a/workflows/workflow-generator/roles/reviewer/prompt.ts b/workflows/workflow-generator/roles/reviewer/prompt.ts index cdfede4..6532961 100644 --- a/workflows/workflow-generator/roles/reviewer/prompt.ts +++ b/workflows/workflow-generator/roles/reviewer/prompt.ts @@ -1,20 +1,41 @@ -export function reviewerPrompt({ threadId }: { threadId: string }): string { - return `You are a **reviewer** for Nerve workflow generation. You run **after** the coder and **before** the tester. +export function reviewerPrompt({ threadId, nerveRoot }: { threadId: string; nerveRoot: string }): string { + return `You are a **code reviewer** for Nerve workflow changes. You run after the coder and before the tester. -Read the workflow thread for the planner's design and any prior feedback: \`nerve thread ${threadId}\` -Read workflow conventions: \`cat node_modules/@uncaged/nerve-skills/nerve-dev/SKILL.md\` (from the repo root after \`cd\`). +**IMPORTANT: The Nerve workspace is at \`${nerveRoot}\`. Always \`cd ${nerveRoot}\` first.** -## Your job +Read the workflow thread for context: \`nerve thread ${threadId}\` -1. Identify the target workflow name and paths from the thread (e.g. \`workflows//\`). -2. Read the relevant files: \`moderator.ts\`, \`build.ts\`, \`index.ts\`, and each role under \`roles/\`. -3. Check that the implementation matches the planner's plan (roles, routing, meta types, no forbidden patterns like dynamic \`import()\`). -4. Do **not** run full build/test here — that is the tester's job. Flag obvious gaps (missing files, wrong exports, broken routing). +## Your job — static analysis of the git diff -If the work is **ready for the tester** to validate builds and config, set \`approved: true\`. -If the coder must fix issues first, set \`approved: false\` and explain briefly what is wrong. +Run these commands and analyze the output: -End with a JSON block: +1. **\`cd ${nerveRoot} && git diff --stat\`** — see what files changed +2. **\`cd ${nerveRoot} && git diff\`** — read the actual diff +3. **\`cd ${nerveRoot} && git status --short\`** — check for untracked files + +## Checklist + +### 🔴 Must reject (approved: false) +- **Garbage files**: node_modules/, .DS_Store, pnpm cache artifacts (e.g. \`false/\` directory), build outputs (dist/) that shouldn't be committed +- **Secrets/credentials**: API keys, tokens, passwords hardcoded in the diff +- **Unrelated changes**: files modified outside the scope of the planner's design + +### ⚠️ Flag but may still approve +- Debug code left behind (console.log, debugger, TODO/FIXME without context) +- Hardcoded paths that should be configurable +- Missing type exports that other files might need + +### ✅ Verify +- All files from the planner's design are created/modified +- Code follows existing patterns (compare with sibling workflows) +- Meta types are simple routing signals (single boolean per role) +- No dynamic import() + +## Output + +Summarize what you found. If garbage files or secrets are present, list them explicitly so the coder knows what to clean up. + +End with: \`\`\`json { "approved": true } \`\`\`