diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1276e45..39c294c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,14 +31,14 @@ jobs: run: | node --check bin/rewrite-release-notes.mjs node --check bin/build-updater-manifest.mjs - node --check bin/review-code.mjs - node --check lib/code-review.mjs + node --check bin/apply-branch-protection.mjs + node --check lib/branch-protection.mjs - name: Help works run: | node bin/rewrite-release-notes.mjs --help node bin/build-updater-manifest.mjs --help - node bin/review-code.mjs --help + node bin/apply-branch-protection.mjs --help # Smoke: dry-run renders without an LLM call. Uses the repo's own tags # if any exist; otherwise creates synthetic ones so the script has @@ -87,5 +87,5 @@ jobs: console.log("manifest smoke ok"); ' - - name: Test reusable review tool + - name: Test run: npm test diff --git a/README.md b/README.md index b08e60d..ac8dc62 100644 --- a/README.md +++ b/README.md @@ -185,57 +185,6 @@ and writes a manifest in the exact shape Tauri's updater expects. Exits non-zero if any binary is missing its signature, so the `publish` job fails loudly when signing didn't run. -## Model-backed code review - -`review-code` is a reusable, Clawpatch-inspired review harness for protoLabs -repos. It maps a repository into bounded feature records, asks an -OpenAI-compatible gateway for strict JSON findings, persists those findings -locally, and emits a Markdown report. It does not edit code. - -```bash -# In the repo you want reviewed: -npx -p @protolabsai/release-tools review-code init -npx -p @protolabsai/release-tools review-code map -GATEWAY_API_KEY=... OPENAI_BASE_URL=https://api.proto-labs.ai/v1 \ - npx -p @protolabsai/release-tools review-code run --model protolabs/fast --limit 1 -npx -p @protolabsai/release-tools review-code report -``` - -Review state is written to `.release-tools-review/`. Add that directory to a -repo's `.gitignore` if you use the tool regularly. The default model is -`protolabs/smart`; override it with `--model` or `CODE_REVIEW_MODEL` when a -gateway key is routed to another review model such as Minimax. - -For better feature boundaries, add a repo-local `review-code.config.json`: - -```json -{ - "features": [ - { - "feature_id": "engine_core", - "name": "Engine core", - "description": "Deterministic resolver and replay-critical state.", - "owned_globs": ["src/engine/**/*.js"], - "context_globs": ["docs/engine/**/*.md"], - "test_globs": ["test/engine/**/*.test.js"] - } - ] -} -``` - -Useful commands: - -```bash -review-code status -review-code run --feature-id engine_core -review-code run --all -review-code report --output ./review-report.md -``` - -The harness keeps prompts bounded, skips external symlinks, rejects findings -outside the reviewed file allowlist, and preserves existing triage fields when -a finding is regenerated. - ## Branch protection defaults `apply-branch-protection` applies the protoLabs recommended branch protection @@ -273,7 +222,6 @@ reference. npm install node bin/rewrite-release-notes.mjs --help node bin/build-updater-manifest.mjs --help -node bin/review-code.mjs --help node bin/apply-branch-protection.mjs --help npm test ``` diff --git a/bin/review-code.mjs b/bin/review-code.mjs deleted file mode 100755 index effa5e0..0000000 --- a/bin/review-code.mjs +++ /dev/null @@ -1,173 +0,0 @@ -#!/usr/bin/env node -/** - * @license - * Copyright 2026 protoLabs - * SPDX-License-Identifier: Apache-2.0 - * - * Runs a bounded model-backed code review loop for any repo. - * - * Usage: - * review-code init [flags] - * review-code map [flags] - * review-code run [flags] - * review-code status [flags] - * review-code report [flags] - * - * Flags: - * --repo-root Repository to review. Default: current directory. - * --state-dir Local review state. Default: .release-tools-review - * --config Optional review-code config JSON with feature specs. - * --model Gateway model alias. Default: CODE_REVIEW_MODEL or protolabs/smart. - * --feature-id Review one mapped feature. - * --limit Review first n mapped features. Default for run: 1. - * --all Review every mapped feature. - * --output Markdown report path for report command. - * --help Show this help. - * - * Environment: - * GATEWAY_API_KEY or OPENAI_API_KEY Bearer token for the gateway. - * OPENAI_BASE_URL Default: https://api.proto-labs.ai/v1 - * CODE_REVIEW_MODEL Default model override. - */ - -import path from 'node:path'; - -import { - CodeReviewError, - DEFAULT_REVIEW_MODEL, - DEFAULT_REVIEW_STATE_DIR, - buildReviewReport, - initReviewState, - mapReviewFeatures, - reviewMappedFeatures, - reviewStatus, -} from '../lib/code-review.mjs'; - -if (process.argv.includes('--help') || process.argv.includes('-h')) { - const fs = await import('node:fs'); - const url = await import('node:url'); - const self = url.fileURLToPath(import.meta.url); - const src = fs.readFileSync(self, 'utf8').split('\n'); - const start = src.findIndex((line) => line.startsWith(' * Runs')); - const end = src.findIndex((line, index) => index > start && line.startsWith(' */')); - const help = src - .slice(start, end) - .map((line) => line.replace(/^ \* ?/, '')) - .join('\n'); - console.log(help); - process.exit(0); -} - -const { command, flags } = parseArgs(process.argv.slice(2)); -if (!command) { - console.error('Missing command. Use --help for usage.'); - process.exit(1); -} - -const repoRoot = flags['repo-root'] ?? process.cwd(); -const stateDir = flags['state-dir'] ?? DEFAULT_REVIEW_STATE_DIR; -const configPath = flags.config ?? null; -const model = - flags.model ?? process.env.CODE_REVIEW_MODEL ?? DEFAULT_REVIEW_MODEL; - -try { - if (command === 'init') { - const result = initReviewState({ repoRoot, stateDir, model, configPath }); - console.log(`review_state=${result.state_dir}`); - console.log(`model=${result.config.provider.model}`); - process.exit(0); - } - - if (command === 'map') { - const result = mapReviewFeatures({ repoRoot, stateDir, configPath }); - console.log(`features=${result.features.length}`); - console.log(`path=${resultPath(repoRoot, stateDir, 'features.json')}`); - process.exit(0); - } - - if (command === 'run') { - if (!process.env.GATEWAY_API_KEY && !process.env.OPENAI_API_KEY) { - throw new CodeReviewError('GATEWAY_API_KEY or OPENAI_API_KEY is required'); - } - const limit = runLimit(flags); - const result = await reviewMappedFeatures({ - repoRoot, - stateDir, - model, - limit, - featureId: flags['feature-id'] ?? null, - configPath, - }); - console.log( - `reviewed=${result.reviewed.length} findings=${result.finding_count} ` + - `rejected=${result.rejected_count} model=${result.model}`, - ); - process.exit(0); - } - - if (command === 'status') { - const result = reviewStatus({ repoRoot, stateDir }); - console.log( - `features=${result.feature_count} findings=${result.finding_count} ` + - `statuses=${JSON.stringify(result.status_counts)}`, - ); - process.exit(0); - } - - if (command === 'report') { - const result = buildReviewReport({ - repoRoot, - stateDir, - output: flags.output ?? null, - }); - console.log(`report=${result.path} findings=${result.finding_count}`); - process.exit(0); - } - - console.error(`Unknown command: ${command}`); - process.exit(1); -} catch (error) { - console.error(error instanceof Error ? error.message : String(error)); - process.exit(1); -} - -function parseArgs(args) { - const positional = []; - const flags = {}; - for (let index = 0; index < args.length; index += 1) { - const arg = args[index]; - if (!arg.startsWith('--')) { - positional.push(arg); - continue; - } - const key = arg.slice(2); - if (key === 'all') { - flags[key] = true; - continue; - } - const value = args[index + 1]; - if (!value || value.startsWith('--')) { - throw new CodeReviewError(`Missing value for --${key}`); - } - flags[key] = value; - index += 1; - } - return { command: positional[0], flags }; -} - -function runLimit(flags) { - if (flags.all) return null; - const raw = flags.limit ?? '1'; - const parsed = Number(raw); - if (!Number.isInteger(parsed) || parsed <= 0) { - throw new CodeReviewError('Invalid --limit: must be a positive integer'); - } - return parsed; -} - -function resultPath(repoRoot, stateDir, fileName) { - return path.join( - path.isAbsolute(stateDir) ? stateDir : path.join(repoRoot, stateDir), - fileName, - ); -} diff --git a/docs/handoffs/2026-05-21-review-code.md b/docs/handoffs/2026-05-21-review-code.md deleted file mode 100644 index 24ae779..0000000 --- a/docs/handoffs/2026-05-21-review-code.md +++ /dev/null @@ -1,74 +0,0 @@ -# Handoff: `review-code` v1.1.0 - -Date: 2026-05-21 - -## What Landed - -- Moved the model-backed review harness out of `mythxengine2.0` and into `protoLabsAI/release-tools`. -- Closed MythX PR #92 as superseded by shared tooling. -- Merged release-tools PR #4: https://github.com/protoLabsAI/release-tools/pull/4 -- Added the reusable `review-code` CLI: - - `review-code init` - - `review-code map` - - `review-code run` - - `review-code status` - - `review-code report` -- Added `lib/code-review.mjs` with bounded feature mapping, strict JSON review calls, persisted findings, triage preservation, allowlisted locations, symlink-safe repo scanning, and Markdown reports. -- Added tests, CI syntax checks, smoke coverage, and an ESLint flat config. -- Created GitHub release `v1.1.0`: https://github.com/protoLabsAI/release-tools/releases/tag/v1.1.0 - -## Validation - -Local validation passed: - -```bash -node --check bin/rewrite-release-notes.mjs -node --check bin/build-updater-manifest.mjs -node --check bin/review-code.mjs -node --check lib/code-review.mjs -npm test -npm run smoke -npm run lint -``` - -Live gateway smoke passed with `protolabs/fast`: - -```bash -review-code init -review-code map -review-code run --model protolabs/fast --limit 1 -review-code report -``` - -## Current Usage - -npm still serves `@protolabsai/release-tools@1.0.0`, so use the GitHub tag until npm publish is fixed: - -```bash -npx --yes -p github:protoLabsAI/release-tools#v1.1.0 review-code --help -``` - -Once npm publish is fixed, the intended command is: - -```bash -npx --yes -p @protolabsai/release-tools review-code --help -``` - -## Open Items - -1. Fix npm publishing for `@protolabsai/release-tools@1.1.0`. - - The release workflow created tag `v1.1.0`. - - GitHub release exists. - - npm publish failed with registry 404 / permission-style output. - - `npm view @protolabsai/release-tools version` still returns `1.0.0`. - - Likely cause: `NPM_TOKEN` scope/package permission for the `@protolabsai` org package. -2. After token/scope access is fixed, rerun the release workflow or publish `1.1.0`. -3. Minimax is supported through `--model` / `CODE_REVIEW_MODEL`, but the current gateway key rejects likely Minimax aliases as not allowed. - - Need gateway model allowlist update before using Minimax for review runs. - -## Notes - -- `review-code` is intentionally report-only. It does not patch code. -- Findings are persisted under `.release-tools-review/`. -- Existing triage fields are preserved when findings regenerate. -- Repo-local feature boundaries can be supplied with `review-code.config.json`. diff --git a/lib/code-review.mjs b/lib/code-review.mjs deleted file mode 100644 index f7faaab..0000000 --- a/lib/code-review.mjs +++ /dev/null @@ -1,832 +0,0 @@ -/** - * @license - * Copyright 2026 protoLabs - * SPDX-License-Identifier: Apache-2.0 - */ - -import { execFileSync } from 'node:child_process'; -import { createHash } from 'node:crypto'; -import fs from 'node:fs'; -import path from 'node:path'; - -export const REVIEW_SCHEMA_VERSION = '0.1.0'; -export const DEFAULT_REVIEW_STATE_DIR = '.release-tools-review'; -export const DEFAULT_REVIEW_MODEL = 'protolabs/smart'; -export const DEFAULT_REVIEW_BASE_URL = 'https://api.proto-labs.ai/v1'; -export const MAX_REVIEW_CONTEXT_CHARS = 12000; -export const MAX_REVIEW_FILE_CHARS = 4000; - -const DEFAULT_FEATURE_SPECS = [ - { - feature_id: 'release_package', - name: 'Release package metadata', - description: 'Package metadata, README usage, actions, and workflow wiring.', - owned_globs: [ - 'package.json', - 'README.md', - 'action.yml', - '.github/workflows/*.yml', - '.github/workflows/*.yaml', - ], - context_globs: [], - test_globs: ['test/**/*.mjs', 'test/**/*.js', 'tests/**/*.mjs', 'tests/**/*.js'], - }, - { - feature_id: 'release_tools_cli', - name: 'Release tools CLI', - description: 'Executable CLI entrypoints and reusable release tooling logic.', - owned_globs: ['bin/*.mjs', 'lib/**/*.mjs', 'lib/*.mjs'], - context_globs: ['README.md', 'package.json'], - test_globs: ['test/**/*.mjs', 'test/**/*.js', 'tests/**/*.mjs', 'tests/**/*.js'], - }, - { - feature_id: 'tests', - name: 'Tests and smoke coverage', - description: 'Node tests, smoke scripts, and CI validation paths.', - owned_globs: ['test/**/*.mjs', 'test/**/*.js', 'tests/**/*.mjs', 'tests/**/*.js'], - context_globs: ['package.json', '.github/workflows/*.yml', '.github/workflows/*.yaml'], - test_globs: [], - }, -]; - -export class CodeReviewError extends Error { - constructor(message) { - super(message); - this.name = 'CodeReviewError'; - } -} - -export function initReviewState({ - repoRoot = process.cwd(), - stateDir = DEFAULT_REVIEW_STATE_DIR, - model = DEFAULT_REVIEW_MODEL, - configPath = null, -} = {}) { - const root = path.resolve(repoRoot); - const state = resolveStateDir(root, stateDir); - fs.mkdirSync(state, { recursive: true }); - const config = { - schema_version: REVIEW_SCHEMA_VERSION, - state_dir: state, - provider: { - name: 'openai-compatible', - model, - env: { - api_key: 'GATEWAY_API_KEY or OPENAI_API_KEY', - base_url: 'OPENAI_BASE_URL', - model: 'CODE_REVIEW_MODEL', - }, - }, - review: { - max_context_chars: MAX_REVIEW_CONTEXT_CHARS, - max_file_chars: MAX_REVIEW_FILE_CHARS, - max_findings_per_feature: 8, - }, - config_path: configPath, - feature_specs: loadFeatureSpecs(root, configPath), - }; - const project = { - schema_version: REVIEW_SCHEMA_VERSION, - project: path.basename(root), - kind: 'code-review-target', - root, - git_head: gitHead(root), - generated_at: utcNow(), - }; - writeJson(path.join(state, 'config.json'), config); - writeJson(path.join(state, 'project.json'), project); - return { state_dir: state, config, project }; -} - -export function mapReviewFeatures({ - repoRoot = process.cwd(), - stateDir = DEFAULT_REVIEW_STATE_DIR, - configPath = null, -} = {}) { - const root = path.resolve(repoRoot); - const state = resolveStateDir(root, stateDir); - const allFiles = repoFiles(root, state); - const featureSpecs = loadFeatureSpecs(root, configPath, state); - const features = featureSpecs.map((spec) => featureRecord(root, allFiles, spec)); - const payload = { - schema_version: REVIEW_SCHEMA_VERSION, - generated_at: utcNow(), - git_head: gitHead(root), - features, - }; - fs.mkdirSync(state, { recursive: true }); - writeJson(path.join(state, 'features.json'), payload); - return payload; -} - -export async function reviewMappedFeatures({ - repoRoot = process.cwd(), - stateDir = DEFAULT_REVIEW_STATE_DIR, - model = DEFAULT_REVIEW_MODEL, - limit = null, - featureId = null, - configPath = null, - client = null, -} = {}) { - const root = path.resolve(repoRoot); - const state = resolveStateDir(root, stateDir); - let features = [...loadOrMapFeatures(root, state, configPath).features]; - if (featureId) { - features = features.filter((feature) => feature.feature_id === featureId); - if (features.length === 0) { - throw new CodeReviewError(`Unknown feature_id: ${featureId}`); - } - } - if (limit !== null && limit !== undefined) { - const parsedLimit = Number(limit); - if (!Number.isInteger(parsedLimit) || parsedLimit <= 0) { - throw new CodeReviewError('Invalid --limit: must be a positive integer'); - } - features = features.slice(0, parsedLimit); - } - if (features.length === 0) { - throw new CodeReviewError('No feature records selected for review'); - } - - const provider = client ?? { - createStructuredOutput: (request) => callStructuredReview(request), - }; - const findingsDir = path.join(state, 'findings'); - fs.mkdirSync(findingsDir, { recursive: true }); - const acceptedFindings = []; - const reviewed = []; - let rejectedCount = 0; - - for (const feature of features) { - const response = await provider.createStructuredOutput({ - model, - systemPrompt: reviewSystemPrompt(), - userPrompt: reviewPrompt(root, feature), - schema: reviewOutputSchema(), - }); - const { findings, rejected } = normalizeFindings(root, feature, response); - rejectedCount += rejected; - for (const finding of findings) { - const findingPath = path.join(findingsDir, `${finding.finding_id}.json`); - const existing = loadJson(findingPath, null); - const merged = preserveTriage(existing, finding); - writeJson(findingPath, merged); - acceptedFindings.push(merged); - } - reviewed.push({ - feature_id: feature.feature_id, - finding_count: findings.length, - rejected_count: rejected, - }); - } - - const run = { - schema_version: REVIEW_SCHEMA_VERSION, - generated_at: utcNow(), - git_head: gitHead(root), - model, - reviewed, - finding_count: acceptedFindings.length, - rejected_count: rejectedCount, - findings: acceptedFindings.map((finding) => finding.finding_id), - }; - const runsDir = path.join(state, 'runs'); - fs.mkdirSync(runsDir, { recursive: true }); - writeJson(path.join(runsDir, `review_${timestampId()}.json`), run); - return run; -} - -export function reviewStatus({ - repoRoot = process.cwd(), - stateDir = DEFAULT_REVIEW_STATE_DIR, -} = {}) { - const root = path.resolve(repoRoot); - const state = resolveStateDir(root, stateDir); - const featuresPayload = loadJson(path.join(state, 'features.json'), {}); - const findings = loadReviewFindings({ stateDir: state }); - const statusCounts = {}; - const severityCounts = {}; - for (const finding of findings) { - const status = String(finding.status ?? 'open'); - const severity = String(finding.severity ?? 'low'); - statusCounts[status] = (statusCounts[status] ?? 0) + 1; - severityCounts[severity] = (severityCounts[severity] ?? 0) + 1; - } - return { - state_dir: state, - git_head: gitHead(root), - feature_count: Array.isArray(featuresPayload.features) - ? featuresPayload.features.length - : 0, - finding_count: findings.length, - status_counts: statusCounts, - severity_counts: severityCounts, - }; -} - -export function loadReviewFindings({ stateDir = DEFAULT_REVIEW_STATE_DIR } = {}) { - const findingsDir = path.join(path.resolve(stateDir), 'findings'); - if (!fs.existsSync(findingsDir)) return []; - const findings = []; - for (const name of fs.readdirSync(findingsDir).sort()) { - if (!name.endsWith('.json')) continue; - const payload = loadJson(path.join(findingsDir, name), null); - if (payload && typeof payload === 'object' && payload.finding_id) { - findings.push(payload); - } - } - return findings.sort((a, b) => { - const left = [ - severityRank(a.severity), - String(a.feature_id ?? ''), - String(a.path ?? ''), - Number(a.line ?? 0), - ]; - const right = [ - severityRank(b.severity), - String(b.feature_id ?? ''), - String(b.path ?? ''), - Number(b.line ?? 0), - ]; - return JSON.stringify(left).localeCompare(JSON.stringify(right)); - }); -} - -export function buildReviewReport({ - repoRoot = process.cwd(), - stateDir = DEFAULT_REVIEW_STATE_DIR, - output = null, -} = {}) { - const root = path.resolve(repoRoot); - const state = resolveStateDir(root, stateDir); - const reportPath = output - ? path.resolve(root, output) - : path.join(state, 'report.md'); - const status = reviewStatus({ repoRoot: root, stateDir: state }); - const findings = loadReviewFindings({ stateDir: state }); - const lines = [ - '# protoLabs Code Review Report', - '', - `- Generated: \`${utcNow()}\``, - `- Git head: \`${status.git_head}\``, - `- Features mapped: \`${status.feature_count}\``, - `- Findings: \`${status.finding_count}\``, - '', - ]; - if (findings.length === 0) { - lines.push('No findings recorded.', ''); - } else { - lines.push('## Findings', ''); - for (const finding of findings) { - lines.push( - `### ${String(finding.severity).toUpperCase()} - ${finding.title}`, - '', - `- ID: \`${finding.finding_id}\``, - `- Status: \`${finding.status ?? 'open'}\``, - `- Feature: \`${finding.feature_id ?? ''}\``, - `- Category: \`${finding.category ?? ''}\``, - `- Confidence: \`${finding.confidence ?? ''}\``, - `- Location: \`${finding.path ?? ''}:${finding.line ?? ''}\``, - '', - '**Evidence**', - '', - String(finding.evidence ?? '').trim(), - '', - '**Recommendation**', - '', - String(finding.recommendation ?? '').trim(), - '', - ); - } - } - fs.mkdirSync(path.dirname(reportPath), { recursive: true }); - fs.writeFileSync(reportPath, `${lines.join('\n')}\n`, 'utf8'); - return { path: reportPath, finding_count: findings.length }; -} - -export function reviewPrompt(repoRoot, feature) { - const snippets = []; - for (const [label, key, maxFiles] of [ - ['owned', 'owned_files', 6], - ['tests', 'test_files', 4], - ['context', 'context_files', 3], - ]) { - for (const relPath of (feature[key] ?? []).slice(0, maxFiles)) { - snippets.push(fileExcerpt(repoRoot, relPath, label)); - } - } - return [ - 'Review this bounded feature record.', - '', - `Feature:\n${JSON.stringify(feature, null, 2)}`, - '', - 'Relevant files:', - '', - boundedFileContext(snippets), - '', - 'Return findings only when they are still-valid and actionable.', - 'If no issue is worth filing, return {"findings": []}.', - ].join('\n'); -} - -export function reviewOutputSchema() { - return { - type: 'object', - additionalProperties: false, - properties: { - findings: { - type: 'array', - maxItems: 8, - items: { - type: 'object', - additionalProperties: false, - properties: { - title: { type: 'string' }, - category: { - type: 'string', - enum: [ - 'bug', - 'security', - 'performance', - 'docs-gap', - 'test-gap', - 'maintainability', - ], - }, - severity: { - type: 'string', - enum: ['critical', 'high', 'medium', 'low'], - }, - confidence: { - type: 'string', - enum: ['high', 'medium', 'low'], - }, - path: { type: 'string' }, - line: { type: 'integer', minimum: 1 }, - evidence: { type: 'string' }, - recommendation: { type: 'string' }, - }, - required: [ - 'title', - 'category', - 'severity', - 'confidence', - 'path', - 'line', - 'evidence', - 'recommendation', - ], - }, - }, - }, - required: ['findings'], - }; -} - -function reviewSystemPrompt() { - return [ - 'You are reviewing a software repository through protoLabs release-tools.', - 'Return strict JSON only.', - 'Focus on actionable correctness, security, determinism, contract drift, and missing tests.', - 'Do not report style nits. Do not invent files or private context.', - ].join(' '); -} - -async function callStructuredReview({ model, systemPrompt, userPrompt, schema }) { - const apiKey = process.env.GATEWAY_API_KEY || process.env.OPENAI_API_KEY; - if (!apiKey) { - throw new CodeReviewError('GATEWAY_API_KEY or OPENAI_API_KEY is required'); - } - const baseUrl = normalizeBaseUrl( - process.env.OPENAI_BASE_URL || DEFAULT_REVIEW_BASE_URL, - ); - const rawTimeoutMs = Number(process.env.CODE_REVIEW_TIMEOUT_MS || '30000'); - const timeoutMs = - Number.isFinite(rawTimeoutMs) && rawTimeoutMs > 0 ? rawTimeoutMs : 30000; - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), timeoutMs); - let res; - try { - res = await fetch(`${baseUrl}/chat/completions`, { - method: 'POST', - signal: controller.signal, - headers: { - 'Content-Type': 'application/json', - Authorization: `Bearer ${apiKey}`, - }, - body: JSON.stringify({ - model, - messages: [ - { role: 'system', content: systemPrompt }, - { role: 'user', content: userPrompt }, - ], - response_format: { - type: 'json_schema', - json_schema: { - name: 'code_review_findings', - strict: true, - schema, - }, - }, - }), - }); - } catch (error) { - if (error?.name === 'AbortError') { - throw new CodeReviewError(`LLM API request timed out after ${timeoutMs}ms`); - } - throw error; - } finally { - clearTimeout(timeout); - } - if (!res.ok) { - throw new CodeReviewError(`LLM API error ${res.status}: ${await res.text()}`); - } - const payload = await res.json(); - const content = payload.choices?.[0]?.message?.content; - if (typeof content !== 'string') { - throw new CodeReviewError('LLM response did not include message content'); - } - try { - const parsed = JSON.parse(content); - if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { - throw new CodeReviewError('LLM response JSON must be an object'); - } - return parsed; - } catch (error) { - if (error instanceof CodeReviewError) throw error; - throw new CodeReviewError(`LLM response was not valid JSON: ${error.message}`); - } -} - -function normalizeFindings(repoRoot, feature, response) { - const rawFindings = response?.findings; - if (!Array.isArray(rawFindings)) { - throw new CodeReviewError('Review provider response must include findings list'); - } - const findings = []; - let rejected = 0; - for (const rawFinding of rawFindings) { - if (!rawFinding || typeof rawFinding !== 'object') { - rejected += 1; - continue; - } - const location = validateFindingLocation(repoRoot, feature, rawFinding); - if (!location) { - rejected += 1; - continue; - } - const finding = { - schema_version: REVIEW_SCHEMA_VERSION, - status: 'open', - feature_id: String(feature.feature_id ?? ''), - title: requiredString(rawFinding, 'title'), - category: requiredString(rawFinding, 'category'), - severity: requiredString(rawFinding, 'severity'), - confidence: requiredString(rawFinding, 'confidence'), - path: location.path, - line: location.line, - evidence: requiredString(rawFinding, 'evidence'), - recommendation: requiredString(rawFinding, 'recommendation'), - created_at: utcNow(), - }; - finding.finding_id = findingId(feature, finding); - findings.push(finding); - } - return { findings, rejected }; -} - -function validateFindingLocation(repoRoot, feature, finding) { - const relPath = typeof finding.path === 'string' ? normalizeRelPath(finding.path) : ''; - const line = Number(finding.line); - if (!relPath || !Number.isInteger(line) || line <= 0) return null; - const allowlist = new Set([ - ...(feature.owned_files ?? []), - ...(feature.context_files ?? []), - ...(feature.test_files ?? []), - ]); - if (!allowlist.has(relPath)) return null; - const absolute = safeRepoPath(repoRoot, relPath); - if (!absolute) return null; - let text; - try { - text = fs.readFileSync(absolute, 'utf8'); - } catch { - return null; - } - const lineCount = Math.max(1, text.split('\n').length); - if (line > lineCount) return null; - return { path: relPath, line }; -} - -function preserveTriage(existing, finding) { - if (!existing || typeof existing !== 'object') return finding; - const preserved = { ...finding }; - for (const key of ['status', 'triage', 'resolution', 'notes', 'owner']) { - if (existing[key] !== undefined) preserved[key] = existing[key]; - } - return preserved; -} - -function loadFeatureSpecs(repoRoot, configPath = null, stateDir = null) { - const candidates = []; - if (configPath) candidates.push(path.resolve(repoRoot, configPath)); - if (stateDir) candidates.push(path.join(stateDir, 'config.json')); - candidates.push(path.join(repoRoot, 'review-code.config.json')); - for (const candidate of candidates) { - const payload = loadJson(candidate, null); - if (!payload || typeof payload !== 'object') continue; - const specs = payload.features ?? payload.feature_specs; - if (Array.isArray(specs) && specs.length > 0) { - return specs.map(normalizeFeatureSpec); - } - } - return DEFAULT_FEATURE_SPECS.map(normalizeFeatureSpec); -} - -function normalizeFeatureSpec(spec) { - return { - feature_id: requiredConfigString(spec, 'feature_id'), - name: spec.name ? String(spec.name) : String(spec.feature_id), - description: spec.description ? String(spec.description) : '', - owned_globs: normalizeStringList(spec.owned_globs), - context_globs: normalizeStringList(spec.context_globs), - test_globs: normalizeStringList(spec.test_globs), - }; -} - -function normalizeStringList(value) { - if (!Array.isArray(value)) return []; - return value.filter((item) => typeof item === 'string' && item.trim()).map((item) => item.trim()); -} - -function requiredConfigString(spec, key) { - const value = spec?.[key]; - if (typeof value !== 'string' || !value.trim()) { - throw new CodeReviewError(`Feature spec field ${key} must be a non-empty string`); - } - return value.trim(); -} - -function loadOrMapFeatures(repoRoot, stateDir, configPath) { - const payload = loadJson(path.join(stateDir, 'features.json'), null); - if (payload && Array.isArray(payload.features)) return payload; - return mapReviewFeatures({ repoRoot, stateDir, configPath }); -} - -function featureRecord(repoRoot, allFiles, spec) { - const ownedFiles = matchFiles(allFiles, spec.owned_globs); - const contextFiles = matchFiles(allFiles, spec.context_globs).filter( - (relPath) => !ownedFiles.includes(relPath), - ); - const testFiles = matchFiles(allFiles, spec.test_globs); - return { - feature_id: spec.feature_id, - name: spec.name, - description: spec.description, - entrypoints: entrypoints(ownedFiles), - owned_files: ownedFiles, - context_files: contextFiles, - test_files: testFiles, - owned_file_count: ownedFiles.length, - context_file_count: contextFiles.length, - test_file_count: testFiles.length, - owned_bytes: ownedFiles.reduce((total, relPath) => { - try { - return total + fs.statSync(path.join(repoRoot, relPath)).size; - } catch { - return total; - } - }, 0), - }; -} - -function repoFiles(repoRoot, stateDir) { - const excludeDirs = new Set([ - '.git', - 'node_modules', - 'coverage', - 'dist', - '.clawpatch', - '.release-tools-review', - path.basename(stateDir), - ]); - const files = []; - walkRepo(repoRoot, repoRoot, excludeDirs, files); - return files.sort(); -} - -function walkRepo(repoRoot, currentDir, excludeDirs, files) { - let entries; - try { - entries = fs.readdirSync(currentDir, { withFileTypes: true }); - } catch { - return; - } - for (const entry of entries) { - if (excludeDirs.has(entry.name)) continue; - const absolute = path.join(currentDir, entry.name); - const relPath = normalizeRelPath(path.relative(repoRoot, absolute)); - if (entry.isSymbolicLink()) { - const resolved = safeResolvedPath(repoRoot, absolute); - if (!resolved) continue; - let stats; - try { - stats = fs.statSync(resolved); - } catch { - continue; - } - if (stats.isFile()) files.push(relPath); - continue; - } - if (entry.isDirectory()) { - walkRepo(repoRoot, absolute, excludeDirs, files); - continue; - } - if (entry.isFile()) files.push(relPath); - } -} - -function matchFiles(files, patterns) { - const matched = []; - for (const pattern of patterns) { - const regex = globToRegExp(pattern); - for (const relPath of files) { - if (regex.test(relPath) && !matched.includes(relPath)) { - matched.push(relPath); - } - } - } - return matched; -} - -function globToRegExp(pattern) { - const normalized = normalizeRelPath(pattern); - let source = ''; - for (let index = 0; index < normalized.length; index += 1) { - const char = normalized[index]; - const next = normalized[index + 1]; - const afterNext = normalized[index + 2]; - if (char === '*' && next === '*' && afterNext === '/') { - source += '(?:.*/)?'; - index += 2; - continue; - } - if (char === '*' && next === '*') { - source += '.*'; - index += 1; - continue; - } - if (char === '*') { - source += '[^/]*'; - continue; - } - source += char.replace(/[.+^${}()|[\]\\]/g, '\\$&'); - } - return new RegExp(`^${source}$`); -} - -function entrypoints(ownedFiles) { - const priority = ['package.json', 'action.yml']; - const prioritized = priority.filter((relPath) => ownedFiles.includes(relPath)); - return prioritized.length > 0 ? prioritized : ownedFiles.slice(0, 2); -} - -function fileExcerpt(repoRoot, relPath, label) { - const absolute = safeRepoPath(repoRoot, relPath); - if (!absolute) return `### ${label}: ${relPath}\n(unreadable: outside repo)`; - let text; - try { - text = fs.readFileSync(absolute, 'utf8'); - } catch (error) { - return `### ${label}: ${relPath}\n(unreadable: ${error.message})`; - } - if (text.length > MAX_REVIEW_FILE_CHARS) { - text = `${text.slice(0, MAX_REVIEW_FILE_CHARS)}\n......`; - } - const numbered = text - .split('\n') - .map((line, index) => `${String(index + 1).padStart(4, '0')}: ${line}`) - .join('\n'); - return `### ${label}: ${relPath}\n\`\`\`text\n${numbered}\n\`\`\``; -} - -function boundedFileContext(snippets) { - const kept = []; - let total = 0; - let omitted = 0; - for (const snippet of snippets) { - const extra = snippet.length + (kept.length > 0 ? 2 : 0); - if (kept.length > 0 && total + extra > MAX_REVIEW_CONTEXT_CHARS) { - omitted += 1; - continue; - } - if (kept.length === 0 && extra > MAX_REVIEW_CONTEXT_CHARS) { - const marker = '\n......'; - const budget = Math.max(0, MAX_REVIEW_CONTEXT_CHARS - marker.length); - kept.push(`${snippet.slice(0, budget)}${marker}`); - total = kept[0].length; - continue; - } - kept.push(snippet); - total += extra; - } - let rendered = kept.join('\n\n'); - if (omitted > 0) { - rendered += `\n\n...<${omitted} file excerpt(s) omitted by review context budget>...`; - } - return rendered; -} - -function requiredString(mapping, key) { - const value = mapping?.[key]; - if (typeof value !== 'string' || !value.trim()) { - throw new CodeReviewError(`Finding field ${key} must be a non-empty string`); - } - return value.trim(); -} - -function findingId(feature, finding) { - const raw = [ - feature.feature_id ?? '', - finding.title ?? '', - finding.path ?? '', - finding.line ?? '', - finding.evidence ?? '', - ].join('|'); - return createHash('sha256').update(raw).digest('hex').slice(0, 12); -} - -function severityRank(severity) { - return { critical: 0, high: 1, medium: 2, low: 3 }[severity] ?? 4; -} - -function resolveStateDir(repoRoot, stateDir) { - return path.isAbsolute(stateDir) ? stateDir : path.join(repoRoot, stateDir); -} - -function safeRepoPath(repoRoot, relPath) { - const absolute = path.resolve(repoRoot, relPath); - return isInside(repoRoot, absolute) ? absolute : null; -} - -function safeResolvedPath(repoRoot, absolute) { - try { - const resolved = fs.realpathSync(absolute); - return isInside(repoRoot, resolved) ? resolved : null; - } catch { - return null; - } -} - -function isInside(root, target) { - const relative = path.relative(path.resolve(root), path.resolve(target)); - return relative === '' || (!relative.startsWith('..') && !path.isAbsolute(relative)); -} - -function normalizeRelPath(value) { - return String(value).replaceAll(path.sep, '/').replace(/^\.\/+/, ''); -} - -function normalizeBaseUrl(value) { - const url = new URL(value); - if (!['http:', 'https:'].includes(url.protocol) || !url.host) { - throw new CodeReviewError('OPENAI_BASE_URL must use http or https'); - } - if (url.pathname === '' || url.pathname === '/') { - url.pathname = '/v1'; - } - return url.toString().replace(/\/$/, ''); -} - -function gitHead(repoRoot) { - try { - return execFileSync('git', ['rev-parse', 'HEAD'], { - cwd: repoRoot, - encoding: 'utf8', - stdio: ['ignore', 'pipe', 'ignore'], - }).trim(); - } catch { - return 'unknown'; - } -} - -function writeJson(filePath, payload) { - fs.mkdirSync(path.dirname(filePath), { recursive: true }); - fs.writeFileSync(filePath, `${JSON.stringify(payload, null, 2)}\n`, 'utf8'); -} - -function loadJson(filePath, fallback) { - try { - return JSON.parse(fs.readFileSync(filePath, 'utf8')); - } catch { - return fallback; - } -} - -function utcNow() { - return new Date().toISOString().replace(/\.\d{3}Z$/, 'Z'); -} - -function timestampId() { - return new Date().toISOString().replace(/[-:.]/g, ''); -} diff --git a/package.json b/package.json index b628dfd..bba350e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@protolabsai/release-tools", - "version": "1.1.0", + "version": "2.0.0", "description": "Release-notes generator + GitHub Action for protoLabs repos. Rewrites raw git commits into themed release notes via the protoLabs LLM gateway and posts a Discord embed.", "type": "module", "license": "Apache-2.0", @@ -18,7 +18,6 @@ "bin": { "rewrite-release-notes": "./bin/rewrite-release-notes.mjs", "build-updater-manifest": "./bin/build-updater-manifest.mjs", - "review-code": "./bin/review-code.mjs", "apply-branch-protection": "./bin/apply-branch-protection.mjs" }, "files": [ @@ -35,7 +34,7 @@ }, "scripts": { "lint": "eslint .", - "smoke": "node bin/rewrite-release-notes.mjs --help && node bin/build-updater-manifest.mjs --help && node bin/review-code.mjs --help && node bin/apply-branch-protection.mjs --help", + "smoke": "node bin/rewrite-release-notes.mjs --help && node bin/build-updater-manifest.mjs --help && node bin/apply-branch-protection.mjs --help", "test": "node --test test/*.mjs" }, "devDependencies": { diff --git a/test/code-review.test.mjs b/test/code-review.test.mjs deleted file mode 100644 index a984dab..0000000 --- a/test/code-review.test.mjs +++ /dev/null @@ -1,235 +0,0 @@ -import assert from 'node:assert/strict'; -import { execFileSync } from 'node:child_process'; -import fs from 'node:fs'; -import os from 'node:os'; -import path from 'node:path'; -import test from 'node:test'; -import { fileURLToPath } from 'node:url'; - -import { - MAX_REVIEW_CONTEXT_CHARS, - buildReviewReport, - initReviewState, - mapReviewFeatures, - reviewMappedFeatures, - reviewPrompt, - reviewStatus, -} from '../lib/code-review.mjs'; - -test('maps configured features and skips external symlinks', () => { - const { repo, cleanup } = createRepo(); - const outside = path.join(os.tmpdir(), `release-tools-leak-${process.pid}.js`); - fs.writeFileSync(outside, 'secret();\n', 'utf8'); - try { - try { - fs.symlinkSync(outside, path.join(repo, 'src', 'leak.js')); - } catch { - // Some platforms or filesystems disallow symlinks; the mapping behavior is - // still covered by the real file assertions below. - } - - const mapped = mapReviewFeatures({ - repoRoot: repo, - stateDir: '.review-state', - configPath: 'review-code.config.json', - }); - - const feature = mapped.features.find((item) => item.feature_id === 'app'); - assert.ok(feature); - assert.deepEqual(feature.owned_files, ['src/app.js']); - assert.deepEqual(feature.test_files, ['test/app.test.js']); - assert.ok(!feature.owned_files.includes('src/leak.js')); - } finally { - cleanup(); - fs.rmSync(outside, { force: true }); - } -}); - -test('review run persists findings, preserves triage, and reports status', async () => { - const { repo, cleanup } = createRepo(); - try { - initReviewState({ - repoRoot: repo, - stateDir: '.review-state', - configPath: 'review-code.config.json', - }); - mapReviewFeatures({ - repoRoot: repo, - stateDir: '.review-state', - configPath: 'review-code.config.json', - }); - const client = new FakeClient([ - { - title: 'App bug', - category: 'bug', - severity: 'medium', - confidence: 'high', - path: 'src/app.js', - line: 1, - evidence: 'The app exports a value.', - recommendation: 'Add a clearer assertion.', - }, - ]); - - const first = await reviewMappedFeatures({ - repoRoot: repo, - stateDir: '.review-state', - featureId: 'app', - client, - }); - - assert.equal(first.finding_count, 1); - const findingsDir = path.join(repo, '.review-state', 'findings'); - const findingFile = path.join(findingsDir, fs.readdirSync(findingsDir)[0]); - const persisted = JSON.parse(fs.readFileSync(findingFile, 'utf8')); - persisted.status = 'resolved'; - persisted.notes = 'Handled elsewhere.'; - fs.writeFileSync(findingFile, `${JSON.stringify(persisted, null, 2)}\n`, 'utf8'); - - const second = await reviewMappedFeatures({ - repoRoot: repo, - stateDir: '.review-state', - featureId: 'app', - client, - }); - const status = reviewStatus({ repoRoot: repo, stateDir: '.review-state' }); - const report = buildReviewReport({ repoRoot: repo, stateDir: '.review-state' }); - const finalFinding = JSON.parse(fs.readFileSync(findingFile, 'utf8')); - - assert.equal(second.finding_count, 1); - assert.equal(status.finding_count, 1); - assert.equal(finalFinding.status, 'resolved'); - assert.equal(finalFinding.notes, 'Handled elsewhere.'); - assert.equal(report.finding_count, 1); - assert.match( - fs.readFileSync(path.join(repo, '.review-state', 'report.md'), 'utf8'), - /App bug/, - ); - } finally { - cleanup(); - } -}); - -test('review run rejects out-of-scope findings', async () => { - const { repo, cleanup } = createRepo(); - try { - mapReviewFeatures({ - repoRoot: repo, - stateDir: '.review-state', - configPath: 'review-code.config.json', - }); - const client = new FakeClient([ - { - title: 'Outside file', - category: 'bug', - severity: 'high', - confidence: 'high', - path: 'package.json', - line: 1, - evidence: 'Not in the reviewed feature allowlist.', - recommendation: 'Do not persist this.', - }, - { - title: 'Bad line', - category: 'bug', - severity: 'low', - confidence: 'high', - path: 'src/app.js', - line: 999, - evidence: 'Line is outside file bounds.', - recommendation: 'Do not persist this either.', - }, - ]); - - const result = await reviewMappedFeatures({ - repoRoot: repo, - stateDir: '.review-state', - featureId: 'app', - client, - }); - - assert.equal(result.finding_count, 0); - assert.equal(result.rejected_count, 2); - } finally { - cleanup(); - } -}); - -test('review prompts stay bounded', () => { - const { repo, cleanup } = createRepo(); - try { - for (let index = 0; index < 8; index += 1) { - fs.writeFileSync( - path.join(repo, 'src', `large-${index}.js`), - `${'x'.repeat(5000)}\n`, - 'utf8', - ); - } - const mapped = mapReviewFeatures({ - repoRoot: repo, - stateDir: '.review-state', - configPath: 'review-code.config.json', - }); - const feature = mapped.features.find((item) => item.feature_id === 'app'); - const prompt = reviewPrompt(repo, feature); - - assert.ok(prompt.length < MAX_REVIEW_CONTEXT_CHARS + 2500); - assert.match(prompt, /omitted by review context budget/); - } finally { - cleanup(); - } -}); - -test('review-code CLI help works', () => { - const output = execFileSync('node', ['bin/review-code.mjs', '--help'], { - cwd: path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'), - encoding: 'utf8', - }); - assert.match(output, /review-code run/); -}); - -class FakeClient { - constructor(findings) { - this.findings = findings; - } - - async createStructuredOutput() { - return { findings: this.findings }; - } -} - -function createRepo() { - const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'release-tools-review-')); - fs.mkdirSync(path.join(repo, 'src'), { recursive: true }); - fs.mkdirSync(path.join(repo, 'test'), { recursive: true }); - fs.writeFileSync(path.join(repo, 'src', 'app.js'), 'export const value = 1;\n', 'utf8'); - fs.writeFileSync( - path.join(repo, 'test', 'app.test.js'), - 'import assert from "node:assert/strict";\nassert.equal(1, 1);\n', - 'utf8', - ); - fs.writeFileSync( - path.join(repo, 'review-code.config.json'), - JSON.stringify( - { - features: [ - { - feature_id: 'app', - name: 'App', - description: 'App source and tests.', - owned_globs: ['src/**/*.js'], - context_globs: [], - test_globs: ['test/**/*.js'], - }, - ], - }, - null, - 2, - ), - 'utf8', - ); - return { - repo, - cleanup: () => fs.rmSync(repo, { recursive: true, force: true }), - }; -}