mirror of
https://github.com/tiennm99/ccs.git
synced 2026-08-06 00:22:23 +00:00
feat(ci): migrate AI review to self-hosted PR-Agent
This commit is contained in:
@@ -2,35 +2,36 @@ import { describe, expect, test } from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
|
||||
function loadWorkflow() {
|
||||
const workflowPath = path.resolve(import.meta.dir, '../../../../.github/workflows/ai-review.yml');
|
||||
return fs.readFileSync(workflowPath, 'utf8');
|
||||
function resolvePath(relativePath: string) {
|
||||
return path.resolve(import.meta.dir, relativePath);
|
||||
}
|
||||
|
||||
describe('ai-review workflow', () => {
|
||||
test('uses the claude-code-action reviewer path with configurable review runtime and PR-sha comment markers', () => {
|
||||
const workflow = loadWorkflow();
|
||||
describe('PR-Agent review lane migration', () => {
|
||||
test('keeps ai-review.yml as the PR-Agent workflow on the self-hosted cliproxy runner', () => {
|
||||
const workflowPath = resolvePath('../../../../.github/workflows/ai-review.yml');
|
||||
const prAgentConfigPath = resolvePath('../../../../.pr_agent.toml');
|
||||
|
||||
expect(workflow).toContain('timeout-minutes: 20');
|
||||
expect(workflow).toContain('Variables: AI_REVIEW_BASE_URL, AI_REVIEW_MODEL');
|
||||
expect(workflow).toContain('Secrets: AI_REVIEW_API_KEY');
|
||||
expect(workflow).toContain('ANTHROPIC_BASE_URL: ${{ vars.AI_REVIEW_BASE_URL }}');
|
||||
expect(workflow).toContain('REVIEW_MODEL: ${{ vars.AI_REVIEW_MODEL }}');
|
||||
expect(workflow).toContain('ANTHROPIC_AUTH_TOKEN: ${{ secrets.AI_REVIEW_API_KEY }}');
|
||||
expect(workflow).toContain('ANTHROPIC_MODEL: ${{ vars.AI_REVIEW_MODEL }}');
|
||||
expect(workflow).toContain('ANTHROPIC_DEFAULT_OPUS_MODEL: ${{ vars.AI_REVIEW_MODEL }}');
|
||||
expect(workflow).toContain('ANTHROPIC_DEFAULT_SONNET_MODEL: ${{ vars.AI_REVIEW_MODEL }}');
|
||||
expect(workflow).toContain('ANTHROPIC_DEFAULT_HAIKU_MODEL: ${{ vars.AI_REVIEW_MODEL }}');
|
||||
expect(workflow).toContain('uses: anthropics/claude-code-action@v1');
|
||||
expect(workflow).toContain('anthropic_api_key: ${{ secrets.AI_REVIEW_API_KEY }}');
|
||||
expect(workflow).toContain('--model ${{ env.REVIEW_MODEL }}');
|
||||
expect(workflow).toContain('--max-turns 45');
|
||||
expect(workflow).toContain('--json-schema');
|
||||
expect(workflow).toContain('normalize-ai-review-output.mjs');
|
||||
expect(workflow).not.toContain('build-ai-review-packet.mjs');
|
||||
expect(workflow).not.toContain('run-ai-review-direct.mjs');
|
||||
expect(workflow).toContain('pr:${{ needs.prepare.outputs.pr_number }}');
|
||||
expect(workflow).toContain('sha:${{ needs.prepare.outputs.head_sha }}');
|
||||
expect(workflow).not.toContain('run:${{ github.run_id }}');
|
||||
expect(fs.existsSync(workflowPath)).toBe(true);
|
||||
expect(fs.existsSync(prAgentConfigPath)).toBe(true);
|
||||
|
||||
const workflow = fs.readFileSync(workflowPath, 'utf8');
|
||||
const config = fs.readFileSync(prAgentConfigPath, 'utf8');
|
||||
|
||||
expect(workflow).toContain('name: AI Code Review');
|
||||
expect(workflow).toContain('runs-on: [self-hosted, cliproxy]');
|
||||
expect(workflow).toContain('uses: qodo-ai/pr-agent');
|
||||
expect(workflow).toContain('OPENAI.API_BASE');
|
||||
expect(workflow).toContain('OPENAI_KEY');
|
||||
expect(workflow).toContain('config.model');
|
||||
expect(workflow).toContain('github_action_config.auto_review');
|
||||
expect(workflow).not.toContain('uses: anthropics/claude-code-action@v1');
|
||||
expect(workflow).not.toContain('AI_REVIEW_API_KEY');
|
||||
|
||||
expect(config).toContain('[config]');
|
||||
expect(config).toContain('git_provider = "github"');
|
||||
expect(config).toMatch(/\bmodel\s*=\s*"[^"\n]+"/);
|
||||
expect(config).toContain('[pr_reviewer]');
|
||||
expect(config).not.toContain('auto_review = true');
|
||||
expect(config).not.toContain('claude-code-action');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,104 +0,0 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
|
||||
const packetBuilder = await import('../../../../scripts/github/build-ai-review-packet.mjs');
|
||||
|
||||
function withTempDir(prefix: string, run: (tempDir: string) => void) {
|
||||
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
|
||||
try {
|
||||
run(tempDir);
|
||||
} finally {
|
||||
fs.rmSync(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
describe('build-ai-review-packet', () => {
|
||||
test('adds stable line numbers to packet content blocks', () => {
|
||||
expect(packetBuilder.addLineNumbers('first\nsecond')).toBe(' 1 | first\n 2 | second');
|
||||
});
|
||||
|
||||
test('builds a packet with current and base snapshots for selected files', () => {
|
||||
withTempDir('ai-review-packet-', (tempDir) => {
|
||||
const rootDir = path.join(tempDir, 'repo');
|
||||
const baseDir = path.join(tempDir, '.ccs-ai-review-base');
|
||||
fs.mkdirSync(path.join(rootDir, 'src'), { recursive: true });
|
||||
fs.mkdirSync(path.join(baseDir, 'src'), { recursive: true });
|
||||
fs.writeFileSync(path.join(rootDir, 'src/example.ts'), 'export const value = 2;\nconsole.log(value);\n');
|
||||
fs.writeFileSync(path.join(baseDir, 'src/example.ts'), 'export const value = 1;\n');
|
||||
|
||||
const packet = packetBuilder.buildReviewPacket({
|
||||
scopeMarkdown: '# AI Review Scope\n\n- Selected files: 1 of 1 reviewable files',
|
||||
files: ['src/example.ts'],
|
||||
rootDir,
|
||||
baseDir,
|
||||
maxChars: 20000,
|
||||
perFileMaxLines: 40,
|
||||
perFileMaxChars: 4000,
|
||||
}).packet;
|
||||
|
||||
expect(packet).toContain('# AI Review Packet');
|
||||
expect(packet).toContain('## File: `src/example.ts`');
|
||||
expect(packet).toContain('### Current file content');
|
||||
expect(packet).toContain(' 1 | export const value = 2;');
|
||||
expect(packet).toContain('### Base snapshot content');
|
||||
expect(packet).toContain(' 1 | export const value = 1;');
|
||||
expect(packet).toContain('- Selected files in packet: 1 of 1');
|
||||
});
|
||||
});
|
||||
|
||||
test('omits file snapshots when the global packet budget is exceeded', () => {
|
||||
withTempDir('ai-review-packet-', (tempDir) => {
|
||||
const rootDir = path.join(tempDir, 'repo');
|
||||
const baseDir = path.join(tempDir, '.ccs-ai-review-base');
|
||||
fs.mkdirSync(path.join(rootDir, 'src'), { recursive: true });
|
||||
fs.mkdirSync(path.join(baseDir, 'src'), { recursive: true });
|
||||
|
||||
fs.writeFileSync(path.join(rootDir, 'src/one.ts'), 'export const one = 1;\n'.repeat(30));
|
||||
fs.writeFileSync(path.join(rootDir, 'src/two.ts'), 'export const two = 2;\n'.repeat(30));
|
||||
fs.writeFileSync(path.join(baseDir, 'src/one.ts'), 'export const one = 0;\n');
|
||||
fs.writeFileSync(path.join(baseDir, 'src/two.ts'), 'export const two = 0;\n');
|
||||
|
||||
const packet = packetBuilder.buildReviewPacket({
|
||||
scopeMarkdown: '# AI Review Scope\n\n' + '- review scope metadata\n'.repeat(60),
|
||||
files: ['src/one.ts', 'src/two.ts'],
|
||||
rootDir,
|
||||
baseDir,
|
||||
maxChars: 1500,
|
||||
perFileMaxLines: 120,
|
||||
perFileMaxChars: 8000,
|
||||
}).packet;
|
||||
|
||||
expect(packet).not.toContain('## File: `src/two.ts`');
|
||||
expect(packet).toContain('Additional selected files omitted from packet due to the global context budget: 2');
|
||||
});
|
||||
});
|
||||
|
||||
test('keeps the full packet within the configured maxChars budget', () => {
|
||||
withTempDir('ai-review-packet-', (tempDir) => {
|
||||
const rootDir = path.join(tempDir, 'repo');
|
||||
const baseDir = path.join(tempDir, '.ccs-ai-review-base');
|
||||
fs.mkdirSync(path.join(rootDir, 'src'), { recursive: true });
|
||||
fs.mkdirSync(path.join(baseDir, 'src'), { recursive: true });
|
||||
|
||||
fs.writeFileSync(path.join(rootDir, 'src/huge.ts'), 'export const huge = 1;\n'.repeat(80));
|
||||
fs.writeFileSync(path.join(baseDir, 'src/huge.ts'), 'export const base = 0;\n'.repeat(80));
|
||||
|
||||
const result = packetBuilder.buildReviewPacket({
|
||||
scopeMarkdown: '# AI Review Scope\n\n' + '- selected file\n'.repeat(200),
|
||||
files: ['src/huge.ts'],
|
||||
rootDir,
|
||||
baseDir,
|
||||
maxChars: 1000,
|
||||
perFileMaxLines: 200,
|
||||
perFileMaxChars: 12000,
|
||||
});
|
||||
|
||||
expect(result.packet.length).toBeLessThanOrEqual(1000);
|
||||
expect(result.totalSelectedFiles).toBe(1);
|
||||
expect(result.includedFilePaths).toEqual([]);
|
||||
expect(result.packet).toContain('Scope metadata was truncated to preserve packet budget for file contents.');
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,235 +0,0 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
|
||||
const reviewOutput = await import('../../../../scripts/github/normalize-ai-review-output.mjs');
|
||||
|
||||
function withTempDir(prefix: string, run: (tempDir: string) => void) {
|
||||
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
|
||||
try {
|
||||
run(tempDir);
|
||||
} finally {
|
||||
fs.rmSync(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
describe('normalize-ai-review-output', () => {
|
||||
test('renders validated structured output into the legacy long-form markdown layout', () => {
|
||||
const validation = reviewOutput.normalizeStructuredOutput(
|
||||
JSON.stringify({
|
||||
summary: 'The PR is mostly correct, but one blocking regression remains.',
|
||||
findings: [
|
||||
{
|
||||
severity: 'high',
|
||||
title: 'Ambiguous account lookup drops valid matches',
|
||||
file: 'src/cliproxy/accounts/query.ts',
|
||||
line: 61,
|
||||
what: 'Exact email matches can return null when duplicate accounts exist.',
|
||||
why: 'That breaks normal selection flows for users with multiple Codex sessions.',
|
||||
fix: 'Match by stable account identity first and keep ambiguous email lookups out of exact-match paths.',
|
||||
},
|
||||
],
|
||||
securityChecklist: [
|
||||
{
|
||||
check: 'Injection safety',
|
||||
status: 'pass',
|
||||
notes: 'No user-controlled input reaches a shell, SQL, or HTML boundary in this diff.',
|
||||
},
|
||||
],
|
||||
ccsCompliance: [
|
||||
{
|
||||
rule: 'No emojis in CLI',
|
||||
status: 'na',
|
||||
notes: 'This change affects GitHub PR comments only, not CLI stdout.',
|
||||
},
|
||||
],
|
||||
informational: ['The renderer still escapes markdown before publishing comment content.'],
|
||||
strengths: [
|
||||
'The formatter owns the output shape instead of trusting the model to author markdown.',
|
||||
],
|
||||
overallAssessment: 'changes_requested',
|
||||
overallRationale: 'The blocking lookup regression should be fixed before merge.',
|
||||
})
|
||||
);
|
||||
|
||||
expect(validation.ok).toBe(true);
|
||||
const markdown = reviewOutput.renderStructuredReview(validation.value, {
|
||||
model: 'glm-5-turbo',
|
||||
});
|
||||
|
||||
expect(markdown).toContain('### 📋 Summary');
|
||||
expect(markdown).toContain('### 🔍 Findings');
|
||||
expect(markdown).toContain('### 🔴 High');
|
||||
expect(markdown).toContain(
|
||||
'**`src/cliproxy/accounts/query.ts:61` — Ambiguous account lookup drops valid matches**'
|
||||
);
|
||||
expect(markdown).toContain(
|
||||
'Problem: Exact email matches can return null when duplicate accounts exist.'
|
||||
);
|
||||
expect(markdown).toContain(
|
||||
'Why it matters: That breaks normal selection flows for users with multiple Codex sessions.'
|
||||
);
|
||||
expect(markdown).toContain(
|
||||
'Suggested fix: Match by stable account identity first and keep ambiguous email lookups out of exact-match paths.'
|
||||
);
|
||||
expect(markdown).toContain('### 🔒 Security Checklist');
|
||||
expect(markdown).toContain(
|
||||
'| Injection safety | ✅ | No user-controlled input reaches a shell, SQL, or HTML boundary in this diff. |'
|
||||
);
|
||||
expect(markdown).toContain('### 📊 CCS Compliance');
|
||||
expect(markdown).toContain(
|
||||
'| No emojis in CLI | N/A | This change affects GitHub PR comments only, not CLI stdout. |'
|
||||
);
|
||||
expect(markdown).toContain('### 💡 Informational');
|
||||
expect(markdown).toContain("### ✅ What's Done Well");
|
||||
expect(markdown).toContain('### 🎯 Overall Assessment');
|
||||
expect(markdown).toContain('**❌ CHANGES REQUESTED**');
|
||||
expect(markdown).toContain('> 🤖 Reviewed by `glm-5-turbo`');
|
||||
});
|
||||
|
||||
test('keeps summary-first layout while still rendering review context metadata', () => {
|
||||
const validation = reviewOutput.normalizeStructuredOutput(
|
||||
JSON.stringify({
|
||||
summary: 'The large diff review stayed focused on the riskiest hotspots.',
|
||||
findings: [],
|
||||
securityChecklist: [
|
||||
{
|
||||
check: 'Workflow safety',
|
||||
status: 'pass',
|
||||
notes: 'The review stayed read-only and did not invoke write-capable tools.',
|
||||
},
|
||||
],
|
||||
ccsCompliance: [
|
||||
{
|
||||
rule: 'Plain structured output',
|
||||
status: 'pass',
|
||||
notes: 'The assistant returned data fields only, without layout markdown.',
|
||||
},
|
||||
],
|
||||
informational: [],
|
||||
strengths: [],
|
||||
overallAssessment: 'approved_with_notes',
|
||||
overallRationale: 'The review stayed bounded and did not surface blocking regressions.',
|
||||
})
|
||||
);
|
||||
|
||||
expect(validation.ok).toBe(true);
|
||||
const markdown = reviewOutput.renderStructuredReview(validation.value, {
|
||||
model: 'glm-5-turbo',
|
||||
rendering: {
|
||||
mode: 'triage',
|
||||
selectedFiles: 8,
|
||||
reviewableFiles: 34,
|
||||
selectedChanges: 620,
|
||||
reviewableChanges: 2140,
|
||||
packetIncludedFiles: 6,
|
||||
packetTotalFiles: 8,
|
||||
packetOmittedFiles: 2,
|
||||
maxTurns: 6,
|
||||
timeoutMinutes: 5,
|
||||
},
|
||||
});
|
||||
|
||||
expect(markdown).toContain('### 📋 Summary');
|
||||
expect(markdown).toContain(
|
||||
'> 🧭 `triage` • 8/34 files • 620/2140 lines • packet 6/8 • 6 turns / 5 minutes'
|
||||
);
|
||||
expect(markdown).toContain('### 🎯 Overall Assessment');
|
||||
});
|
||||
|
||||
test('renders finding snippets as renderer-owned fenced code blocks', () => {
|
||||
const validation = reviewOutput.normalizeStructuredOutput(
|
||||
JSON.stringify({
|
||||
summary: 'One non-blocking follow-up remains.',
|
||||
findings: [
|
||||
{
|
||||
severity: 'medium',
|
||||
title: 'Fallback branch still writes the stale marker file',
|
||||
file: '.github/workflows/ai-review.yml',
|
||||
line: 181,
|
||||
what: 'One branch still writes the old marker file path.',
|
||||
why: 'That can leave duplicate bot comments on reruns for the same PR SHA.',
|
||||
fix: 'Keep the rerun marker keyed to PR plus head SHA in every publish branch.',
|
||||
snippets: [
|
||||
{
|
||||
label: 'Current publish branch',
|
||||
language: 'bash',
|
||||
code: 'marker_file=\"$RUNNER_TEMP/.ai-review-marker\"\nprintf \"%s\\n\" \"$REVIEW_MARKER\" > \"$marker_file\"',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
securityChecklist: [{ check: 'Workflow safety', status: 'pass', notes: 'Covered.' }],
|
||||
ccsCompliance: [{ rule: 'Renderer-owned markdown', status: 'pass', notes: 'Covered.' }],
|
||||
informational: [],
|
||||
strengths: [],
|
||||
overallAssessment: 'approved_with_notes',
|
||||
overallRationale: 'This is a deterministic formatting-only follow-up.',
|
||||
})
|
||||
);
|
||||
|
||||
expect(validation.ok).toBe(true);
|
||||
const markdown = reviewOutput.renderStructuredReview(validation.value, {
|
||||
model: 'glm-5-turbo',
|
||||
});
|
||||
|
||||
expect(markdown).toContain('Evidence: Current publish branch');
|
||||
expect(markdown).toContain('```bash');
|
||||
expect(markdown).toContain('marker_file="$RUNNER_TEMP/.ai-review-marker"');
|
||||
});
|
||||
|
||||
test('writes a safe incomplete comment instead of leaking raw assistant text', () => {
|
||||
withTempDir('ai-review-', (tempDir) => {
|
||||
const executionFile = path.join(tempDir, 'claude-execution-output.json');
|
||||
const outputFile = path.join(tempDir, 'pr_review.md');
|
||||
|
||||
fs.writeFileSync(
|
||||
executionFile,
|
||||
JSON.stringify([
|
||||
{ type: 'system', subtype: 'init', tools: ['Bash', 'Edit', 'Read'] },
|
||||
{
|
||||
type: 'result',
|
||||
subtype: 'success',
|
||||
num_turns: 25,
|
||||
result: 'Now let me verify the findings before I finalize the review...',
|
||||
},
|
||||
])
|
||||
);
|
||||
|
||||
const result = reviewOutput.writeReviewFromEnv({
|
||||
AI_REVIEW_EXECUTION_FILE: executionFile,
|
||||
AI_REVIEW_MODEL: 'glm-5-turbo',
|
||||
AI_REVIEW_OUTPUT_FILE: outputFile,
|
||||
AI_REVIEW_RUN_URL: 'https://github.com/kaitranntt/ccs/actions/runs/23758377592',
|
||||
AI_REVIEW_STRUCTURED_OUTPUT: '',
|
||||
});
|
||||
|
||||
expect(result.usedFallback).toBe(true);
|
||||
|
||||
const markdown = fs.readFileSync(outputFile, 'utf8');
|
||||
expect(markdown).toContain('### ⚠️ AI Review Incomplete');
|
||||
expect(markdown).toContain('Runtime tools: `Bash`, `Edit`, `Read`');
|
||||
expect(markdown).toContain('Turns used: 25');
|
||||
expect(markdown).not.toContain('Now let me verify the findings');
|
||||
});
|
||||
});
|
||||
|
||||
test('rejects ad hoc layout markup inside structured fields', () => {
|
||||
const validation = reviewOutput.normalizeStructuredOutput(
|
||||
JSON.stringify({
|
||||
summary: '# PR #860 Review',
|
||||
findings: [],
|
||||
securityChecklist: [{ check: 'Injection safety', status: 'pass', notes: 'Covered.' }],
|
||||
ccsCompliance: [{ rule: 'ASCII-only CLI output', status: 'pass', notes: 'Unaffected.' }],
|
||||
informational: [],
|
||||
strengths: [],
|
||||
overallAssessment: 'approved_with_notes',
|
||||
overallRationale: 'The review is otherwise valid.',
|
||||
})
|
||||
);
|
||||
|
||||
expect(validation.ok).toBe(false);
|
||||
expect(validation.reason).toContain('summary contains');
|
||||
});
|
||||
});
|
||||
@@ -1,213 +0,0 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
|
||||
const reviewScope = await import('../../../../scripts/github/prepare-ai-review-scope.mjs');
|
||||
|
||||
describe('prepare-ai-review-scope', () => {
|
||||
test('paginates pull request files and preserves all pages', async () => {
|
||||
const pageOneHeaders = new Headers({
|
||||
link: '<https://api.github.com/repos/kaitranntt/ccs/pulls/880/files?page=2>; rel="next"',
|
||||
});
|
||||
const pageTwoHeaders = new Headers();
|
||||
|
||||
const files = await reviewScope.collectPullRequestFiles(
|
||||
'https://api.github.com/repos/kaitranntt/ccs/pulls/880/files?page=1',
|
||||
async (url: string) => {
|
||||
if (url.endsWith('page=1')) {
|
||||
return {
|
||||
body: [{ filename: 'src/commands/review.ts', status: 'modified', additions: 5, deletions: 2, patch: '+a' }],
|
||||
headers: pageOneHeaders,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
body: [{ filename: 'scripts/github/normalize-ai-review-output.mjs', status: 'modified', additions: 8, deletions: 1, patch: '+b' }],
|
||||
headers: pageTwoHeaders,
|
||||
};
|
||||
}
|
||||
);
|
||||
|
||||
expect(files).toHaveLength(2);
|
||||
expect(files[1].filename).toBe('scripts/github/normalize-ai-review-output.mjs');
|
||||
});
|
||||
|
||||
test('prefers reviewable high-risk files and excludes low-signal churn in triage mode', () => {
|
||||
const scope = reviewScope.buildReviewScope(
|
||||
reviewScope.normalizePullFiles([
|
||||
{
|
||||
filename: '.github/review-prompt.md',
|
||||
status: 'modified',
|
||||
additions: 12,
|
||||
deletions: 4,
|
||||
changes: 16,
|
||||
patch: '@@ -1 +1 @@\n-old\n+new',
|
||||
},
|
||||
{
|
||||
filename: '.github/workflows/ai-review.yml',
|
||||
status: 'modified',
|
||||
additions: 120,
|
||||
deletions: 45,
|
||||
changes: 165,
|
||||
patch: '@@ -1 +1 @@\n-old\n+new',
|
||||
},
|
||||
{
|
||||
filename: 'scripts/github/normalize-ai-review-output.mjs',
|
||||
status: 'modified',
|
||||
additions: 40,
|
||||
deletions: 10,
|
||||
changes: 50,
|
||||
patch: '@@ -1 +1 @@\n-old\n+new',
|
||||
},
|
||||
{
|
||||
filename: 'README.md',
|
||||
status: 'modified',
|
||||
additions: 300,
|
||||
deletions: 0,
|
||||
changes: 300,
|
||||
patch: '@@ -1 +1 @@\n-old\n+new',
|
||||
},
|
||||
{
|
||||
filename: 'docs/ai-review.md',
|
||||
status: 'modified',
|
||||
additions: 180,
|
||||
deletions: 10,
|
||||
changes: 190,
|
||||
patch: '@@ -1 +1 @@\n-old\n+new',
|
||||
},
|
||||
]),
|
||||
'triage'
|
||||
);
|
||||
|
||||
expect(scope.mode).toBe('triage');
|
||||
expect(scope.selected.map((file: { filename: string }) => file.filename)).toEqual(
|
||||
expect.arrayContaining([
|
||||
'.github/review-prompt.md',
|
||||
'.github/workflows/ai-review.yml',
|
||||
'scripts/github/normalize-ai-review-output.mjs',
|
||||
])
|
||||
);
|
||||
expect(scope.lowSignal.map((file: { filename: string }) => file.filename)).toEqual([
|
||||
'README.md',
|
||||
'docs/ai-review.md',
|
||||
]);
|
||||
expect(scope.reviewableFiles).toBe(3);
|
||||
});
|
||||
|
||||
test('falls back to low-signal files when they are the only changed files', () => {
|
||||
const scope = reviewScope.buildReviewScope(
|
||||
reviewScope.normalizePullFiles([
|
||||
{
|
||||
filename: 'README.md',
|
||||
status: 'modified',
|
||||
additions: 20,
|
||||
deletions: 3,
|
||||
changes: 23,
|
||||
patch: '@@ -1 +1 @@\n-old\n+new',
|
||||
},
|
||||
]),
|
||||
'fast'
|
||||
);
|
||||
|
||||
expect(scope.selected).toHaveLength(1);
|
||||
expect(scope.selected[0].filename).toBe('README.md');
|
||||
expect(scope.reviewableFiles).toBe(1);
|
||||
expect(scope.scopeLabel).toBe('changed files');
|
||||
});
|
||||
|
||||
test('keeps broad triage coverage for xlarge PRs when the review packet can still fit', () => {
|
||||
const scope = reviewScope.buildReviewScope(
|
||||
reviewScope.normalizePullFiles([
|
||||
{
|
||||
filename: '.github/workflows/ai-review.yml',
|
||||
status: 'modified',
|
||||
additions: 130,
|
||||
deletions: 30,
|
||||
changes: 160,
|
||||
patch: '@@ -1 +1 @@\n-old\n+new',
|
||||
},
|
||||
{
|
||||
filename: 'scripts/github/prepare-ai-review-scope.mjs',
|
||||
status: 'modified',
|
||||
additions: 120,
|
||||
deletions: 20,
|
||||
changes: 140,
|
||||
patch: '@@ -1 +1 @@\n-old\n+new',
|
||||
},
|
||||
{
|
||||
filename: 'src/commands/help-command.ts',
|
||||
status: 'modified',
|
||||
additions: 70,
|
||||
deletions: 10,
|
||||
changes: 80,
|
||||
patch: '@@ -1 +1 @@\n-old\n+new',
|
||||
},
|
||||
{
|
||||
filename: 'src/ccs.ts',
|
||||
status: 'modified',
|
||||
additions: 50,
|
||||
deletions: 15,
|
||||
changes: 65,
|
||||
patch: '@@ -1 +1 @@\n-old\n+new',
|
||||
},
|
||||
{
|
||||
filename: 'tests/unit/commands/help-command.test.ts',
|
||||
status: 'modified',
|
||||
additions: 40,
|
||||
deletions: 5,
|
||||
changes: 45,
|
||||
patch: '@@ -1 +1 @@\n-old\n+new',
|
||||
},
|
||||
]),
|
||||
'triage',
|
||||
{ sizeClass: 'xlarge' }
|
||||
);
|
||||
|
||||
expect(scope.selected.length).toBe(5);
|
||||
expect(scope.selected.map((file: { filename: string }) => file.filename)).toEqual(
|
||||
expect.arrayContaining([
|
||||
'.github/workflows/ai-review.yml',
|
||||
'scripts/github/prepare-ai-review-scope.mjs',
|
||||
])
|
||||
);
|
||||
expect(scope.selectedChanges).toBe(490);
|
||||
expect(scope.limits).toEqual({
|
||||
maxFiles: 24,
|
||||
maxChangedLines: 2400,
|
||||
maxPatchLines: 140,
|
||||
maxPatchChars: 12000,
|
||||
});
|
||||
});
|
||||
|
||||
test('renders deterministic scope metadata and fences patch content safely', () => {
|
||||
const oversizedPatch = ['+line 1', '```', ...Array.from({ length: 118 }, (_, index) => `+line ${index + 2}`)].join('\n');
|
||||
const scope = reviewScope.buildReviewScope(
|
||||
reviewScope.normalizePullFiles([
|
||||
{
|
||||
filename: '.github/workflows/ai-review.yml',
|
||||
status: 'modified',
|
||||
additions: 120,
|
||||
deletions: 0,
|
||||
changes: 120,
|
||||
patch: oversizedPatch,
|
||||
},
|
||||
]),
|
||||
'triage'
|
||||
);
|
||||
|
||||
const markdown = reviewScope.renderReviewScope({
|
||||
prNumber: 880,
|
||||
baseRef: 'dev',
|
||||
turnBudget: 6,
|
||||
timeoutMinutes: 5,
|
||||
scope,
|
||||
});
|
||||
|
||||
expect(markdown).toContain('# AI Review Scope');
|
||||
expect(markdown).toContain('- Mode: `triage` (expanded packaged review with broader coverage)');
|
||||
expect(markdown).toContain('- Selected files: 1 of 1 reviewable files (1 total changed files)');
|
||||
expect(markdown).toContain('- Turn budget: 6');
|
||||
expect(markdown).toContain('- Workflow cap: 5 minutes');
|
||||
expect(markdown).toContain('````diff');
|
||||
expect(markdown).toContain('```');
|
||||
expect(markdown).not.toContain('... patch trimmed for bounded review ...');
|
||||
});
|
||||
});
|
||||
@@ -1,230 +0,0 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
|
||||
const directReview = await import('../../../../scripts/github/run-ai-review-direct.mjs');
|
||||
|
||||
function withTempDir(prefix: string, run: (tempDir: string) => Promise<void> | void) {
|
||||
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
|
||||
return Promise.resolve()
|
||||
.then(() => run(tempDir))
|
||||
.finally(() => {
|
||||
fs.rmSync(tempDir, { recursive: true, force: true });
|
||||
});
|
||||
}
|
||||
|
||||
function createResponse(text: string) {
|
||||
return {
|
||||
ok: true,
|
||||
async json() {
|
||||
return {
|
||||
content: [{ type: 'text', text }],
|
||||
};
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe('run-ai-review-direct', () => {
|
||||
test('reserves the tail of the step budget for deterministic fallback publication', () => {
|
||||
const window = directReview.resolveAttemptWindow({
|
||||
timeoutMinutes: 8,
|
||||
configuredTimeoutMs: 240000,
|
||||
requestBufferMs: 45000,
|
||||
minAttemptMs: 20000,
|
||||
startedAt: 0,
|
||||
now: 420001,
|
||||
});
|
||||
|
||||
expect(window.canAttempt).toBe(false);
|
||||
expect(window.timeoutMs).toBeNull();
|
||||
});
|
||||
|
||||
test('extracts json candidates from fenced or chatty model replies', () => {
|
||||
expect(directReview.extractJsonCandidate('```json\n{"ok":true}\n```')).toBe('{"ok":true}');
|
||||
expect(directReview.extractJsonCandidate('Here is the result:\n{"ok":true}\nThanks')).toBe(
|
||||
'{"ok":true}'
|
||||
);
|
||||
});
|
||||
|
||||
test('uses the included-manifest files for fallback coverage instead of assuming a prefix slice', async () => {
|
||||
await withTempDir('ai-review-direct-', async (tempDir) => {
|
||||
const outputFile = path.join(tempDir, 'review.md');
|
||||
const logFile = path.join(tempDir, 'attempts.json');
|
||||
const packetFile = path.join(tempDir, 'packet.md');
|
||||
const manifestFile = path.join(tempDir, 'selected-files.txt');
|
||||
const includedManifestFile = path.join(tempDir, 'included-files.txt');
|
||||
fs.writeFileSync(packetFile, '# AI Review Packet\n\npacket body\n');
|
||||
fs.writeFileSync(manifestFile, 'src/large.ts\nsrc/small.ts\n');
|
||||
fs.writeFileSync(includedManifestFile, 'src/small.ts\n');
|
||||
|
||||
const result = await directReview.writeDirectReviewFromEnv(
|
||||
{
|
||||
ANTHROPIC_BASE_URL: 'https://api.z.ai/api/anthropic',
|
||||
ANTHROPIC_AUTH_TOKEN: 'test-token',
|
||||
REVIEW_MODEL: 'glm-5-turbo',
|
||||
GITHUB_REPOSITORY: 'kaitranntt/ccs',
|
||||
AI_REVIEW_PROMPT: 'You are a reviewer.',
|
||||
AI_REVIEW_PACKET_FILE: packetFile,
|
||||
AI_REVIEW_SCOPE_MANIFEST_FILE: manifestFile,
|
||||
AI_REVIEW_PACKET_INCLUDED_MANIFEST_FILE: includedManifestFile,
|
||||
AI_REVIEW_OUTPUT_FILE: outputFile,
|
||||
AI_REVIEW_LOG_FILE: logFile,
|
||||
AI_REVIEW_RUN_URL: 'https://github.com/kaitranntt/ccs/actions/runs/1',
|
||||
AI_REVIEW_MODE: 'triage',
|
||||
AI_REVIEW_SELECTED_FILES: '2',
|
||||
AI_REVIEW_REVIEWABLE_FILES: '4',
|
||||
AI_REVIEW_SELECTED_CHANGES: '120',
|
||||
AI_REVIEW_REVIEWABLE_CHANGES: '220',
|
||||
AI_REVIEW_SCOPE_LABEL: 'reviewable files',
|
||||
AI_REVIEW_PACKET_INCLUDED_FILES: '1',
|
||||
AI_REVIEW_PACKET_TOTAL_FILES: '2',
|
||||
AI_REVIEW_PACKET_OMITTED_FILES: '1',
|
||||
AI_REVIEW_TIMEOUT_MINUTES: '8',
|
||||
AI_REVIEW_REQUEST_TIMEOUT_MS: '50',
|
||||
AI_REVIEW_MAX_ATTEMPTS: '1',
|
||||
AI_REVIEW_PR_NUMBER: '888',
|
||||
},
|
||||
async () => {
|
||||
throw new Error('forced direct review failure');
|
||||
}
|
||||
);
|
||||
|
||||
expect(result.usedFallback).toBe(true);
|
||||
const markdown = fs.readFileSync(outputFile, 'utf8');
|
||||
expect(markdown).toContain('`src/small.ts`');
|
||||
expect(markdown).not.toContain('`src/large.ts`');
|
||||
});
|
||||
});
|
||||
|
||||
test('writes the legacy long-form review markdown when the first response validates', async () => {
|
||||
await withTempDir('ai-review-direct-', async (tempDir) => {
|
||||
const outputFile = path.join(tempDir, 'review.md');
|
||||
const logFile = path.join(tempDir, 'attempts.json');
|
||||
const packetFile = path.join(tempDir, 'packet.md');
|
||||
const manifestFile = path.join(tempDir, 'selected-files.txt');
|
||||
const includedManifestFile = path.join(tempDir, 'included-files.txt');
|
||||
fs.writeFileSync(packetFile, '# AI Review Packet\n\npacket body\n');
|
||||
fs.writeFileSync(manifestFile, 'src/example.ts\n');
|
||||
fs.writeFileSync(includedManifestFile, 'src/example.ts\n');
|
||||
|
||||
const result = await directReview.writeDirectReviewFromEnv(
|
||||
{
|
||||
ANTHROPIC_BASE_URL: 'https://api.z.ai/api/anthropic',
|
||||
ANTHROPIC_AUTH_TOKEN: 'test-token',
|
||||
REVIEW_MODEL: 'glm-5-turbo',
|
||||
GITHUB_REPOSITORY: 'kaitranntt/ccs',
|
||||
AI_REVIEW_PROMPT: 'You are a reviewer.',
|
||||
AI_REVIEW_PACKET_FILE: packetFile,
|
||||
AI_REVIEW_SCOPE_MANIFEST_FILE: manifestFile,
|
||||
AI_REVIEW_PACKET_INCLUDED_MANIFEST_FILE: includedManifestFile,
|
||||
AI_REVIEW_OUTPUT_FILE: outputFile,
|
||||
AI_REVIEW_LOG_FILE: logFile,
|
||||
AI_REVIEW_RUN_URL: 'https://github.com/kaitranntt/ccs/actions/runs/1',
|
||||
AI_REVIEW_MODE: 'fast',
|
||||
AI_REVIEW_SELECTED_FILES: '1',
|
||||
AI_REVIEW_REVIEWABLE_FILES: '1',
|
||||
AI_REVIEW_SELECTED_CHANGES: '18',
|
||||
AI_REVIEW_REVIEWABLE_CHANGES: '18',
|
||||
AI_REVIEW_SCOPE_LABEL: 'reviewable files',
|
||||
AI_REVIEW_PACKET_INCLUDED_FILES: '1',
|
||||
AI_REVIEW_PACKET_TOTAL_FILES: '1',
|
||||
AI_REVIEW_PACKET_OMITTED_FILES: '0',
|
||||
AI_REVIEW_TIMEOUT_MINUTES: '8',
|
||||
AI_REVIEW_PR_NUMBER: '888',
|
||||
},
|
||||
async () =>
|
||||
createResponse(
|
||||
JSON.stringify({
|
||||
summary: 'The PR looks correct.',
|
||||
findings: [],
|
||||
securityChecklist: [{ check: 'Injection safety', status: 'pass', notes: 'Covered.' }],
|
||||
ccsCompliance: [
|
||||
{ rule: 'ASCII-only CLI output', status: 'na', notes: 'No CLI changes.' },
|
||||
],
|
||||
informational: ['The packet covered the selected file.'],
|
||||
strengths: ['The response validated on the first attempt.'],
|
||||
overallAssessment: 'approved',
|
||||
overallRationale: 'No confirmed regressions remain.',
|
||||
})
|
||||
)
|
||||
);
|
||||
|
||||
expect(result.usedFallback).toBe(false);
|
||||
const markdown = fs.readFileSync(outputFile, 'utf8');
|
||||
expect(markdown).toContain('### 📋 Summary');
|
||||
expect(markdown).toContain('### 🔍 Findings');
|
||||
expect(markdown).toContain('**✅ APPROVED**');
|
||||
expect(markdown).toContain('> 🧭 `fast` • 1/1 files • 18/18 lines • packet 1/1 • 8 minutes');
|
||||
expect(JSON.parse(fs.readFileSync(logFile, 'utf8')).attempts).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
test('retries with a repair attempt when the first response is invalid', async () => {
|
||||
await withTempDir('ai-review-direct-', async (tempDir) => {
|
||||
const outputFile = path.join(tempDir, 'review.md');
|
||||
const logFile = path.join(tempDir, 'attempts.json');
|
||||
const packetFile = path.join(tempDir, 'packet.md');
|
||||
const manifestFile = path.join(tempDir, 'selected-files.txt');
|
||||
const includedManifestFile = path.join(tempDir, 'included-files.txt');
|
||||
fs.writeFileSync(packetFile, '# AI Review Packet\n\npacket body\n');
|
||||
fs.writeFileSync(manifestFile, 'src/example.ts\n');
|
||||
fs.writeFileSync(includedManifestFile, 'src/example.ts\n');
|
||||
|
||||
const responses = [
|
||||
createResponse('{"summary":"missing required fields"}'),
|
||||
createResponse(
|
||||
JSON.stringify({
|
||||
summary: 'The PR needs a small follow-up only.',
|
||||
findings: [],
|
||||
securityChecklist: [{ check: 'Injection safety', status: 'pass', notes: 'Covered.' }],
|
||||
ccsCompliance: [
|
||||
{ rule: 'ASCII-only CLI output', status: 'na', notes: 'No CLI changes.' },
|
||||
],
|
||||
informational: [],
|
||||
strengths: ['The repair path returned valid JSON.'],
|
||||
overallAssessment: 'approved_with_notes',
|
||||
overallRationale: 'No blocking issues remain after the repair pass.',
|
||||
})
|
||||
),
|
||||
];
|
||||
|
||||
const result = await directReview.writeDirectReviewFromEnv(
|
||||
{
|
||||
ANTHROPIC_BASE_URL: 'https://api.z.ai/api/anthropic',
|
||||
ANTHROPIC_AUTH_TOKEN: 'test-token',
|
||||
REVIEW_MODEL: 'glm-5-turbo',
|
||||
GITHUB_REPOSITORY: 'kaitranntt/ccs',
|
||||
AI_REVIEW_PROMPT: 'You are a reviewer.',
|
||||
AI_REVIEW_PACKET_FILE: packetFile,
|
||||
AI_REVIEW_SCOPE_MANIFEST_FILE: manifestFile,
|
||||
AI_REVIEW_PACKET_INCLUDED_MANIFEST_FILE: includedManifestFile,
|
||||
AI_REVIEW_OUTPUT_FILE: outputFile,
|
||||
AI_REVIEW_LOG_FILE: logFile,
|
||||
AI_REVIEW_RUN_URL: 'https://github.com/kaitranntt/ccs/actions/runs/1',
|
||||
AI_REVIEW_MODE: 'triage',
|
||||
AI_REVIEW_SELECTED_FILES: '3',
|
||||
AI_REVIEW_REVIEWABLE_FILES: '5',
|
||||
AI_REVIEW_SELECTED_CHANGES: '140',
|
||||
AI_REVIEW_REVIEWABLE_CHANGES: '180',
|
||||
AI_REVIEW_SCOPE_LABEL: 'reviewable files',
|
||||
AI_REVIEW_PACKET_INCLUDED_FILES: '2',
|
||||
AI_REVIEW_PACKET_TOTAL_FILES: '3',
|
||||
AI_REVIEW_PACKET_OMITTED_FILES: '1',
|
||||
AI_REVIEW_TIMEOUT_MINUTES: '10',
|
||||
AI_REVIEW_PR_NUMBER: '888',
|
||||
},
|
||||
async () => responses.shift() as ReturnType<typeof createResponse>
|
||||
);
|
||||
|
||||
expect(result.usedFallback).toBe(false);
|
||||
const markdown = fs.readFileSync(outputFile, 'utf8');
|
||||
expect(markdown).toContain('### 🎯 Overall Assessment');
|
||||
expect(markdown).toContain('**⚠️ APPROVED WITH NOTES**');
|
||||
expect(markdown).toContain(
|
||||
'> 🧭 `triage` • 3/5 files • 140/180 lines • packet 2/3 • 10 minutes'
|
||||
);
|
||||
expect(JSON.parse(fs.readFileSync(logFile, 'utf8')).attempts).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user