feat(T19): add code review functionality (stage 1)

.env.example

@@ -3,6 +3,8 @@ ANTHROPIC_API_KEY=your_anthropic_api_key_here
 # GitHub
 GITHUB_TOKEN=your_github_personal_access_token_here
 TARGET_GITHUB_USERNAME=your_github_username_here
+# PR review: absolute path to a local clone of the repo whose PRs you review
+REPO_PATH=/absolute/path/to/local/clone/of/the/repo
 # Your personal oh-my-workers DB
 DATABASE_URL=postgresql://user:password@localhost:5432/work_coordinator
 # Company DB (update these when you change jobs)

.gitignore

@@ -5,3 +5,4 @@ data/
 wiki/
 src/assets/slides_to_video/
 src/scripts/test-registry.ts
+dist/

package.json

@@ -13,6 +13,7 @@
     "news": "pnpm run dev --job=news",
     "jobs": "pnpm run dev --list-jobs",
     "seed-mock": "node --loader ts-node/esm src/scripts/seed-mock-users.ts",
+    "review:dev": "node --loader ts-node/esm src/scripts/run-pr-review.ts",
     "format": "prettier --write \"src/**/*.ts\"",
     "format:check": "prettier --check \"src/**/*.ts\""
   },

src/agent/pr-review.agent.ts

@@ -0,0 +1,68 @@
+import { createAgent, toolCallLimitMiddleware } from 'langchain'
+import { ChatAnthropic } from '@langchain/anthropic'
+import { AIMessage } from '@langchain/core/messages'
+import { getPrDiffTool, readFileTool, searchCodeTool } from '../tools/pr-review.tool.js'
+import { PR_REVIEW_PROMPT } from './prompt.js'
+// import { REVIEW_LLM, MAX_REVIEW_TOOL_CALLS } from '../constants/index.js'
+import { DEFAULT_LLM, MAX_REVIEW_TOOL_CALLS } from '../constants/index.js'
+import { ReviewResultSchema, type ReviewResult } from '../schemas/index.js'
+import { parsePrUrl } from '../utils/pr-url.js'
+
+const llm = new ChatAnthropic({ model: DEFAULT_LLM, temperature: 0 })
+
+// Unlike the single-call fetch agents, the reviewer loops: read the diff, pull in
+// context with read_file/search_code, then judge. The tool-call limit caps that loop.
+//
+// NOTE: we deliberately do NOT use createAgent's `responseFormat` here. In this langchain
+// build, combining structured-output machinery with an afterModel middleware produces
+// concurrent writes to the internal `jumpTo` channel (INVALID_CONCURRENT_GRAPH_UPDATE).
+// Instead the agent returns a plain-text review, and we convert it to the typed
+// ReviewResult in a separate withStructuredOutput call below.
+export const prReviewAgent = createAgent({
+  model: llm,
+  tools: [getPrDiffTool, readFileTool, searchCodeTool],
+  systemPrompt: PR_REVIEW_PROMPT,
+  middleware: [toolCallLimitMiddleware({ runLimit: MAX_REVIEW_TOOL_CALLS, exitBehavior: 'end' })],
+})
+
+// A separate, single-shot model call that turns the agent's free-text review into the
+// typed ReviewResult. Runs outside the agent graph, so it avoids the jumpTo conflict.
+const structuredLlm = new ChatAnthropic({ model: DEFAULT_LLM, temperature: 0 }).withStructuredOutput(ReviewResultSchema)
+
+// Extract the text of the agent's final assistant message (content may be a string or
+// an array of content blocks).
+function finalMessageText(messages: Array<{ content: unknown }>): string {
+  const last = [...messages].reverse().find((m) => AIMessage.isInstance(m))
+  const content = last?.content
+
+  if (typeof content === 'string') return content
+  if (Array.isArray(content)) {
+    return content.map((block) => (typeof block === 'object' && block !== null && 'text' in block ? (block as { text: string }).text : '')).join('')
+  }
+
+  return ''
+}
+
+// Orchestrator shared by the Task 1 dev script and the future Task 2 CLI.
+export async function reviewPullRequest(prUrl: string): Promise<ReviewResult> {
+  const { owner, repo, number } = parsePrUrl(prUrl)
+
+  console.log(`🔍 Reviewing ${owner}/${repo} PR #${number} ...\n`)
+
+  const result = await prReviewAgent.invoke({
+    messages: [
+      {
+        role: 'user',
+        content: `Review this pull request. owner="${owner}", repo="${repo}", prNumber=${number}. Start by calling get_pr_diff, then investigate the surrounding code before reporting any issues.`,
+      },
+    ],
+  })
+
+  const reviewText = finalMessageText(result.messages)
+
+  console.log('\n🧩 Structuring findings...\n')
+
+  return structuredLlm.invoke(
+    `Convert the following code-review notes into the structured schema. Preserve every finding exactly; do not invent new ones.\n\n${reviewText}`
+  )
+}

src/agent/prompt.ts

@@ -54,3 +54,29 @@ When curating:
 Call curate_trending_repos with the result immediately.`
 
 export const TRENDING_TELEGRAM_PROMPT = `You are a Telegram delivery agent. Your only job is to call send_trending_telegram with the repos provided. Send it immediately and return the result.`
+
+export const PR_REVIEW_PROMPT = `You are a senior code reviewer with deep expertise in TypeScript, JavaScript, Node.js, Vue.js and NestJS. You review a single pull request and report only genuine BUGS, SECURITY issues, and PERFORMANCE problems. Do NOT comment on style, formatting, or naming.
+
+You have three tools:
+- get_pr_diff — fetch the PR's changed files and diffs. Call this FIRST.
+- read_file — read any file from the local checkout (callers, definitions, types, config).
+- search_code — find where a symbol is defined or used across the codebase.
+
+How to review — investigate before judging:
+1. Call get_pr_diff to see exactly what changed.
+2. For each meaningful change, DO NOT judge from the diff alone. Pull in context first:
+   - read_file the file being changed (the diff only shows a slice) and the files/functions/types it touches.
+   - search_code for callers and definitions to understand how the changed code is actually used.
+3. Only after you understand the surrounding code, decide whether something is a real issue. Examples worth reporting:
+   - Bugs: null/undefined dereferences, wrong/edge-case logic, broken contracts with callers, unhandled promise rejections, incorrect async/await, off-by-one, type mismatches that survive at runtime.
+   - Security: injection (SQL/command/XSS), missing authz/authn, secret/PII leakage, unsafe deserialization, SSRF, path traversal.
+   - Performance: N+1 queries, unbounded loops/allocations, blocking the event loop, missing pagination, redundant network/db calls.
+
+Rules:
+- Report a finding ONLY if you are confident it is a real problem after inspecting the surrounding code. When in doubt, leave it out — false positives waste the reviewer's time.
+- Every finding must cite the specific file and line in the changed code.
+- Explain WHY it is a problem, referencing the context you read (e.g. "parseUser() returns null when the row is missing — see src/db/user.ts:42").
+- Give a concrete suggested fix.
+- If you find no genuine issues, say so clearly.
+
+When done investigating, write your final answer as plain text: a one-paragraph overall summary, then a numbered list of findings. For EACH finding include severity (critical/high/medium/low), category (bug/security/performance), the file path and line number, a short title, an explanation of why it is a real problem, and a concrete suggested fix.`

src/constants/index.ts

@@ -1,4 +1,6 @@
-export const DEFAULT_LLM = 'claude-haiku-4-5-20251001'
+export const DEFAULT_LLM = 'claude-haiku-4-5'
+export const REVIEW_LLM = 'claude-sonnet-4-6' // stronger model for code review reasoning (bugs/security/performance)
+export const MAX_REVIEW_TOOL_CALLS = 30 // cap how many times the review agent can read/search context before deciding
 export const COMPANY_CLEANUP_TABLE = 'mockTestUsers'
 export const COMPANY_CLEANUP_THRESHOLD_DAYS = '30'
 export const DEFAULT_CRONJOB_TIME = '0 17 * * *'

src/schemas/index.ts

@@ -78,6 +78,22 @@ export const TrendingRepoLogSchema = TrendingRepoSchema.extend({
   updated_at: z.string(),
 })
 
+// --- PR code review (bugs + security + performance) ---
+export const ReviewFindingSchema = z.object({
+  severity: z.enum(['critical', 'high', 'medium', 'low']),
+  category: z.enum(['bug', 'security', 'performance']),
+  file: z.string().describe('Repo-relative path of the file the issue is in'),
+  line: z.number().describe('Line number in the changed file where the issue occurs'),
+  title: z.string().describe('Short one-line summary of the issue'),
+  explanation: z.string().describe('Why this is a real issue, referencing the surrounding code that was inspected'),
+  suggestion: z.string().describe('Concrete fix or change to resolve the issue'),
+})
+
+export const ReviewResultSchema = z.object({
+  summary: z.string().describe('One-paragraph overall assessment of the PR'),
+  findings: z.array(ReviewFindingSchema),
+})
+
 // TypeScript types inferred from schemas
 export type GitHubDigest = z.infer<typeof GitHubDigestSchema>
 export type GithubKpiInput = z.infer<typeof GithubKpiInputSchema>
@@ -87,3 +103,5 @@ export type DiaryEntry = z.infer<typeof DiaryEntrySchema>
 export type CleanupResult = z.infer<typeof CleanupResultSchema>
 export type TrendingRepo = z.infer<typeof TrendingRepoSchema>
 export type TrendingRepoLog = z.infer<typeof TrendingRepoLogSchema>
+export type ReviewFinding = z.infer<typeof ReviewFindingSchema>
+export type ReviewResult = z.infer<typeof ReviewResultSchema>

src/scripts/run-pr-review.ts

@@ -0,0 +1,17 @@
+import 'dotenv/config'
+import { reviewPullRequest } from '../agent/pr-review.agent.js'
+import { formatReview } from '../utils/review-formatter.js'
+
+// Task 1: hardcode the PR URL here. CLI arg parsing comes in Task 2.
+// Use a real PR you can access; make sure REPO_PATH points at a local clone with the PR's branch checked out.
+const PR_URL = 'https://github.com/DamengRandom/oh-my-workers/pull/3/changes'
+
+async function main(): Promise<void> {
+  const result = await reviewPullRequest(PR_URL)
+  console.log('\n' + formatReview(result) + '\n')
+}
+
+main().catch((err) => {
+  console.error('Fatal error during PR review:', err instanceof Error ? err.message : err)
+  process.exit(1)
+})

src/tools/pr-review.tool.ts

@@ -0,0 +1,148 @@
+import { DynamicStructuredTool } from '@langchain/core/tools'
+import { Octokit } from '@octokit/rest'
+import { execFile } from 'node:child_process'
+import { promisify } from 'node:util'
+import { readFile } from 'node:fs/promises'
+import { realpathSync } from 'node:fs'
+import path from 'node:path'
+import { z } from 'zod'
+
+const execFileAsync = promisify(execFile)
+
+const octokit = new Octokit({ auth: process.env.GITHUB_TOKEN })
+
+// Resolve the local clone root once. Every file/search tool is sandboxed to this directory.
+function repoRoot(): string {
+  const root = process.env.REPO_PATH
+  if (!root) throw new Error('REPO_PATH is not set — point it at the local clone of the repo being reviewed.')
+  return path.resolve(root)
+}
+
+// Resolve a repo-relative path and refuse anything that escapes the clone (path-traversal guard).
+// path.resolve() is purely lexical and does NOT follow symlinks, so we resolve the real path
+// (via realpathSync) before checking containment — otherwise a symlink committed inside the repo
+// and pointing outside (e.g. link -> /etc) could be used to read files beyond REPO_PATH.
+function resolveInsideRepo(relativePath: string): string {
+  const root = realpathSync(repoRoot())
+  const resolved = path.resolve(root, relativePath)
+
+  // Follow symlinks for paths that exist. If the path doesn't exist there is nothing to read,
+  // so the lexical check on `resolved` is sufficient (path.resolve already normalises any '..').
+  let real = resolved
+  try {
+    real = realpathSync(resolved)
+  } catch {
+    // non-existent path — fall through to the lexical check below
+    throw new Error(`Refusing to read "${relativePath}" — it does not exist.`)
+  }
+
+  if (real !== root && !real.startsWith(root + path.sep)) {
+    throw new Error(`Refusing to read "${relativePath}" — it resolves outside REPO_PATH.`)
+  }
+  return resolved
+}
+
+// ── get_pr_diff: the one network call — fetch what changed in the PR ──────────
+export const getPrDiffTool = new DynamicStructuredTool({
+  name: 'get_pr_diff',
+  description:
+    'Fetches the changed files and their diffs (patches) for a pull request, plus the base/head branches. Call this FIRST to see what the PR changed.',
+  schema: z.object({
+    owner: z.string().describe('Repository owner (org or user)'),
+    repo: z.string().describe('Repository name'),
+    prNumber: z.number().describe('Pull request number'),
+  }),
+  func: async ({ owner, repo, prNumber }) => {
+    const { data: pr } = await octokit.rest.pulls.get({ owner, repo, pull_number: prNumber })
+    const files = await octokit.paginate(octokit.rest.pulls.listFiles, { owner, repo, pull_number: prNumber, per_page: 100 })
+
+    const result = {
+      title: pr.title,
+      baseBranch: pr.base.ref,
+      headBranch: pr.head.ref,
+      changedFiles: files.map((f) => ({
+        filename: f.filename,
+        status: f.status,
+        additions: f.additions,
+        deletions: f.deletions,
+        patch: f.patch ?? '(no textual diff — binary or too large)',
+      })),
+    }
+
+    console.log(`✅ get_pr_diff: ${result.changedFiles.length} changed file(s) in PR #${prNumber} (${result.headBranch} → ${result.baseBranch})`)
+
+    return JSON.stringify(result)
+  },
+})
+
+// ── read_file: read any file from the local clone to gather context ───────────
+export const readFileTool = new DynamicStructuredTool({
+  name: 'read_file',
+  description:
+    'Reads a file from the local checkout of the repository so you can inspect the surrounding code (callers, definitions, types) before deciding whether a change is a real issue. Path is relative to the repo root.',
+  schema: z.object({
+    path: z.string().describe('Repo-relative path of the file to read, e.g. "src/utils/parser.ts"'),
+  }),
+  func: async ({ path: relativePath }) => {
+    const absolutePath = resolveInsideRepo(relativePath)
+
+    let contents: string
+    try {
+      contents = await readFile(absolutePath, 'utf8')
+    } catch {
+      console.log(`⚠️ read_file: could not read "${relativePath}"`)
+      return JSON.stringify({ path: relativePath, error: 'File not found or unreadable in the local clone.' })
+    }
+
+    // Prefix line numbers so the agent can cite accurate file:line references.
+    const numbered = contents
+      .split('\n')
+      .map((line, i) => `${i + 1}\t${line}`)
+      .join('\n')
+
+    console.log(`✅ read_file: ${relativePath} (${contents.split('\n').length} lines)`)
+
+    return JSON.stringify({ path: relativePath, contents: numbered })
+  },
+})
+
+// ── search_code: grep the local clone to find callers / definitions ───────────
+export const searchCodeTool = new DynamicStructuredTool({
+  name: 'search_code',
+  description:
+    'Searches the local checkout for a string or symbol (e.g. a function name) to locate its definition and callers. Use this to understand how changed code is used elsewhere before judging it.',
+  schema: z.object({
+    query: z.string().describe('Literal text or symbol to search for'),
+    glob: z.string().optional().describe('Optional file glob to narrow the search, e.g. "*.ts"'),
+  }),
+  func: async ({ query, glob }) => {
+    const root = repoRoot()
+    // --no-follow is ripgrep's default; we pin it explicitly so the search can never be made to
+    // traverse a symlink out of the repo (defence-in-depth, matching the read_file guard).
+    const args = ['--line-number', '--no-heading', '--color', 'never', '--max-count', '50', '--fixed-strings', '--no-follow']
+    if (glob) args.push('--glob', glob)
+    args.push('--', query, root)
+
+    try {
+      const { stdout } = await execFileAsync('rg', args, { maxBuffer: 1024 * 1024 })
+      // Trim absolute prefix back to repo-relative paths for readability.
+      const matches = stdout
+        .split('\n')
+        .filter(Boolean)
+        .slice(0, 100)
+        .map((line) => line.replace(`${root}${path.sep}`, ''))
+
+      console.log(`✅ search_code: "${query}" → ${matches.length} match(es)`)
+      return JSON.stringify({ query, matches })
+    } catch (err) {
+      // ripgrep exits with code 1 when there are no matches — that's not an error for us.
+      const e = err as { code?: number }
+      if (e.code === 1) {
+        console.log(`✅ search_code: "${query}" → 0 matches`)
+        return JSON.stringify({ query, matches: [] })
+      }
+      console.log(`⚠️ search_code failed for "${query}"`)
+      return JSON.stringify({ query, error: 'Search failed.' })
+    }
+  },
+})

src/utils/pr-url.ts

@@ -0,0 +1,22 @@
+// Parses a GitHub pull request URL into its owner / repo / number parts.
+// Basic validation only — full guardrails (size limits, richer input handling) are deferred to a later task.
+
+export type ParsedPr = {
+  owner: string
+  repo: string
+  number: number
+}
+
+const PR_URL_REGEX = /github\.com\/([^/\s]+)\/([^/\s]+)\/pull\/(\d+)/i
+
+export function parsePrUrl(url: string): ParsedPr {
+  const match = url.trim().match(PR_URL_REGEX)
+
+  if (!match) {
+    throw new Error(`Invalid GitHub PR URL: "${url}". Expected a URL like https://github.com/<owner>/<repo>/pull/<number>`)
+  }
+
+  const [, owner, repo, number] = match
+
+  return { owner, repo, number: Number(number) }
+}