9b017475c9
TDD task breakdown for the walking skeleton: diff parsing, forge adapter port with a local git implementation, rules-based tiering, config loading, analyzer runner behind a ModelClient port, pi SDK client, aggregator, JSONL emitter, CLI. Later phases recorded as milestones with done-when criteria. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Ye1KNFMkkUtmzTypHXkoK
1734 lines
48 KiB
Markdown
1734 lines
48 KiB
Markdown
# pragent Phase 1 (Walking Skeleton) Implementation Plan
|
||
|
||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
|
||
|
||
**Goal:** `pragent review` runs against a local git range, classifies the change into a tier, runs one analyzer through a real model, prints ranked findings, writes a JSONL run record, and exits with a meaningful code.
|
||
|
||
**Architecture:** A CLI composed of small pure modules behind ports. `ForgeAdapter` supplies the diff and receives findings; `TierEngine` is pure functions over a parsed diff; `ModelClient` is a port with a fake for tests and a `pi`-SDK adapter for real runs; `Emitter` writes JSONL. Everything except the two adapters is tested without network access.
|
||
|
||
**Tech Stack:** TypeScript 5 (strict, ESM, `NodeNext`), Node 22+, Vitest, Zod for config/output validation, `@earendil-works/pi-coding-agent` + `@earendil-works/pi-ai` for the agent loop, `commander` for the CLI.
|
||
|
||
**Repo:** `~/Projects/pragent` (Gitea: `gitea_admin/pragent`). Design doc: `docs/plans/2026-08-04-pragent-design.md`.
|
||
|
||
---
|
||
|
||
## Ground rules for the implementer
|
||
|
||
- **TDD, strictly.** Write the failing test, watch it fail, write the minimum code, watch it pass, commit. A step that says "run it and see it fail" is not decoration — a test that passes before the implementation exists is a broken test.
|
||
- **No network in unit tests.** Only `tests/integration/` may call a model, and it is skipped unless `ANTHROPIC_API_KEY` is set.
|
||
- **ESM everywhere.** `"type": "module"` in `package.json`; relative imports carry the `.js` extension even in `.ts` sources (`NodeNext` resolution). This trips people up — it is not a typo.
|
||
- **Commit after every task.** Message style: Conventional Commits, imperative subject under 50 chars.
|
||
- **Do not build Phase 2+ features.** No Gitea adapter, no `pragent init`, no prompt caching, no fan-out. They are listed at the end as milestones so you know where this is heading, not as work to do now.
|
||
|
||
---
|
||
|
||
## Task 1: Project scaffold
|
||
|
||
**Files:**
|
||
- Create: `package.json`, `tsconfig.json`, `vitest.config.ts`, `src/index.ts`, `tests/smoke.test.ts`
|
||
|
||
**Step 1: Initialize the package**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent
|
||
npm init -y
|
||
npm pkg set type=module name=pragent version=0.1.0 \
|
||
description="Extensible, forge-agnostic PR review framework"
|
||
npm pkg set bin.pragent=dist/cli.js
|
||
npm pkg set scripts.build="tsc -p tsconfig.json"
|
||
npm pkg set scripts.test="vitest run"
|
||
npm pkg set scripts.typecheck="tsc --noEmit"
|
||
npm install -D typescript vitest @types/node
|
||
npm install commander zod
|
||
```
|
||
|
||
**Step 2: Write `tsconfig.json`**
|
||
|
||
```json
|
||
{
|
||
"compilerOptions": {
|
||
"target": "ES2022",
|
||
"module": "NodeNext",
|
||
"moduleResolution": "NodeNext",
|
||
"strict": true,
|
||
"noUncheckedIndexedAccess": true,
|
||
"declaration": true,
|
||
"outDir": "dist",
|
||
"rootDir": "src",
|
||
"skipLibCheck": true
|
||
},
|
||
"include": ["src/**/*.ts"]
|
||
}
|
||
```
|
||
|
||
**Step 3: Write `vitest.config.ts`**
|
||
|
||
```typescript
|
||
import { defineConfig } from "vitest/config";
|
||
|
||
export default defineConfig({
|
||
test: {
|
||
include: ["tests/**/*.test.ts"],
|
||
testTimeout: 20_000,
|
||
},
|
||
});
|
||
```
|
||
|
||
**Step 4: Write the smoke test**
|
||
|
||
`tests/smoke.test.ts`:
|
||
|
||
```typescript
|
||
import { expect, test } from "vitest";
|
||
import { VERSION } from "../src/index.js";
|
||
|
||
test("exports a version string", () => {
|
||
expect(VERSION).toMatch(/^\d+\.\d+\.\d+$/);
|
||
});
|
||
```
|
||
|
||
**Step 5: Run it and watch it fail**
|
||
|
||
Run: `npm test`
|
||
Expected: FAIL — `Failed to resolve import "../src/index.js"`.
|
||
|
||
**Step 6: Write the minimum implementation**
|
||
|
||
`src/index.ts`:
|
||
|
||
```typescript
|
||
export const VERSION = "0.1.0";
|
||
```
|
||
|
||
**Step 7: Run it and watch it pass**
|
||
|
||
Run: `npm test` → 1 passed. Then `npm run typecheck` → no errors.
|
||
|
||
**Step 8: Commit**
|
||
|
||
```bash
|
||
git add -A
|
||
git commit -m "chore: scaffold TypeScript project with vitest"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 2: Diff parsing
|
||
|
||
Everything downstream reads a parsed diff, so this is the foundation. Parse unified diff text into structured changed files.
|
||
|
||
**Files:**
|
||
- Create: `src/diff/types.ts`, `src/diff/parse.ts`, `tests/diff/parse.test.ts`
|
||
|
||
**Step 1: Write the failing test**
|
||
|
||
`tests/diff/parse.test.ts`:
|
||
|
||
```typescript
|
||
import { expect, test } from "vitest";
|
||
import { parseUnifiedDiff } from "../../src/diff/parse.js";
|
||
|
||
const SAMPLE = `diff --git a/src/auth/login.ts b/src/auth/login.ts
|
||
index 1111111..2222222 100644
|
||
--- a/src/auth/login.ts
|
||
+++ b/src/auth/login.ts
|
||
@@ -10,3 +10,4 @@ export function login(u: string) {
|
||
const t = mint(u);
|
||
- return t;
|
||
+ log(t);
|
||
+ return t;
|
||
}
|
||
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
|
||
index 3333333..4444444 100644
|
||
--- a/pnpm-lock.yaml
|
||
+++ b/pnpm-lock.yaml
|
||
@@ -1,2 +1,2 @@
|
||
-old
|
||
+new
|
||
`;
|
||
|
||
test("parses paths, status, and line counts", () => {
|
||
const files = parseUnifiedDiff(SAMPLE);
|
||
expect(files).toHaveLength(2);
|
||
expect(files[0]!.path).toBe("src/auth/login.ts");
|
||
expect(files[0]!.status).toBe("modified");
|
||
expect(files[0]!.added).toBe(2);
|
||
expect(files[0]!.removed).toBe(1);
|
||
expect(files[1]!.path).toBe("pnpm-lock.yaml");
|
||
});
|
||
|
||
test("totals are the sum across files", () => {
|
||
const files = parseUnifiedDiff(SAMPLE);
|
||
const total = files.reduce((n, f) => n + f.added + f.removed, 0);
|
||
expect(total).toBe(5);
|
||
});
|
||
|
||
test("returns an empty array for an empty diff", () => {
|
||
expect(parseUnifiedDiff("")).toEqual([]);
|
||
});
|
||
|
||
test("detects added and deleted files", () => {
|
||
const d = `diff --git a/new.ts b/new.ts
|
||
new file mode 100644
|
||
--- /dev/null
|
||
+++ b/new.ts
|
||
@@ -0,0 +1,1 @@
|
||
+hello
|
||
diff --git a/gone.ts b/gone.ts
|
||
deleted file mode 100644
|
||
--- a/gone.ts
|
||
+++ /dev/null
|
||
@@ -1,1 +0,0 @@
|
||
-bye
|
||
`;
|
||
const files = parseUnifiedDiff(d);
|
||
expect(files[0]!.status).toBe("added");
|
||
expect(files[1]!.status).toBe("deleted");
|
||
});
|
||
```
|
||
|
||
**Step 2: Run it and watch it fail**
|
||
|
||
Run: `npx vitest run tests/diff/parse.test.ts`
|
||
Expected: FAIL — cannot resolve `parse.js`.
|
||
|
||
**Step 3: Write the types**
|
||
|
||
`src/diff/types.ts`:
|
||
|
||
```typescript
|
||
export type FileStatus = "added" | "modified" | "deleted" | "renamed";
|
||
|
||
export interface ChangedFile {
|
||
path: string;
|
||
status: FileStatus;
|
||
added: number;
|
||
removed: number;
|
||
/** The raw hunk text for this file, passed verbatim to analyzers. */
|
||
patch: string;
|
||
}
|
||
|
||
export interface ParsedDiff {
|
||
files: ChangedFile[];
|
||
totalAdded: number;
|
||
totalRemoved: number;
|
||
}
|
||
```
|
||
|
||
**Step 4: Write the parser**
|
||
|
||
`src/diff/parse.ts`:
|
||
|
||
```typescript
|
||
import type { ChangedFile, FileStatus } from "./types.js";
|
||
|
||
const FILE_HEADER = /^diff --git a\/(.+?) b\/(.+)$/;
|
||
|
||
export function parseUnifiedDiff(text: string): ChangedFile[] {
|
||
if (!text.trim()) return [];
|
||
|
||
const files: ChangedFile[] = [];
|
||
let current: ChangedFile | null = null;
|
||
|
||
for (const line of text.split("\n")) {
|
||
const header = FILE_HEADER.exec(line);
|
||
if (header) {
|
||
if (current) files.push(current);
|
||
current = {
|
||
path: header[2]!,
|
||
status: "modified",
|
||
added: 0,
|
||
removed: 0,
|
||
patch: "",
|
||
};
|
||
continue;
|
||
}
|
||
if (!current) continue;
|
||
|
||
current.patch += line + "\n";
|
||
|
||
if (line.startsWith("new file mode")) current.status = "added";
|
||
else if (line.startsWith("deleted file mode")) current.status = "deleted";
|
||
else if (line.startsWith("rename to ")) current.status = "renamed";
|
||
else if (line.startsWith("+++") || line.startsWith("---")) continue;
|
||
else if (line.startsWith("+")) current.added++;
|
||
else if (line.startsWith("-")) current.removed++;
|
||
}
|
||
|
||
if (current) files.push(current);
|
||
return files;
|
||
}
|
||
```
|
||
|
||
**Step 5: Run it and watch it pass**
|
||
|
||
Run: `npx vitest run tests/diff/parse.test.ts` → 4 passed.
|
||
|
||
**Step 6: Commit**
|
||
|
||
```bash
|
||
git add src/diff tests/diff
|
||
git commit -m "feat: parse unified diffs into changed files"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 3: Forge adapter port and local git adapter
|
||
|
||
**Files:**
|
||
- Create: `src/forge/types.ts`, `src/forge/local.ts`, `tests/forge/local.test.ts`
|
||
|
||
The adapter is the seam that makes Gitea and GitLab cheap later. Phase 1 ships only the local one.
|
||
|
||
**Step 1: Write the failing test**
|
||
|
||
`tests/forge/local.test.ts`:
|
||
|
||
```typescript
|
||
import { execFileSync } from "node:child_process";
|
||
import { mkdtempSync, writeFileSync } from "node:fs";
|
||
import { tmpdir } from "node:os";
|
||
import { join } from "node:path";
|
||
import { expect, test } from "vitest";
|
||
import { LocalForgeAdapter } from "../../src/forge/local.js";
|
||
|
||
function fixtureRepo(): string {
|
||
const dir = mkdtempSync(join(tmpdir(), "pragent-"));
|
||
const git = (...args: string[]) =>
|
||
execFileSync("git", args, { cwd: dir, stdio: "pipe" });
|
||
|
||
git("init", "-q", "-b", "main");
|
||
git("config", "user.email", "t@example.com");
|
||
git("config", "user.name", "Test");
|
||
writeFileSync(join(dir, "app.ts"), "export const a = 1;\n");
|
||
git("add", "-A");
|
||
git("commit", "-q", "-m", "base");
|
||
writeFileSync(join(dir, "app.ts"), "export const a = 1;\nexport const b = 2;\n");
|
||
git("add", "-A");
|
||
git("commit", "-q", "-m", "change");
|
||
return dir;
|
||
}
|
||
|
||
test("reads a diff for a git range", async () => {
|
||
const dir = fixtureRepo();
|
||
const adapter = new LocalForgeAdapter({ cwd: dir, range: "HEAD~1..HEAD" });
|
||
const ctx = await adapter.getContext();
|
||
|
||
expect(ctx.files).toHaveLength(1);
|
||
expect(ctx.files[0]!.path).toBe("app.ts");
|
||
expect(ctx.files[0]!.added).toBe(1);
|
||
expect(ctx.headSha).toMatch(/^[0-9a-f]{40}$/);
|
||
});
|
||
|
||
test("rejects an invalid range with a clear message", async () => {
|
||
const dir = fixtureRepo();
|
||
const adapter = new LocalForgeAdapter({ cwd: dir, range: "nope..alsonope" });
|
||
await expect(adapter.getContext()).rejects.toThrow(/git diff failed/i);
|
||
});
|
||
```
|
||
|
||
**Step 2: Run it and watch it fail**
|
||
|
||
Run: `npx vitest run tests/forge/local.test.ts` → FAIL, module not found.
|
||
|
||
**Step 3: Write the port**
|
||
|
||
`src/forge/types.ts`:
|
||
|
||
```typescript
|
||
import type { ChangedFile } from "../diff/types.js";
|
||
import type { Finding } from "../analyze/types.js";
|
||
|
||
export interface ReviewContext {
|
||
repo: string;
|
||
/** PR/MR number where the forge has one; null for local runs. */
|
||
pullRequest: number | null;
|
||
headSha: string;
|
||
baseSha: string;
|
||
title: string;
|
||
files: ChangedFile[];
|
||
}
|
||
|
||
export interface ForgeAdapter {
|
||
readonly name: string;
|
||
getContext(): Promise<ReviewContext>;
|
||
publish(ctx: ReviewContext, findings: Finding[]): Promise<void>;
|
||
}
|
||
```
|
||
|
||
**Step 4: Write the local adapter**
|
||
|
||
`src/forge/local.ts`:
|
||
|
||
```typescript
|
||
import { execFile } from "node:child_process";
|
||
import { basename } from "node:path";
|
||
import { promisify } from "node:util";
|
||
import { parseUnifiedDiff } from "../diff/parse.js";
|
||
import type { Finding } from "../analyze/types.js";
|
||
import type { ForgeAdapter, ReviewContext } from "./types.js";
|
||
|
||
const run = promisify(execFile);
|
||
|
||
export interface LocalForgeOptions {
|
||
cwd: string;
|
||
/** A git range such as "main..HEAD" or "HEAD~1..HEAD". */
|
||
range: string;
|
||
}
|
||
|
||
export class LocalForgeAdapter implements ForgeAdapter {
|
||
readonly name = "local";
|
||
|
||
constructor(private readonly opts: LocalForgeOptions) {}
|
||
|
||
async getContext(): Promise<ReviewContext> {
|
||
const { cwd, range } = this.opts;
|
||
let diff: string;
|
||
try {
|
||
const res = await run("git", ["diff", "--no-color", range], {
|
||
cwd,
|
||
maxBuffer: 64 * 1024 * 1024,
|
||
});
|
||
diff = res.stdout;
|
||
} catch (err) {
|
||
throw new Error(
|
||
`git diff failed for range "${range}" in ${cwd}: ${(err as Error).message}`,
|
||
);
|
||
}
|
||
|
||
const rev = async (ref: string) =>
|
||
(await run("git", ["rev-parse", ref], { cwd })).stdout.trim();
|
||
|
||
const [base, head] = range.split("..");
|
||
return {
|
||
repo: basename(cwd),
|
||
pullRequest: null,
|
||
baseSha: await rev(base || "HEAD~1"),
|
||
headSha: await rev(head || "HEAD"),
|
||
title: `local diff ${range}`,
|
||
files: parseUnifiedDiff(diff),
|
||
};
|
||
}
|
||
|
||
async publish(_ctx: ReviewContext, findings: Finding[]): Promise<void> {
|
||
for (const f of findings) {
|
||
console.log(
|
||
`${f.file}:${f.line} [${f.severity}/${f.confidence}] ${f.claim}` +
|
||
(f.fix ? `\n fix: ${f.fix}` : ""),
|
||
);
|
||
}
|
||
}
|
||
}
|
||
```
|
||
|
||
**Step 5: Run it and watch it pass**
|
||
|
||
Run: `npx vitest run tests/forge/local.test.ts` → 2 passed.
|
||
(`src/analyze/types.ts` does not exist yet — create it as a stub now with `export interface Finding { file: string; line: number; severity: string; confidence: number; claim: string; fix?: string }`. Task 6 replaces it with the real schema.)
|
||
|
||
**Step 6: Commit**
|
||
|
||
```bash
|
||
git add src/forge src/analyze tests/forge
|
||
git commit -m "feat: add forge adapter port and local git adapter"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 4: Tier engine (rules only)
|
||
|
||
The heart of the cost model. Pure functions, no I/O, no model calls — and it must record **why** it chose a tier.
|
||
|
||
**Files:**
|
||
- Create: `src/tier/rules.ts`, `src/tier/types.ts`, `tests/tier/rules.test.ts`
|
||
|
||
**Step 1: Write the failing test**
|
||
|
||
`tests/tier/rules.test.ts`:
|
||
|
||
```typescript
|
||
import { expect, test } from "vitest";
|
||
import { classify, DEFAULT_TIER_POLICY } from "../../src/tier/rules.js";
|
||
import type { ChangedFile } from "../../src/diff/types.js";
|
||
|
||
const file = (path: string, added = 10, removed = 0): ChangedFile => ({
|
||
path,
|
||
status: "modified",
|
||
added,
|
||
removed,
|
||
patch: "",
|
||
});
|
||
|
||
const P = DEFAULT_TIER_POLICY;
|
||
|
||
test("lockfile-only changes are trivial", () => {
|
||
const r = classify([file("pnpm-lock.yaml", 400, 300)], P);
|
||
expect(r.tier).toBe("trivial");
|
||
expect(r.reason).toMatch(/generated/);
|
||
});
|
||
|
||
test("a small ordinary change is lite", () => {
|
||
const r = classify([file("src/util/format.ts", 20, 5)], P);
|
||
expect(r.tier).toBe("lite");
|
||
});
|
||
|
||
test("a risk path forces full even when tiny", () => {
|
||
const r = classify([file("src/auth/session.ts", 3, 1)], P);
|
||
expect(r.tier).toBe("full");
|
||
expect(r.reason).toMatch(/risk_path/);
|
||
});
|
||
|
||
test("a large change is full", () => {
|
||
const r = classify([file("src/big.ts", 900, 100)], P);
|
||
expect(r.tier).toBe("full");
|
||
expect(r.reason).toMatch(/size/);
|
||
});
|
||
|
||
test("a very large change is oversized", () => {
|
||
const files = Array.from({ length: 60 }, (_, i) => file(`src/f${i}.ts`, 60, 10));
|
||
const r = classify(files, P);
|
||
expect(r.tier).toBe("oversized");
|
||
});
|
||
|
||
test("risk paths win over the generated-file rule", () => {
|
||
const r = classify(
|
||
[file("pnpm-lock.yaml", 10), file("infra/main.tf", 4)],
|
||
P,
|
||
);
|
||
expect(r.tier).toBe("full");
|
||
expect(r.reason).toMatch(/risk_path/);
|
||
});
|
||
|
||
test("an empty diff is trivial", () => {
|
||
expect(classify([], P).tier).toBe("trivial");
|
||
});
|
||
```
|
||
|
||
**Step 2: Run it and watch it fail**
|
||
|
||
Run: `npx vitest run tests/tier/rules.test.ts` → FAIL.
|
||
|
||
**Step 3: Write the types**
|
||
|
||
`src/tier/types.ts`:
|
||
|
||
```typescript
|
||
export type Tier = "trivial" | "lite" | "full" | "oversized";
|
||
|
||
export interface TierDecision {
|
||
tier: Tier;
|
||
/** Machine-readable provenance, e.g. "rule:risk_path(**\/auth\/**)". */
|
||
reason: string;
|
||
}
|
||
|
||
export interface TierPolicy {
|
||
riskPaths: string[];
|
||
generatedPaths: string[];
|
||
liteMaxLines: number;
|
||
fullMaxLines: number;
|
||
fullMaxFiles: number;
|
||
}
|
||
```
|
||
|
||
**Step 4: Write the rules**
|
||
|
||
`src/tier/rules.ts`:
|
||
|
||
```typescript
|
||
import type { ChangedFile } from "../diff/types.js";
|
||
import type { TierDecision, TierPolicy } from "./types.js";
|
||
|
||
export const DEFAULT_TIER_POLICY: TierPolicy = {
|
||
riskPaths: [
|
||
"**/auth/**",
|
||
"**/migrations/**",
|
||
"infra/**",
|
||
"**/*.tf",
|
||
"**/security/**",
|
||
],
|
||
generatedPaths: [
|
||
"**/*-lock.yaml",
|
||
"**/*.lock",
|
||
"package-lock.json",
|
||
"**/*.pb.go",
|
||
"**/generated/**",
|
||
],
|
||
liteMaxLines: 150,
|
||
fullMaxLines: 2000,
|
||
fullMaxFiles: 50,
|
||
};
|
||
|
||
/** Minimal glob matcher: supports `**` and `*`. No brace or char classes. */
|
||
export function matchGlob(pattern: string, path: string): boolean {
|
||
const rx = pattern
|
||
.split("**")
|
||
.map((seg) => seg.split("*").map(escapeRegex).join("[^/]*"))
|
||
.join(".*");
|
||
return new RegExp(`^${rx}$`).test(path);
|
||
}
|
||
|
||
function escapeRegex(s: string): string {
|
||
return s.replace(/[.+?^${}()|[\]\\]/g, "\\$&");
|
||
}
|
||
|
||
export function classify(
|
||
files: ChangedFile[],
|
||
policy: TierPolicy,
|
||
): TierDecision {
|
||
if (files.length === 0) {
|
||
return { tier: "trivial", reason: "rule:empty_diff" };
|
||
}
|
||
|
||
// Risk paths outrank every other rule — a three-line auth change is never trivial.
|
||
for (const f of files) {
|
||
for (const p of policy.riskPaths) {
|
||
if (matchGlob(p, f.path)) {
|
||
return { tier: "full", reason: `rule:risk_path(${p})` };
|
||
}
|
||
}
|
||
}
|
||
|
||
const lines = files.reduce((n, f) => n + f.added + f.removed, 0);
|
||
|
||
if (lines > policy.fullMaxLines || files.length > policy.fullMaxFiles) {
|
||
return {
|
||
tier: "oversized",
|
||
reason: `rule:size(lines=${lines},files=${files.length})`,
|
||
};
|
||
}
|
||
|
||
const allGenerated = files.every((f) =>
|
||
policy.generatedPaths.some((p) => matchGlob(p, f.path)),
|
||
);
|
||
if (allGenerated) {
|
||
return { tier: "trivial", reason: "rule:generated_only" };
|
||
}
|
||
|
||
if (lines <= policy.liteMaxLines) {
|
||
return { tier: "lite", reason: `rule:size(lines=${lines})` };
|
||
}
|
||
|
||
return { tier: "full", reason: `rule:size(lines=${lines})` };
|
||
}
|
||
```
|
||
|
||
**Step 5: Run it and watch it pass**
|
||
|
||
Run: `npx vitest run tests/tier/rules.test.ts` → 7 passed.
|
||
|
||
**Step 6: Commit**
|
||
|
||
```bash
|
||
git add src/tier tests/tier
|
||
git commit -m "feat: add rules-based tier classification"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 5: Config loading
|
||
|
||
**Files:**
|
||
- Create: `src/config/schema.ts`, `src/config/load.ts`, `tests/config/load.test.ts`
|
||
|
||
**Step 1: Write the failing test**
|
||
|
||
`tests/config/load.test.ts`:
|
||
|
||
```typescript
|
||
import { mkdtempSync, mkdirSync, writeFileSync } from "node:fs";
|
||
import { tmpdir } from "node:os";
|
||
import { join } from "node:path";
|
||
import { expect, test } from "vitest";
|
||
import { loadConfig } from "../../src/config/load.js";
|
||
|
||
function repoWith(config: string | null): string {
|
||
const dir = mkdtempSync(join(tmpdir(), "pragent-cfg-"));
|
||
if (config !== null) {
|
||
mkdirSync(join(dir, ".pragent"));
|
||
writeFileSync(join(dir, ".pragent", "config.json"), config);
|
||
}
|
||
return dir;
|
||
}
|
||
|
||
test("returns defaults when no config file exists", async () => {
|
||
const cfg = await loadConfig(repoWith(null));
|
||
expect(cfg.tierPolicy.liteMaxLines).toBe(150);
|
||
expect(cfg.maxCostUsd).toBeGreaterThan(0);
|
||
});
|
||
|
||
test("repo config overrides defaults", async () => {
|
||
const dir = repoWith(JSON.stringify({ tierPolicy: { liteMaxLines: 40 } }));
|
||
const cfg = await loadConfig(dir);
|
||
expect(cfg.tierPolicy.liteMaxLines).toBe(40);
|
||
expect(cfg.tierPolicy.fullMaxLines).toBe(2000); // untouched default survives
|
||
});
|
||
|
||
test("rejects an invalid config with a useful message", async () => {
|
||
const dir = repoWith(JSON.stringify({ tierPolicy: { liteMaxLines: "big" } }));
|
||
await expect(loadConfig(dir)).rejects.toThrow(/liteMaxLines/);
|
||
});
|
||
```
|
||
|
||
**Step 2: Run it and watch it fail**
|
||
|
||
Run: `npx vitest run tests/config/load.test.ts` → FAIL.
|
||
|
||
**Step 3: Write the schema**
|
||
|
||
`src/config/schema.ts`:
|
||
|
||
```typescript
|
||
import { z } from "zod";
|
||
import { DEFAULT_TIER_POLICY } from "../tier/rules.js";
|
||
|
||
export const TierPolicySchema = z.object({
|
||
riskPaths: z.array(z.string()).default(DEFAULT_TIER_POLICY.riskPaths),
|
||
generatedPaths: z.array(z.string()).default(DEFAULT_TIER_POLICY.generatedPaths),
|
||
liteMaxLines: z.number().int().positive().default(DEFAULT_TIER_POLICY.liteMaxLines),
|
||
fullMaxLines: z.number().int().positive().default(DEFAULT_TIER_POLICY.fullMaxLines),
|
||
fullMaxFiles: z.number().int().positive().default(DEFAULT_TIER_POLICY.fullMaxFiles),
|
||
});
|
||
|
||
export const ConfigSchema = z.object({
|
||
tierPolicy: TierPolicySchema.default({}),
|
||
/** Fail open above this spend; a partial review beats a blocked pipeline. */
|
||
maxCostUsd: z.number().positive().default(5),
|
||
minConfidence: z.number().min(0).max(1).default(0.5),
|
||
maxComments: z.number().int().positive().default(20),
|
||
models: z
|
||
.object({
|
||
lite: z.string().default("claude-sonnet-5"),
|
||
full: z.string().default("claude-opus-5"),
|
||
})
|
||
.default({}),
|
||
emitPath: z.string().default(".pragent/runs.jsonl"),
|
||
});
|
||
|
||
export type Config = z.infer<typeof ConfigSchema>;
|
||
```
|
||
|
||
**Step 4: Write the loader**
|
||
|
||
`src/config/load.ts`:
|
||
|
||
```typescript
|
||
import { readFile } from "node:fs/promises";
|
||
import { join } from "node:path";
|
||
import { ConfigSchema, type Config } from "./schema.js";
|
||
|
||
export async function loadConfig(repoRoot: string): Promise<Config> {
|
||
let raw: unknown = {};
|
||
try {
|
||
const text = await readFile(join(repoRoot, ".pragent", "config.json"), "utf8");
|
||
raw = JSON.parse(text);
|
||
} catch (err) {
|
||
if ((err as NodeJS.ErrnoException).code !== "ENOENT") throw err;
|
||
}
|
||
|
||
const parsed = ConfigSchema.safeParse(raw);
|
||
if (!parsed.success) {
|
||
const issues = parsed.error.issues
|
||
.map((i) => `${i.path.join(".")}: ${i.message}`)
|
||
.join("; ");
|
||
throw new Error(`invalid .pragent/config.json — ${issues}`);
|
||
}
|
||
return parsed.data;
|
||
}
|
||
```
|
||
|
||
**Step 5: Run it and watch it pass**
|
||
|
||
Run: `npx vitest run tests/config/load.test.ts` → 3 passed.
|
||
|
||
**Step 6: Commit**
|
||
|
||
```bash
|
||
git add src/config tests/config
|
||
git commit -m "feat: load and validate repo config with defaults"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 6: Analyzer runner against a fake model
|
||
|
||
The model is a **port**. This task builds the analyzer logic and tests it with a fake — no network, deterministic, fast. Task 7 plugs in the real SDK.
|
||
|
||
**Files:**
|
||
- Create: `src/analyze/types.ts` (replacing the Task 3 stub), `src/analyze/model.ts`, `src/analyze/runner.ts`, `src/analyze/prompts/code-quality.ts`, `tests/analyze/runner.test.ts`
|
||
|
||
**Step 1: Write the failing test**
|
||
|
||
`tests/analyze/runner.test.ts`:
|
||
|
||
```typescript
|
||
import { expect, test } from "vitest";
|
||
import { runAnalyzer } from "../../src/analyze/runner.js";
|
||
import type { ModelClient } from "../../src/analyze/model.js";
|
||
import type { ReviewContext } from "../../src/forge/types.js";
|
||
|
||
const ctx: ReviewContext = {
|
||
repo: "demo",
|
||
pullRequest: null,
|
||
headSha: "a".repeat(40),
|
||
baseSha: "b".repeat(40),
|
||
title: "test",
|
||
files: [
|
||
{ path: "src/a.ts", status: "modified", added: 3, removed: 1, patch: "+ bad()" },
|
||
],
|
||
};
|
||
|
||
const fakeModel = (reply: string): ModelClient => ({
|
||
name: "fake",
|
||
async complete() {
|
||
return { text: reply, usage: { inputTokens: 100, outputTokens: 20, costUsd: 0.01 } };
|
||
},
|
||
});
|
||
|
||
test("parses well-formed findings from the model", async () => {
|
||
const model = fakeModel(
|
||
JSON.stringify({
|
||
findings: [
|
||
{
|
||
file: "src/a.ts",
|
||
line: 12,
|
||
severity: "high",
|
||
confidence: 0.9,
|
||
category: "correctness",
|
||
claim: "Null deref when input is empty",
|
||
fix: "Guard the empty case",
|
||
},
|
||
],
|
||
}),
|
||
);
|
||
|
||
const res = await runAnalyzer({ id: "code-quality", model, ctx });
|
||
expect(res.findings).toHaveLength(1);
|
||
expect(res.findings[0]!.analyzer).toBe("code-quality");
|
||
expect(res.usage.costUsd).toBeCloseTo(0.01);
|
||
});
|
||
|
||
test("tolerates prose wrapped around the JSON", async () => {
|
||
const model = fakeModel(
|
||
'Here is what I found:\n```json\n{"findings":[]}\n```\nThat is all.',
|
||
);
|
||
const res = await runAnalyzer({ id: "code-quality", model, ctx });
|
||
expect(res.findings).toEqual([]);
|
||
});
|
||
|
||
test("fails open on unparseable output instead of throwing", async () => {
|
||
const model = fakeModel("I could not complete the review.");
|
||
const res = await runAnalyzer({ id: "code-quality", model, ctx });
|
||
expect(res.findings).toEqual([]);
|
||
expect(res.error).toMatch(/parse/i);
|
||
});
|
||
|
||
test("drops findings that reference a file outside the diff", async () => {
|
||
const model = fakeModel(
|
||
JSON.stringify({
|
||
findings: [
|
||
{ file: "src/elsewhere.ts", line: 1, severity: "high", confidence: 0.9,
|
||
category: "correctness", claim: "hallucinated" },
|
||
{ file: "src/a.ts", line: 2, severity: "low", confidence: 0.8,
|
||
category: "style", claim: "real" },
|
||
],
|
||
}),
|
||
);
|
||
const res = await runAnalyzer({ id: "code-quality", model, ctx });
|
||
expect(res.findings).toHaveLength(1);
|
||
expect(res.findings[0]!.claim).toBe("real");
|
||
});
|
||
```
|
||
|
||
That last test matters more than it looks: a reviewer that invents files loses user trust faster than one that misses bugs.
|
||
|
||
**Step 2: Run it and watch it fail**
|
||
|
||
Run: `npx vitest run tests/analyze/runner.test.ts` → FAIL.
|
||
|
||
**Step 3: Write the types**
|
||
|
||
`src/analyze/types.ts` (replaces the stub from Task 3):
|
||
|
||
```typescript
|
||
import { z } from "zod";
|
||
|
||
export const SeveritySchema = z.enum(["critical", "high", "medium", "low"]);
|
||
|
||
export const FindingSchema = z.object({
|
||
file: z.string(),
|
||
line: z.number().int().nonnegative(),
|
||
severity: SeveritySchema,
|
||
confidence: z.number().min(0).max(1),
|
||
category: z.string(),
|
||
claim: z.string().min(1),
|
||
fix: z.string().optional(),
|
||
});
|
||
|
||
export type Severity = z.infer<typeof SeveritySchema>;
|
||
export type RawFinding = z.infer<typeof FindingSchema>;
|
||
|
||
/** A finding after the runner stamps its provenance. */
|
||
export interface Finding extends RawFinding {
|
||
analyzer: string;
|
||
}
|
||
|
||
export const AnalyzerOutputSchema = z.object({
|
||
findings: z.array(FindingSchema),
|
||
});
|
||
```
|
||
|
||
**Step 4: Write the model port**
|
||
|
||
`src/analyze/model.ts`:
|
||
|
||
```typescript
|
||
export interface Usage {
|
||
inputTokens: number;
|
||
outputTokens: number;
|
||
costUsd: number;
|
||
}
|
||
|
||
export interface CompletionResult {
|
||
text: string;
|
||
usage: Usage;
|
||
}
|
||
|
||
/** The seam between analyzers and any agent SDK. Keep it this small. */
|
||
export interface ModelClient {
|
||
readonly name: string;
|
||
complete(prompt: string): Promise<CompletionResult>;
|
||
}
|
||
```
|
||
|
||
**Step 5: Write the prompt**
|
||
|
||
`src/analyze/prompts/code-quality.ts`:
|
||
|
||
```typescript
|
||
import type { ReviewContext } from "../../forge/types.js";
|
||
|
||
export function buildCodeQualityPrompt(ctx: ReviewContext): string {
|
||
const diff = ctx.files.map((f) => `--- ${f.path} ---\n${f.patch}`).join("\n");
|
||
|
||
return `You are reviewing a code change for correctness and maintainability.
|
||
|
||
Report only defects you can point to in the diff below: logic errors, unhandled
|
||
cases, resource leaks, race conditions, and changes that contradict the
|
||
surrounding code's conventions. Do not report formatting preferences, and do not
|
||
report a problem you cannot tie to a specific line.
|
||
|
||
Only reference files that appear in the diff. If you find nothing, return an
|
||
empty list — that is a valid and useful answer.
|
||
|
||
Respond with JSON only, in this shape:
|
||
{"findings":[{"file":"path","line":12,"severity":"high","confidence":0.8,
|
||
"category":"correctness","claim":"one sentence","fix":"one sentence"}]}
|
||
|
||
Repository: ${ctx.repo}
|
||
Change: ${ctx.title}
|
||
|
||
${diff}`;
|
||
}
|
||
```
|
||
|
||
**Step 6: Write the runner**
|
||
|
||
`src/analyze/runner.ts`:
|
||
|
||
```typescript
|
||
import type { ReviewContext } from "../forge/types.js";
|
||
import type { ModelClient, Usage } from "./model.js";
|
||
import { buildCodeQualityPrompt } from "./prompts/code-quality.js";
|
||
import { AnalyzerOutputSchema, type Finding } from "./types.js";
|
||
|
||
export interface AnalyzerRun {
|
||
id: string;
|
||
model: ModelClient;
|
||
ctx: ReviewContext;
|
||
}
|
||
|
||
export interface AnalyzerResult {
|
||
analyzer: string;
|
||
findings: Finding[];
|
||
usage: Usage;
|
||
error?: string;
|
||
}
|
||
|
||
/** Pull the first balanced JSON object out of a model reply. */
|
||
export function extractJson(text: string): string | null {
|
||
const start = text.indexOf("{");
|
||
if (start === -1) return null;
|
||
let depth = 0;
|
||
for (let i = start; i < text.length; i++) {
|
||
if (text[i] === "{") depth++;
|
||
else if (text[i] === "}" && --depth === 0) return text.slice(start, i + 1);
|
||
}
|
||
return null;
|
||
}
|
||
|
||
export async function runAnalyzer(run: AnalyzerRun): Promise<AnalyzerResult> {
|
||
const prompt = buildCodeQualityPrompt(run.ctx);
|
||
const { text, usage } = await run.model.complete(prompt);
|
||
|
||
const json = extractJson(text);
|
||
if (!json) {
|
||
return {
|
||
analyzer: run.id,
|
||
findings: [],
|
||
usage,
|
||
error: "could not parse JSON from model output",
|
||
};
|
||
}
|
||
|
||
let parsed;
|
||
try {
|
||
parsed = AnalyzerOutputSchema.safeParse(JSON.parse(json));
|
||
} catch {
|
||
return { analyzer: run.id, findings: [], usage, error: "invalid JSON syntax" };
|
||
}
|
||
if (!parsed.success) {
|
||
return { analyzer: run.id, findings: [], usage, error: "schema parse failed" };
|
||
}
|
||
|
||
const known = new Set(run.ctx.files.map((f) => f.path));
|
||
const findings = parsed.data.findings
|
||
.filter((f) => known.has(f.file))
|
||
.map((f) => ({ ...f, analyzer: run.id }));
|
||
|
||
return { analyzer: run.id, findings, usage };
|
||
}
|
||
```
|
||
|
||
**Step 7: Run it and watch it pass**
|
||
|
||
Run: `npx vitest run tests/analyze/runner.test.ts` → 4 passed. Then `npm test` — the Task 3 forge test must still pass against the real `Finding` type.
|
||
|
||
**Step 8: Commit**
|
||
|
||
```bash
|
||
git add src/analyze tests/analyze
|
||
git commit -m "feat: add analyzer runner with schema-validated findings"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 7: `pi`-backed model client
|
||
|
||
The only module that touches the SDK. Its unit test asserts wiring, not model behaviour; the real call lives in a gated integration test.
|
||
|
||
**Files:**
|
||
- Create: `src/analyze/pi-model.ts`, `tests/integration/pi-model.test.ts`
|
||
|
||
**Step 1: Install the SDK**
|
||
|
||
```bash
|
||
npm install @earendil-works/pi-coding-agent @earendil-works/pi-ai
|
||
```
|
||
|
||
**Step 2: Write the gated integration test**
|
||
|
||
`tests/integration/pi-model.test.ts`:
|
||
|
||
```typescript
|
||
import { describe, expect, test } from "vitest";
|
||
import { PiModelClient } from "../../src/analyze/pi-model.js";
|
||
|
||
const live = process.env.ANTHROPIC_API_KEY ? describe : describe.skip;
|
||
|
||
live("PiModelClient (live)", () => {
|
||
test("returns text and non-zero usage", async () => {
|
||
const client = new PiModelClient({ model: "claude-sonnet-5" });
|
||
const res = await client.complete(
|
||
'Reply with exactly this JSON and nothing else: {"findings":[]}',
|
||
);
|
||
expect(res.text).toContain("findings");
|
||
expect(res.usage.inputTokens).toBeGreaterThan(0);
|
||
}, 60_000);
|
||
});
|
||
```
|
||
|
||
**Step 3: Run it and watch it skip**
|
||
|
||
Run: `npx vitest run tests/integration/pi-model.test.ts`
|
||
Expected: skipped when `ANTHROPIC_API_KEY` is unset. That is the correct outcome for CI.
|
||
|
||
**Step 4: Write the client**
|
||
|
||
`src/analyze/pi-model.ts`:
|
||
|
||
```typescript
|
||
import { createAgentSession, ModelRuntime } from "@earendil-works/pi-coding-agent";
|
||
import { getModel } from "@earendil-works/pi-ai";
|
||
import type { CompletionResult, ModelClient } from "./model.js";
|
||
|
||
export interface PiModelOptions {
|
||
model: string;
|
||
thinkingLevel?: "off" | "low" | "medium" | "high";
|
||
cwd?: string;
|
||
}
|
||
|
||
export class PiModelClient implements ModelClient {
|
||
readonly name: string;
|
||
|
||
constructor(private readonly opts: PiModelOptions) {
|
||
this.name = opts.model;
|
||
}
|
||
|
||
async complete(prompt: string): Promise<CompletionResult> {
|
||
const modelRuntime = await ModelRuntime.create();
|
||
const model = getModel("anthropic", this.opts.model);
|
||
|
||
const { session } = await createAgentSession({
|
||
model,
|
||
thinkingLevel: this.opts.thinkingLevel ?? "medium",
|
||
// Phase 1 sends the whole diff in the prompt: no filesystem tools, so the
|
||
// analyzer cannot wander and the cost is bounded. Tool budgets arrive in Phase 3.
|
||
tools: [],
|
||
cwd: this.opts.cwd ?? process.cwd(),
|
||
modelRuntime,
|
||
});
|
||
|
||
let text = "";
|
||
session.subscribe((event: any) => {
|
||
if (
|
||
event.type === "message_update" &&
|
||
event.assistantMessageEvent?.type === "text_delta"
|
||
) {
|
||
text += event.assistantMessageEvent.delta;
|
||
}
|
||
});
|
||
|
||
await session.prompt(prompt);
|
||
|
||
// Usage accounting is refined in Phase 3 alongside prompt caching; for now
|
||
// record what the session reports and fall back to zeros.
|
||
const usage = (session as any).usage ?? {};
|
||
return {
|
||
text,
|
||
usage: {
|
||
inputTokens: usage.inputTokens ?? 0,
|
||
outputTokens: usage.outputTokens ?? 0,
|
||
costUsd: usage.costUsd ?? 0,
|
||
},
|
||
};
|
||
}
|
||
}
|
||
```
|
||
|
||
**Step 5: Verify against the live API once, by hand**
|
||
|
||
```bash
|
||
ANTHROPIC_API_KEY=... npx vitest run tests/integration/pi-model.test.ts
|
||
```
|
||
|
||
Expected: PASS. **If the SDK surface differs from the snippet above** (event shape, usage location, `getModel` arguments), fix this file against the real types — it is the one place the plan is guessing, and `docs/plans/` is not a contract with the SDK. Record whatever you learn in a short comment at the top of the file.
|
||
|
||
**Step 6: Commit**
|
||
|
||
```bash
|
||
git add src/analyze/pi-model.ts tests/integration package.json package-lock.json
|
||
git commit -m "feat: add pi SDK model client"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 8: Aggregator
|
||
|
||
**Files:**
|
||
- Create: `src/aggregate/aggregate.ts`, `tests/aggregate/aggregate.test.ts`
|
||
|
||
**Step 1: Write the failing test**
|
||
|
||
`tests/aggregate/aggregate.test.ts`:
|
||
|
||
```typescript
|
||
import { expect, test } from "vitest";
|
||
import { aggregate } from "../../src/aggregate/aggregate.js";
|
||
import type { Finding } from "../../src/analyze/types.js";
|
||
|
||
const f = (over: Partial<Finding>): Finding => ({
|
||
file: "a.ts",
|
||
line: 10,
|
||
severity: "medium",
|
||
confidence: 0.8,
|
||
category: "correctness",
|
||
claim: "something",
|
||
analyzer: "code-quality",
|
||
...over,
|
||
});
|
||
|
||
test("drops findings below the confidence gate", () => {
|
||
const r = aggregate([f({ confidence: 0.2 }), f({ confidence: 0.9 })],
|
||
{ minConfidence: 0.5, maxComments: 10 });
|
||
expect(r.posted).toHaveLength(1);
|
||
expect(r.suppressed).toHaveLength(1);
|
||
});
|
||
|
||
test("dedupes near-identical findings within a line window", () => {
|
||
const r = aggregate(
|
||
[f({ line: 10, analyzer: "code-quality" }), f({ line: 12, analyzer: "security" })],
|
||
{ minConfidence: 0.5, maxComments: 10 },
|
||
);
|
||
expect(r.posted).toHaveLength(1);
|
||
});
|
||
|
||
test("ranks by severity then confidence", () => {
|
||
const r = aggregate(
|
||
[
|
||
f({ file: "a.ts", severity: "low", confidence: 0.99 }),
|
||
f({ file: "b.ts", severity: "critical", confidence: 0.6 }),
|
||
],
|
||
{ minConfidence: 0.5, maxComments: 10 },
|
||
);
|
||
expect(r.posted[0]!.severity).toBe("critical");
|
||
});
|
||
|
||
test("caps comment count but keeps the overflow as suppressed", () => {
|
||
const many = Array.from({ length: 30 }, (_, i) =>
|
||
f({ file: `f${i}.ts`, claim: `c${i}` }),
|
||
);
|
||
const r = aggregate(many, { minConfidence: 0.5, maxComments: 5 });
|
||
expect(r.posted).toHaveLength(5);
|
||
expect(r.suppressed).toHaveLength(25);
|
||
});
|
||
```
|
||
|
||
**Step 2: Run it and watch it fail**
|
||
|
||
Run: `npx vitest run tests/aggregate/aggregate.test.ts` → FAIL.
|
||
|
||
**Step 3: Write the aggregator**
|
||
|
||
`src/aggregate/aggregate.ts`:
|
||
|
||
```typescript
|
||
import type { Finding, Severity } from "../analyze/types.js";
|
||
|
||
export interface AggregateOptions {
|
||
minConfidence: number;
|
||
maxComments: number;
|
||
/** Findings within this many lines in the same file+category are duplicates. */
|
||
lineWindow?: number;
|
||
}
|
||
|
||
export interface AggregateResult {
|
||
posted: Finding[];
|
||
suppressed: Finding[];
|
||
}
|
||
|
||
const RANK: Record<Severity, number> = {
|
||
critical: 4,
|
||
high: 3,
|
||
medium: 2,
|
||
low: 1,
|
||
};
|
||
|
||
export function aggregate(
|
||
findings: Finding[],
|
||
opts: AggregateOptions,
|
||
): AggregateResult {
|
||
const window = opts.lineWindow ?? 5;
|
||
const suppressed: Finding[] = [];
|
||
|
||
const confident = findings.filter((f) => {
|
||
if (f.confidence >= opts.minConfidence) return true;
|
||
suppressed.push(f);
|
||
return false;
|
||
});
|
||
|
||
const ranked = [...confident].sort(
|
||
(a, b) =>
|
||
RANK[b.severity] - RANK[a.severity] || b.confidence - a.confidence,
|
||
);
|
||
|
||
const kept: Finding[] = [];
|
||
for (const f of ranked) {
|
||
const dup = kept.some(
|
||
(k) =>
|
||
k.file === f.file &&
|
||
k.category === f.category &&
|
||
Math.abs(k.line - f.line) <= window,
|
||
);
|
||
if (dup) suppressed.push(f);
|
||
else kept.push(f);
|
||
}
|
||
|
||
return {
|
||
posted: kept.slice(0, opts.maxComments),
|
||
suppressed: [...suppressed, ...kept.slice(opts.maxComments)],
|
||
};
|
||
}
|
||
```
|
||
|
||
**Step 4: Run it and watch it pass**
|
||
|
||
Run: `npx vitest run tests/aggregate/aggregate.test.ts` → 4 passed.
|
||
|
||
**Step 5: Commit**
|
||
|
||
```bash
|
||
git add src/aggregate tests/aggregate
|
||
git commit -m "feat: dedupe, rank, and cap findings"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 9: JSONL emitter
|
||
|
||
**Files:**
|
||
- Create: `src/emit/record.ts`, `src/emit/jsonl.ts`, `tests/emit/jsonl.test.ts`
|
||
|
||
**Step 1: Write the failing test**
|
||
|
||
`tests/emit/jsonl.test.ts`:
|
||
|
||
```typescript
|
||
import { mkdtempSync, readFileSync } from "node:fs";
|
||
import { tmpdir } from "node:os";
|
||
import { join } from "node:path";
|
||
import { expect, test } from "vitest";
|
||
import { JsonlEmitter } from "../../src/emit/jsonl.js";
|
||
import type { RunRecord } from "../../src/emit/record.js";
|
||
|
||
const record: RunRecord = {
|
||
runId: "run_1",
|
||
repo: "demo",
|
||
pullRequest: null,
|
||
commit: "a".repeat(40),
|
||
tier: "full",
|
||
tierReason: "rule:risk_path(**/auth/**)",
|
||
analyzers: ["code-quality"],
|
||
tokens: { in: 100, out: 20, cached: 0 },
|
||
costUsd: 0.01,
|
||
latencyMs: 1200,
|
||
findings: { total: 2, posted: 1, suppressed: 1 },
|
||
verdict: "changes_requested",
|
||
models: { "code-quality": "claude-opus-5" },
|
||
pragentVersion: "0.1.0",
|
||
startedAt: "2026-08-04T00:00:00.000Z",
|
||
};
|
||
|
||
test("appends one JSON object per line", async () => {
|
||
const dir = mkdtempSync(join(tmpdir(), "pragent-emit-"));
|
||
const path = join(dir, "nested", "runs.jsonl");
|
||
const emitter = new JsonlEmitter(path);
|
||
|
||
await emitter.emit(record);
|
||
await emitter.emit({ ...record, runId: "run_2" });
|
||
|
||
const lines = readFileSync(path, "utf8").trim().split("\n");
|
||
expect(lines).toHaveLength(2);
|
||
expect(JSON.parse(lines[0]!).run_id).toBe("run_1");
|
||
expect(JSON.parse(lines[1]!).tier_reason).toMatch(/risk_path/);
|
||
});
|
||
```
|
||
|
||
Note the snake_case assertion: the record is camelCase in TypeScript and snake_case on the wire, so downstream consumers (OTel, dashboards, `jq`) see conventional field names.
|
||
|
||
**Step 2: Run it and watch it fail**
|
||
|
||
Run: `npx vitest run tests/emit/jsonl.test.ts` → FAIL.
|
||
|
||
**Step 3: Write the record type**
|
||
|
||
`src/emit/record.ts`:
|
||
|
||
```typescript
|
||
import type { Tier } from "../tier/types.js";
|
||
|
||
export type Verdict = "approved" | "commented" | "changes_requested" | "error";
|
||
|
||
export interface RunRecord {
|
||
runId: string;
|
||
repo: string;
|
||
pullRequest: number | null;
|
||
commit: string;
|
||
tier: Tier;
|
||
tierReason: string;
|
||
analyzers: string[];
|
||
tokens: { in: number; out: number; cached: number };
|
||
costUsd: number;
|
||
latencyMs: number;
|
||
findings: { total: number; posted: number; suppressed: number };
|
||
verdict: Verdict;
|
||
models: Record<string, string>;
|
||
pragentVersion: string;
|
||
startedAt: string;
|
||
}
|
||
|
||
export function toWireFormat(r: RunRecord): Record<string, unknown> {
|
||
return {
|
||
run_id: r.runId,
|
||
repo: r.repo,
|
||
pr: r.pullRequest,
|
||
commit: r.commit,
|
||
tier: r.tier,
|
||
tier_reason: r.tierReason,
|
||
analyzers: r.analyzers,
|
||
tokens: r.tokens,
|
||
cost_usd: r.costUsd,
|
||
latency_ms: r.latencyMs,
|
||
findings: r.findings,
|
||
verdict: r.verdict,
|
||
models: r.models,
|
||
pragent_version: r.pragentVersion,
|
||
started_at: r.startedAt,
|
||
};
|
||
}
|
||
```
|
||
|
||
**Step 4: Write the emitter**
|
||
|
||
`src/emit/jsonl.ts`:
|
||
|
||
```typescript
|
||
import { appendFile, mkdir } from "node:fs/promises";
|
||
import { dirname } from "node:path";
|
||
import { toWireFormat, type RunRecord } from "./record.js";
|
||
|
||
export interface Emitter {
|
||
emit(record: RunRecord): Promise<void>;
|
||
}
|
||
|
||
export class JsonlEmitter implements Emitter {
|
||
constructor(private readonly path: string) {}
|
||
|
||
async emit(record: RunRecord): Promise<void> {
|
||
await mkdir(dirname(this.path), { recursive: true });
|
||
await appendFile(this.path, JSON.stringify(toWireFormat(record)) + "\n", "utf8");
|
||
}
|
||
}
|
||
```
|
||
|
||
**Step 5: Run it and watch it pass**
|
||
|
||
Run: `npx vitest run tests/emit/jsonl.test.ts` → 1 passed.
|
||
|
||
**Step 6: Commit**
|
||
|
||
```bash
|
||
git add src/emit tests/emit
|
||
git commit -m "feat: emit JSONL run records"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 10: Wire the CLI
|
||
|
||
**Files:**
|
||
- Create: `src/cli.ts`, `src/review.ts`, `tests/review.test.ts`
|
||
|
||
**Step 1: Write the failing test**
|
||
|
||
`tests/review.test.ts` — the orchestration test, with every port faked:
|
||
|
||
```typescript
|
||
import { mkdtempSync, readFileSync } from "node:fs";
|
||
import { tmpdir } from "node:os";
|
||
import { join } from "node:path";
|
||
import { expect, test, vi } from "vitest";
|
||
import { review } from "../src/review.js";
|
||
import type { ForgeAdapter, ReviewContext } from "../src/forge/types.js";
|
||
import type { ModelClient } from "../src/analyze/model.js";
|
||
|
||
const ctx: ReviewContext = {
|
||
repo: "demo",
|
||
pullRequest: null,
|
||
headSha: "a".repeat(40),
|
||
baseSha: "b".repeat(40),
|
||
title: "add login",
|
||
files: [
|
||
{ path: "src/auth/login.ts", status: "modified", added: 5, removed: 1, patch: "+x" },
|
||
],
|
||
};
|
||
|
||
const forge = (): ForgeAdapter & { published: unknown[] } => {
|
||
const published: unknown[] = [];
|
||
return {
|
||
name: "fake",
|
||
published,
|
||
async getContext() { return ctx; },
|
||
async publish(_c, findings) { published.push(...findings); },
|
||
};
|
||
};
|
||
|
||
const model: ModelClient = {
|
||
name: "fake",
|
||
async complete() {
|
||
return {
|
||
text: JSON.stringify({
|
||
findings: [{
|
||
file: "src/auth/login.ts", line: 3, severity: "high", confidence: 0.9,
|
||
category: "correctness", claim: "token is logged in plaintext",
|
||
}],
|
||
}),
|
||
usage: { inputTokens: 100, outputTokens: 20, costUsd: 0.02 },
|
||
};
|
||
},
|
||
};
|
||
|
||
test("classifies, analyzes, publishes, and records", async () => {
|
||
const dir = mkdtempSync(join(tmpdir(), "pragent-review-"));
|
||
const f = forge();
|
||
|
||
const result = await review({
|
||
forge: f,
|
||
model,
|
||
repoRoot: dir,
|
||
emitPath: join(dir, "runs.jsonl"),
|
||
});
|
||
|
||
expect(result.tier).toBe("full"); // risk path
|
||
expect(result.verdict).toBe("changes_requested");
|
||
expect(f.published).toHaveLength(1);
|
||
|
||
const line = JSON.parse(readFileSync(join(dir, "runs.jsonl"), "utf8").trim());
|
||
expect(line.tier_reason).toMatch(/risk_path/);
|
||
expect(line.cost_usd).toBeCloseTo(0.02);
|
||
});
|
||
|
||
test("a trivial change skips the model entirely", async () => {
|
||
const dir = mkdtempSync(join(tmpdir(), "pragent-review-"));
|
||
const spy = vi.fn(model.complete);
|
||
const f = forge();
|
||
f.getContext = async () => ({
|
||
...ctx,
|
||
files: [{ path: "pnpm-lock.yaml", status: "modified", added: 200, removed: 100, patch: "" }],
|
||
});
|
||
|
||
const result = await review({
|
||
forge: f,
|
||
model: { name: "fake", complete: spy },
|
||
repoRoot: dir,
|
||
emitPath: join(dir, "runs.jsonl"),
|
||
});
|
||
|
||
expect(result.tier).toBe("trivial");
|
||
expect(spy).not.toHaveBeenCalled(); // this is the whole point of tiering
|
||
expect(result.verdict).toBe("approved");
|
||
});
|
||
```
|
||
|
||
**Step 2: Run it and watch it fail**
|
||
|
||
Run: `npx vitest run tests/review.test.ts` → FAIL.
|
||
|
||
**Step 3: Write the orchestrator**
|
||
|
||
`src/review.ts`:
|
||
|
||
```typescript
|
||
import { randomUUID } from "node:crypto";
|
||
import { runAnalyzer } from "./analyze/runner.js";
|
||
import type { ModelClient } from "./analyze/model.js";
|
||
import { aggregate } from "./aggregate/aggregate.js";
|
||
import { loadConfig } from "./config/load.js";
|
||
import { JsonlEmitter } from "./emit/jsonl.js";
|
||
import type { Verdict } from "./emit/record.js";
|
||
import type { ForgeAdapter } from "./forge/types.js";
|
||
import { classify } from "./tier/rules.js";
|
||
import type { Tier } from "./tier/types.js";
|
||
import { VERSION } from "./index.js";
|
||
|
||
export interface ReviewOptions {
|
||
forge: ForgeAdapter;
|
||
model: ModelClient;
|
||
repoRoot: string;
|
||
emitPath?: string;
|
||
}
|
||
|
||
export interface ReviewResult {
|
||
tier: Tier;
|
||
verdict: Verdict;
|
||
postedCount: number;
|
||
}
|
||
|
||
export async function review(opts: ReviewOptions): Promise<ReviewResult> {
|
||
const startedAt = new Date().toISOString();
|
||
const t0 = Date.now();
|
||
|
||
const config = await loadConfig(opts.repoRoot);
|
||
const ctx = await opts.forge.getContext();
|
||
const decision = classify(ctx.files, config.tierPolicy);
|
||
|
||
// Trivial changes never reach a model — the tier is the cost control.
|
||
if (decision.tier === "trivial") {
|
||
await emit("approved", [], [], { in: 0, out: 0, cached: 0 }, 0, []);
|
||
return { tier: decision.tier, verdict: "approved", postedCount: 0 };
|
||
}
|
||
|
||
const result = await runAnalyzer({ id: "code-quality", model: opts.model, ctx });
|
||
const { posted, suppressed } = aggregate(result.findings, {
|
||
minConfidence: config.minConfidence,
|
||
maxComments: config.maxComments,
|
||
});
|
||
|
||
await opts.forge.publish(ctx, posted);
|
||
|
||
const verdict: Verdict = result.error
|
||
? "error"
|
||
: posted.some((f) => f.severity === "critical" || f.severity === "high")
|
||
? "changes_requested"
|
||
: posted.length > 0
|
||
? "commented"
|
||
: "approved";
|
||
|
||
await emit(
|
||
verdict,
|
||
posted,
|
||
suppressed,
|
||
{ in: result.usage.inputTokens, out: result.usage.outputTokens, cached: 0 },
|
||
result.usage.costUsd,
|
||
["code-quality"],
|
||
);
|
||
|
||
return { tier: decision.tier, verdict, postedCount: posted.length };
|
||
|
||
async function emit(
|
||
verdict: Verdict,
|
||
posted: unknown[],
|
||
suppressed: unknown[],
|
||
tokens: { in: number; out: number; cached: number },
|
||
costUsd: number,
|
||
analyzers: string[],
|
||
) {
|
||
const emitter = new JsonlEmitter(opts.emitPath ?? config.emitPath);
|
||
await emitter.emit({
|
||
runId: `run_${randomUUID()}`,
|
||
repo: ctx.repo,
|
||
pullRequest: ctx.pullRequest,
|
||
commit: ctx.headSha,
|
||
tier: decision.tier,
|
||
tierReason: decision.reason,
|
||
analyzers,
|
||
tokens,
|
||
costUsd,
|
||
latencyMs: Date.now() - t0,
|
||
findings: {
|
||
total: posted.length + suppressed.length,
|
||
posted: posted.length,
|
||
suppressed: suppressed.length,
|
||
},
|
||
verdict,
|
||
models: analyzers.length ? { "code-quality": opts.model.name } : {},
|
||
pragentVersion: VERSION,
|
||
startedAt,
|
||
});
|
||
}
|
||
}
|
||
```
|
||
|
||
**Step 4: Run it and watch it pass**
|
||
|
||
Run: `npx vitest run tests/review.test.ts` → 2 passed.
|
||
|
||
**Step 5: Write the CLI**
|
||
|
||
`src/cli.ts`:
|
||
|
||
```typescript
|
||
#!/usr/bin/env node
|
||
import { Command } from "commander";
|
||
import { PiModelClient } from "./analyze/pi-model.js";
|
||
import { loadConfig } from "./config/load.js";
|
||
import { LocalForgeAdapter } from "./forge/local.js";
|
||
import { review } from "./review.js";
|
||
import { VERSION } from "./index.js";
|
||
|
||
const program = new Command();
|
||
|
||
program.name("pragent").version(VERSION);
|
||
|
||
program
|
||
.command("review")
|
||
.description("Review a change and publish findings")
|
||
.option("--range <range>", "git range to review", "HEAD~1..HEAD")
|
||
.option("--cwd <dir>", "repository root", process.cwd())
|
||
.option("--model <id>", "model id override")
|
||
.action(async (opts) => {
|
||
const config = await loadConfig(opts.cwd);
|
||
const result = await review({
|
||
forge: new LocalForgeAdapter({ cwd: opts.cwd, range: opts.range }),
|
||
model: new PiModelClient({ model: opts.model ?? config.models.full }),
|
||
repoRoot: opts.cwd,
|
||
});
|
||
|
||
console.error(
|
||
`\ntier=${result.tier} verdict=${result.verdict} findings=${result.postedCount}`,
|
||
);
|
||
// 0 clean, 1 findings worth blocking on, 2 internal error.
|
||
process.exit(
|
||
result.verdict === "changes_requested" ? 1 : result.verdict === "error" ? 2 : 0,
|
||
);
|
||
});
|
||
|
||
program.parseAsync();
|
||
```
|
||
|
||
**Step 6: Verify end to end by hand**
|
||
|
||
```bash
|
||
npm run build
|
||
cd ~/Projects/pragent
|
||
ANTHROPIC_API_KEY=... node dist/cli.js review --range HEAD~1..HEAD
|
||
cat .pragent/runs.jsonl | tail -1 | jq
|
||
```
|
||
|
||
Expected: findings (or a clean pass) on stdout, `tier=... verdict=...` on stderr, one JSONL line appended, exit code matching the verdict.
|
||
|
||
**Step 7: Commit**
|
||
|
||
```bash
|
||
git add src/cli.ts src/review.ts tests/review.test.ts
|
||
git commit -m "feat: wire pragent review CLI end to end"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 11: Close out Phase 1
|
||
|
||
**Step 1: Full check**
|
||
|
||
```bash
|
||
npm test && npm run typecheck && npm run build
|
||
```
|
||
|
||
All green before proceeding.
|
||
|
||
**Step 2: Update the README**
|
||
|
||
Replace "implementation not started" with a short *Current state* section: what `pragent review` does today, the local-only limitation, and how to run it.
|
||
|
||
**Step 3: Commit and tag**
|
||
|
||
```bash
|
||
git add -A
|
||
git commit -m "docs: record phase 1 walking skeleton state"
|
||
git tag v0.1.0
|
||
git push origin main --tags
|
||
```
|
||
|
||
---
|
||
|
||
## Later phases (milestones, not tasks)
|
||
|
||
Do not start these without a plan of their own. Each gets the same treatment as above.
|
||
|
||
**Phase 2 — Gitea end to end.** `GiteaForgeAdapter` implementing the same port (diff via API, inline review comments, commit status). Woodpecker pipeline step. Token handling via CI secrets, never in config. *Done when:* a real PR in a Gitea repo gets reviewed automatically on push.
|
||
|
||
**Phase 3 — Profile and full tier.** `pragent init` writes `.pragent/profile.yml`; analyzers fan out with a byte-identical shared prefix; a cache-prefix invariant test fails the build if any analyzer perturbs it; real token/cost accounting from usage headers; tool budgets per analyzer. *Done when:* a full-tier PR runs 4 analyzers and the measured cost lands within the design doc's ~$0.80–2.00 band.
|
||
|
||
**Phase 4 — Extensibility hardening.** Analyzer plugin loading from `.pragent/analyzers/` and npm; config layering with locked org keys; `pragent explain` and `pragent replay`. *Done when:* someone adds a working analyzer without touching `src/`.
|
||
|
||
**Phase 5 — Second forge.** GitLab adapter plus a Jenkins recipe. This is the honest test of the port abstraction; expect it to expose one or two leaks. *Done when:* the same analyzer config works unmodified on Gitea and GitLab.
|
||
|
||
**Phase 6 — Analytics maturity.** OTel span export, the finding-outcome feedback loop (resolved/👎 → `finding_outcome` records), per-analyzer eval harness built on `replay`. *Done when:* false-positive rate per analyzer is a number you can read off a query.
|
||
|
||
---
|
||
|
||
## Known risks
|
||
|
||
- **The `pi` SDK surface in Task 7 is from published docs, not a verified local run.** Expect the event and usage shapes to need correcting on first contact. This is deliberately confined to one file behind the `ModelClient` port so a mismatch costs an hour, not a redesign.
|
||
- **The glob matcher in Task 4 is minimal** (`*` and `**` only). It is enough for the default risk paths. If repo configs start needing brace expansion or character classes, swap in `picomatch` — the seam is `matchGlob`, one function.
|
||
- **Usage accounting is approximate until Phase 3.** Do not build cost dashboards on Phase 1 numbers.
|