From aa64ea86caf370213c674d433c2301475383dfd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B0=8F=E6=A9=98?= Date: Wed, 29 Apr 2026 14:23:36 +0000 Subject: [PATCH] feat: add @uncaged/nerve-role-reviewer package Extract shared reviewer role (diff analysis, convention checking) into a reusable package. Identical logic currently duplicated across develop-sense and develop-workflow workspaces. Refs RFC-004 Phase 1 --- packages/role-committer/src/index.ts | 7 +- packages/role-reviewer/package.json | 26 ++++++ packages/role-reviewer/rslib.config.ts | 19 ++++ packages/role-reviewer/src/index.ts | 2 + packages/role-reviewer/src/reviewer.ts | 91 +++++++++++++++++++ packages/role-reviewer/tsconfig.json | 9 ++ .../src/__tests__/role-decorators.test.ts | 7 +- .../workflow-utils/src/role-decorators.ts | 46 +++++----- pnpm-lock.yaml | 22 +++++ 9 files changed, 192 insertions(+), 37 deletions(-) create mode 100644 packages/role-reviewer/package.json create mode 100644 packages/role-reviewer/rslib.config.ts create mode 100644 packages/role-reviewer/src/index.ts create mode 100644 packages/role-reviewer/src/reviewer.ts create mode 100644 packages/role-reviewer/tsconfig.json diff --git a/packages/role-committer/src/index.ts b/packages/role-committer/src/index.ts index 35f9f7e..d2a084e 100644 --- a/packages/role-committer/src/index.ts +++ b/packages/role-committer/src/index.ts @@ -1,11 +1,6 @@ import type { AgentFn, Role, StartStep } from "@uncaged/nerve-core"; import type { LlmExtractorConfig } from "@uncaged/nerve-workflow-utils"; -import { - createRole, - decorateRole, - withDryRun, - onFail, -} from "@uncaged/nerve-workflow-utils"; +import { createRole, decorateRole, onFail, withDryRun } from "@uncaged/nerve-workflow-utils"; import { z } from "zod"; export const committerMetaSchema = z.object({ diff --git a/packages/role-reviewer/package.json b/packages/role-reviewer/package.json new file mode 100644 index 0000000..211f413 --- /dev/null +++ b/packages/role-reviewer/package.json @@ -0,0 +1,26 @@ +{ + "name": "@uncaged/nerve-role-reviewer", + "version": "0.5.0", + "type": "module", + "main": "dist/index.js", + "types": "dist/index.d.ts", + "files": ["dist"], + "publishConfig": { + "access": "public" + }, + "scripts": { + "prepublishOnly": "bash ../../scripts/prepublish-check.sh", + "build": "rslib build", + "test": "vitest run --passWithNoTests" + }, + "dependencies": { + "@uncaged/nerve-core": "workspace:*", + "@uncaged/nerve-workflow-utils": "workspace:*", + "zod": "^4.3.6" + }, + "devDependencies": { + "@rslib/core": "^0.21.3", + "typescript": "^5.8.3", + "vitest": "^4.1.5" + } +} diff --git a/packages/role-reviewer/rslib.config.ts b/packages/role-reviewer/rslib.config.ts new file mode 100644 index 0000000..87bb8a3 --- /dev/null +++ b/packages/role-reviewer/rslib.config.ts @@ -0,0 +1,19 @@ +import { defineConfig } from "@rslib/core"; + +export default defineConfig({ + lib: [ + { + format: "esm", + dts: true, + }, + ], + source: { + entry: { + index: "src/index.ts", + }, + }, + output: { + target: "node", + cleanDistPath: true, + }, +}); diff --git a/packages/role-reviewer/src/index.ts b/packages/role-reviewer/src/index.ts new file mode 100644 index 0000000..3e5ff76 --- /dev/null +++ b/packages/role-reviewer/src/index.ts @@ -0,0 +1,2 @@ +export { createReviewerRole, reviewerMetaSchema } from "./reviewer.js"; +export type { ReviewerMeta, ReviewerConfig } from "./reviewer.js"; diff --git a/packages/role-reviewer/src/reviewer.ts b/packages/role-reviewer/src/reviewer.ts new file mode 100644 index 0000000..6b7e512 --- /dev/null +++ b/packages/role-reviewer/src/reviewer.ts @@ -0,0 +1,91 @@ +import type { AgentFn, Role, StartStep } from "@uncaged/nerve-core"; +import type { LlmExtractorConfig } from "@uncaged/nerve-workflow-utils"; +import { createRole } from "@uncaged/nerve-workflow-utils"; +import { z } from "zod"; + +export const reviewerMetaSchema = z.object({ + approved: z.boolean().describe("true if the diff is clean and ready to merge"), +}); +export type ReviewerMeta = z.infer; + +export type ReviewerConfig = { + /** Working directory of the project */ + cwd: string; + /** Path to conventions/standards file, relative to cwd. Default: "CONVENTIONS.md" */ + conventionsPath: string | null; + /** Extra checklist items appended to the review criteria */ + extraChecks: ReadonlyArray; +}; + +const defaults: ReviewerConfig = { + cwd: ".", + conventionsPath: "CONVENTIONS.md", + extraChecks: [], +}; + +function reviewerPrompt({ + threadId, + config, +}: { threadId: string; config: ReviewerConfig }): string { + const { cwd, conventionsPath, extraChecks } = config; + + const conventionsBlock = conventionsPath + ? `Read project conventions: \`cat ${cwd}/${conventionsPath}\`\n` + : ""; + + const extraBlock = + extraChecks.length > 0 + ? `\n### 📋 Project-specific checks\n${extraChecks.map((c) => `- ${c}`).join("\n")}\n` + : ""; + + return `You are a **code reviewer**. You run after the coder and before the tester. + +**IMPORTANT: The project is at \`${cwd}\`. Always \`cd ${cwd}\` first.** + +Read the workflow thread for context: \`nerve thread ${threadId}\` +${conventionsBlock} +## Your job — static analysis of the git diff + +Run these commands and analyze the output: + +1. **\`cd ${cwd} && git diff --stat\`** — see what files changed +2. **\`cd ${cwd} && git diff\`** — read the actual diff +3. **\`cd ${cwd} && git status --short\`** — check for untracked files + +## Checklist + +### 🔴 Reject (approved: false) — tell coder exactly what to fix +- **Garbage files**: build artifacts, lockfiles, IDE config that shouldn't be committed +- **Secrets/credentials**: API keys, tokens, passwords hardcoded in the diff +- **Unrelated changes**: files modified outside the scope of the task +${conventionsPath ? `- **Convention violations**: patterns that contradict ${conventionsPath}\n` : ""}${extraBlock} +### ✅ Approve (approved: true) — no comment needed +- Diff is clean, focused, follows project standards + +End with: +\`\`\`json +{ "approved": true } +\`\`\` +or +\`\`\`json +{ "approved": false } +\`\`\``; +} + +/** + * Creates a reviewer role that performs static analysis on git diffs. + * Checks for garbage files, secrets, unrelated changes, and project conventions. + */ +export function createReviewerRole( + adapter: AgentFn, + extract: LlmExtractorConfig, + config: Partial = {}, +): Role { + const resolved: ReviewerConfig = { ...defaults, ...config }; + return createRole( + adapter, + async (start: StartStep) => reviewerPrompt({ threadId: start.meta.threadId, config: resolved }), + reviewerMetaSchema, + extract, + ); +} diff --git a/packages/role-reviewer/tsconfig.json b/packages/role-reviewer/tsconfig.json new file mode 100644 index 0000000..9036088 --- /dev/null +++ b/packages/role-reviewer/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "../../tsconfig.json", + "compilerOptions": { + "outDir": "dist", + "rootDir": "src", + "composite": false + }, + "include": ["src"] +} diff --git a/packages/workflow-utils/src/__tests__/role-decorators.test.ts b/packages/workflow-utils/src/__tests__/role-decorators.test.ts index 894c273..ecf91e1 100644 --- a/packages/workflow-utils/src/__tests__/role-decorators.test.ts +++ b/packages/workflow-utils/src/__tests__/role-decorators.test.ts @@ -1,11 +1,6 @@ import { describe, expect, it } from "vitest"; -import type { - Role, - RoleResult, - StartStep, - WorkflowMessage, -} from "@uncaged/nerve-core"; +import type { Role, StartStep } from "@uncaged/nerve-core"; import { START } from "@uncaged/nerve-core"; import { decorateRole, onFail, withDryRun } from "../role-decorators.js"; diff --git a/packages/workflow-utils/src/role-decorators.ts b/packages/workflow-utils/src/role-decorators.ts index a5cec4e..bc31cd4 100644 --- a/packages/workflow-utils/src/role-decorators.ts +++ b/packages/workflow-utils/src/role-decorators.ts @@ -7,9 +7,7 @@ import { isDryRun } from "./role-types.js"; // --------------------------------------------------------------------------- /** A role decorator: takes a role, returns an enhanced role. */ -export type RoleDecorator> = ( - role: Role, -) => Role; +export type RoleDecorator> = (role: Role) => Role; // --------------------------------------------------------------------------- // decorateRole — compose a chain of decorators @@ -49,16 +47,15 @@ export type WithDryRunOptions = { export function withDryRun>( opts: WithDryRunOptions, ): RoleDecorator { - return (role) => - async (start: StartStep, messages: WorkflowMessage[]) => { - if (isDryRun(start)) { - return { - content: `[dry-run] ${opts.label} skipped`, - meta: opts.meta, - }; - } - return role(start, messages); - }; + return (role) => async (start: StartStep, messages: WorkflowMessage[]) => { + if (isDryRun(start)) { + return { + content: `[dry-run] ${opts.label} skipped`, + meta: opts.meta, + }; + } + return role(start, messages); + }; } // --------------------------------------------------------------------------- @@ -79,16 +76,15 @@ export type OnFailOptions = { export function onFail>( opts: OnFailOptions, ): RoleDecorator { - return (role) => - async (start: StartStep, messages: WorkflowMessage[]) => { - try { - return await role(start, messages); - } catch (e) { - const msg = e instanceof Error ? e.message : String(e); - return { - content: `${opts.label} failed: ${msg}`, - meta: opts.meta, - }; - } - }; + return (role) => async (start: StartStep, messages: WorkflowMessage[]) => { + try { + return await role(start, messages); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + return { + content: `${opts.label} failed: ${msg}`, + meta: opts.meta, + }; + } + }; } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 75beb57..8de2ebc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -178,6 +178,28 @@ importers: specifier: ^4.1.5 version: 4.1.5(@types/node@25.6.0)(vite@8.0.9(@types/node@25.6.0)(esbuild@0.27.7)(yaml@2.8.3)) + packages/role-reviewer: + dependencies: + '@uncaged/nerve-core': + specifier: workspace:* + version: link:../core + '@uncaged/nerve-workflow-utils': + specifier: workspace:* + version: link:../workflow-utils + zod: + specifier: ^4.3.6 + version: 4.3.6 + devDependencies: + '@rslib/core': + specifier: ^0.21.3 + version: 0.21.3(typescript@5.9.3) + typescript: + specifier: ^5.8.3 + version: 5.9.3 + vitest: + specifier: ^4.1.5 + version: 4.1.5(@types/node@25.6.0)(vite@8.0.9(@types/node@25.6.0)(esbuild@0.27.7)(yaml@2.8.3)) + packages/skills: {} packages/store: