diff --git a/CHANGELOG.md b/CHANGELOG.md index a54ae7c..30a5ebb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes will be documented here. This project follows Semantic Versi ### Changed +- Added strict versioned `.agentskit-review.json` policy with lens coverage, budgets, thresholds, context, and safe CI precedence; incomplete profiles require explicit local opt-in. - Made reviews fail closed when any reviewable file has no successful primary lens or cannot be ingested; advisory mode now suppresses finding-based failures only, never source/provider/execution failures. - Added primary-lens execution coverage to review summaries so partial provider degradation is visible. - Repositioned the CLI and GitHub Action as provider-neutral. diff --git a/README.md b/README.md index 98f8016..e972547 100644 --- a/README.md +++ b/README.md @@ -258,6 +258,7 @@ In shortened examples, replace `...` with `npx --yes github:AgentsKit-io/code-re | `--block ` | CI gate floor; default `blocker` | | `--no-fail` | Keep findings advisory | | `--conventions ` | Inject project conventions | +| `--allow-incomplete` | Local-only exception for a config that declares incomplete lens coverage | | `--api` | Back-compatible alias for `--provider anthropic` | | `doctor --provider ` | Offline provider diagnostics; no model request | | `doctor --live` | Explicit provider smoke-test mode | @@ -267,6 +268,33 @@ In shortened examples, replace `...` with `npx --yes github:AgentsKit-io/code-re When no conventions path is supplied, the CLI looks for `CONVENTIONS.md`, `CONTRIBUTING.md`, `.cursorrules`, or `AGENTS.md`. +### Versioned configuration + +The repository may contain one strict `.agentskit-review.json` file. It must use +`configVersion: 1`; unknown fields, secrets, unsupported values, and unsafe lens +policies fail before provider execution with exit `2`. Every built-in lens is +enabled by default, with `correctness`, `security`, and `tests` required. Flags +override file values. A required lens may only be disabled in an explicitly +declared `incompleteProfile`, which requires `--allow-incomplete` locally and is +never accepted in CI. + +```json +{ + "configVersion": 1, + "lenses": { + "performance": { "enabled": false, "required": false } + }, + "votes": 3, + "budget": { "maxFiles": 20, "maxCalls": 200, "concurrency": 1 }, + "thresholds": { "minSeverity": "med", "minConfidence": 0.7 }, + "context": { "mode": "prompt", "patterns": ["src/**"] } +} +``` + +Provider, model, transport, context trust, redaction, and permissions are +trusted execution inputs; a project config cannot set them in CI. Put provider +credentials only in the environment or provider login, never in this file. + ### Doctor Run `doctor` before a review to check a registered provider’s executable, version, transport, model requirement, configuration mode, and credential presence. It is offline by default: API credentials are checked only for presence and values are never printed; local CLI login is represented as login-managed until a provider-specific live check is available. Unknown local CLI versions warn locally and fail when `CI=true`. Exit `0` means healthy, `1` means a failed diagnostic, and `2` means invalid CLI usage. diff --git a/agents/code-review/agent.ts b/agents/code-review/agent.ts index 5126366..e75be3a 100644 --- a/agents/code-review/agent.ts +++ b/agents/code-review/agent.ts @@ -126,6 +126,8 @@ export interface CodeReviewConfig { source: SourceConfig /** Defaults to the 7 built-in lenses. Pass a subset to disable, or add custom lenses. */ lenses?: Lens[] + /** A declared incomplete profile is reported as COMMENT, never APPROVE. */ + incompleteProfile?: boolean /** Project conventions injected into every lens — a string, or a file to read. */ conventions?: string | { path: string } thresholds?: { minSeverity?: Severity; minConfidence?: number; maxPerFile?: number; suppressNits?: boolean } @@ -165,7 +167,7 @@ const Consolidation = z.object({ duplicateGroups: z.array(z.array(z.number())) } const toJson = (s: z.ZodTypeAny): JSONSchema7 => zodToJsonSchema(s) as JSONSchema7 const SEV_RANK: Record = { blocker: 0, high: 1, med: 2, nit: 3 } -const DEFAULT_LENSES: Lens[] = [ +export const DEFAULT_LENSES: Lens[] = [ { key: 'correctness', skill: correctnessLens }, { key: 'security', skill: securityLens }, { key: 'performance', skill: performanceLens }, @@ -175,6 +177,11 @@ const DEFAULT_LENSES: Lens[] = [ { key: 'conventions', skill: conventionsLens, severityCeiling: 'nit' }, ] +export function builtInLenses(enabled: readonly Category[]): Lens[] { + const selected = new Set(enabled) + return DEFAULT_LENSES.filter((lens) => selected.has(lens.key)) +} + type Limiter = (fn: () => Promise) => Promise /** @@ -416,7 +423,7 @@ export function createCodeReviewAgent(config: CodeReviewConfig) { ): ReviewResult { const counts = (['blocker', 'high', 'med', 'nit'] as Severity[]).map((s) => ({ s, n: kept.filter((f) => f.severity === s).length })) const worst = kept.length ? Math.min(...kept.map((f) => SEV_RANK[f.severity])) : 3 - const verdict: Verdict = !kept.length ? 'APPROVE' : worst <= SEV_RANK.high ? 'REQUEST CHANGES' : 'COMMENT' + const verdict: Verdict = config.incompleteProfile ? 'COMMENT' : !kept.length ? 'APPROVE' : worst <= SEV_RANK.high ? 'REQUEST CHANGES' : 'COMMENT' const blocking = kept.some((f) => SEV_RANK[f.severity] <= SEV_RANK[blockingSeverity]) const breakdown = counts.filter((c) => c.n).map((c) => `${c.n} ${c.s}`).join(', ') || 'no findings' const executionSummary = @@ -424,6 +431,7 @@ export function createCodeReviewAgent(config: CodeReviewConfig) { (execution.failed ? `; ${execution.failed} failed` : '') const summary = `${kept.length} finding(s) (${breakdown}) across ${reviewed} file(s)` + + (config.incompleteProfile ? ' Incomplete profile; this review is not an approval.' : '') + (droppedFiles ? `, ${droppedFiles} file(s) skipped for budget` : '') + `. ${executionSummary}.` return { verdict, blocking, findings: kept, dropped, execution, summary } diff --git a/docs/OPERATIONS.md b/docs/OPERATIONS.md index bb1baf0..b418fe9 100644 --- a/docs/OPERATIONS.md +++ b/docs/OPERATIONS.md @@ -36,6 +36,25 @@ Consumer configuration must select a provider through `args`. Keep credentials i The default diff base remains `origin/main`. A pre-commit invocation does not mean the input is limited to the Git staging area. Set `--base` explicitly when the repository uses another integration branch. +## Versioned review configuration + +Use a strict `.agentskit-review.json` at the repository root for review policy. +It requires `configVersion: 1` and supports lens policy (`enabled` and +`required` per built-in lens), votes, retries, thresholds, file/byte/call and +concurrency budgets, conventions, and context selection. All built-in lenses +are enabled by default; correctness, security, and tests are required. + +Flags override file values. The file cannot contain credentials or executable +plugins. Provider, model, transport, trust mode, redaction, permissions, and +other execution inputs are rejected when supplied by the project config in CI. +An intentionally incomplete profile must say `incompleteProfile: true` and be +run locally with `--allow-incomplete`; it is rejected in CI and cannot become an +approval. Malformed, unknown, or unsafe configuration exits `2` before a model +request and diagnostics do not print config values. + +Keep policy-only configuration in the file. Use trusted workflow flags or the +runner environment for provider selection, credentials, and execution mode. + ## Local Ollama review Ollama serves its local API at `http://localhost:11434` by default. Verify the service without sending repository content: diff --git a/llms-full.txt b/llms-full.txt index 0b5c137..67ceb83 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -276,6 +276,7 @@ In shortened examples, replace `...` with `npx --yes github:AgentsKit-io/code-re | `--block ` | CI gate floor; default `blocker` | | `--no-fail` | Keep findings advisory | | `--conventions ` | Inject project conventions | +| `--allow-incomplete` | Local-only exception for a config that declares incomplete lens coverage | | `--api` | Back-compatible alias for `--provider anthropic` | | `doctor --provider ` | Offline provider diagnostics; no model request | | `doctor --live` | Explicit provider smoke-test mode | @@ -285,6 +286,33 @@ In shortened examples, replace `...` with `npx --yes github:AgentsKit-io/code-re When no conventions path is supplied, the CLI looks for `CONVENTIONS.md`, `CONTRIBUTING.md`, `.cursorrules`, or `AGENTS.md`. +### Versioned configuration + +The repository may contain one strict `.agentskit-review.json` file. It must use +`configVersion: 1`; unknown fields, secrets, unsupported values, and unsafe lens +policies fail before provider execution with exit `2`. Every built-in lens is +enabled by default, with `correctness`, `security`, and `tests` required. Flags +override file values. A required lens may only be disabled in an explicitly +declared `incompleteProfile`, which requires `--allow-incomplete` locally and is +never accepted in CI. + +```json +{ + "configVersion": 1, + "lenses": { + "performance": { "enabled": false, "required": false } + }, + "votes": 3, + "budget": { "maxFiles": 20, "maxCalls": 200, "concurrency": 1 }, + "thresholds": { "minSeverity": "med", "minConfidence": 0.7 }, + "context": { "mode": "prompt", "patterns": ["src/**"] } +} +``` + +Provider, model, transport, context trust, redaction, and permissions are +trusted execution inputs; a project config cannot set them in CI. Put provider +credentials only in the environment or provider login, never in this file. + ### Doctor Run `doctor` before a review to check a registered provider’s executable, version, transport, model requirement, configuration mode, and credential presence. It is offline by default: API credentials are checked only for presence and values are never printed; local CLI login is represented as login-managed until a provider-specific live check is available. Unknown local CLI versions warn locally and fail when `CI=true`. Exit `0` means healthy, `1` means a failed diagnostic, and `2` means invalid CLI usage. @@ -397,6 +425,25 @@ Consumer configuration must select a provider through `args`. Keep credentials i The default diff base remains `origin/main`. A pre-commit invocation does not mean the input is limited to the Git staging area. Set `--base` explicitly when the repository uses another integration branch. +## Versioned review configuration + +Use a strict `.agentskit-review.json` at the repository root for review policy. +It requires `configVersion: 1` and supports lens policy (`enabled` and +`required` per built-in lens), votes, retries, thresholds, file/byte/call and +concurrency budgets, conventions, and context selection. All built-in lenses +are enabled by default; correctness, security, and tests are required. + +Flags override file values. The file cannot contain credentials or executable +plugins. Provider, model, transport, trust mode, redaction, permissions, and +other execution inputs are rejected when supplied by the project config in CI. +An intentionally incomplete profile must say `incompleteProfile: true` and be +run locally with `--allow-incomplete`; it is rejected in CI and cannot become an +approval. Malformed, unknown, or unsafe configuration exits `2` before a model +request and diagnostics do not print config values. + +Keep policy-only configuration in the file. Use trusted workflow flags or the +runner environment for provider selection, credentials, and execution mode. + ## Local Ollama review Ollama serves its local API at `http://localhost:11434` by default. Verify the service without sending repository content: @@ -747,6 +794,7 @@ All notable changes will be documented here. This project follows Semantic Versi ### Changed +- Added strict versioned `.agentskit-review.json` policy with lens coverage, budgets, thresholds, context, and safe CI precedence; incomplete profiles require explicit local opt-in. - Made reviews fail closed when any reviewable file has no successful primary lens or cannot be ingested; advisory mode now suppresses finding-based failures only, never source/provider/execution failures. - Added primary-lens execution coverage to review summaries so partial provider degradation is visible. - Repositioned the CLI and GitHub Action as provider-neutral. diff --git a/readme-standard-v1.json b/readme-standard-v1.json index f2e3d03..dd9316d 100644 --- a/readme-standard-v1.json +++ b/readme-standard-v1.json @@ -221,7 +221,7 @@ "docs/OPERATIONS.md", "test/cli-smoke.test.mjs" ], - "sourceHash": "sha256:9e43fecef7502fabffa1bc976a9febebdf14d7522431cd18e8f7bb4553078e68" + "sourceHash": "sha256:1a2be3167c85699211c0b9ba45b870d8d88d0841a5d8d99b2c4bfe211a2b22fc" }, "exceptions": [] } diff --git a/src/cli.ts b/src/cli.ts index 8f10414..8818bf7 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -16,13 +16,14 @@ import type { AdapterFactory } from '@agentskit/core' import { createProgressObserver } from '@agentskit/ink' import { readFileSync } from 'node:fs' -import { createCodeReviewAgent, type CodeReviewConfig, type Reporter, type Severity } from '../agents/code-review/agent.js' +import { builtInLenses, createCodeReviewAgent, type Category, type CodeReviewConfig, type Reporter, type Severity } from '../agents/code-review/agent.js' import { githubInlineReporter, githubSummaryReporter, markdownReporter, sarifReporter } from '../agents/code-review/reporters.js' import { claudeCode } from './claude-code-adapter.js' import { codexCli } from './codex-adapter.js' import { ollamaReview } from './ollama-adapter.js' import type { SourceConfig } from '../agents/code-review/sources.js' import { diagnoseProvider, factoryFor, providerEntry, providerRegistry, resolveProviderId, type DoctorReport, type ProviderEntry } from './provider-registry.js' +import { loadReviewConfig, type ResolvedReviewConfig } from './review-config.js' const HELP = `AgentsKit Code Review — deep, low-noise review with your model @@ -53,6 +54,7 @@ Review options: --max-files Positive file budget --concurrency Parallel model calls (default: 4) --conventions Project conventions file + --allow-incomplete Local-only exception for an explicitly incomplete profile --validate-patch Validate suggested patches with git apply --check --sarif Also write a SARIF report --post Post a PR review (with --pr) @@ -124,9 +126,24 @@ async function main() { await runDoctor() return } + const reviewConfig = loadReviewConfig(process.cwd(), { + ci: has('ci') || process.env.CI === 'true' || process.env.CI === '1', + allowIncomplete: has('allow-incomplete'), + overrides: { + provider: flag('provider') ?? (has('api') ? 'anthropic' : undefined), + model: flag('model'), + transport: flag('transport'), + votes: flag('votes') === undefined ? undefined : Number(flag('votes')), + minSeverity: flag('min-severity') as Severity | undefined, + minConfidence: flag('min-confidence') === undefined ? undefined : Number(flag('min-confidence')), + maxFiles: flag('max-files') === undefined ? undefined : Number(flag('max-files')), + concurrency: flag('concurrency') === undefined ? undefined : Number(flag('concurrency')), + conventions: flag('conventions'), + }, + }) const source = await resolveSource() - await preflightProvider() - const adapter = buildAdapter() + await preflightProvider(reviewConfig) + const adapter = buildAdapter(reviewConfig) const reporters: Reporter[] = [markdownReporter()] const sarif = flag('sarif') @@ -141,21 +158,20 @@ async function main() { source, reporters, observers: [createProgressObserver()], - auditVotes: flag('votes') ? Number(flag('votes')) : undefined, + lenses: builtInLenses(Object.entries(reviewConfig.lenses).filter(([, policy]) => policy.enabled).map(([key]) => key as Category)), + incompleteProfile: reviewConfig.incompleteProfile, + auditVotes: reviewConfig.votes, validatePatch: has('validate-patch'), blockingSeverity: (flag('block') as Severity) ?? 'blocker', - budget: { maxFiles: flag('max-files') ? Number(flag('max-files')) : undefined, concurrency: flag('concurrency') ? Number(flag('concurrency')) : 4 }, - conventions: flag('conventions') ? { path: flag('conventions')! } : autoConventions(), - thresholds: { - minSeverity: flag('min-severity') as Severity | undefined, - minConfidence: flag('min-confidence') ? Number(flag('min-confidence')) : undefined, - }, + budget: { maxFiles: reviewConfig.budget.maxFiles, concurrency: reviewConfig.budget.concurrency }, + conventions: reviewConfig.conventions ? { path: reviewConfig.conventions } : autoConventions(), + thresholds: reviewConfig.thresholds, } const review = await createCodeReviewAgent(config).run() // --no-fail = advisory: post the review but never fail the job (exit 0). Real errors // still surface via the catch below (exit 2). - process.exit(review.blocking && !has('no-fail') ? 1 : 0) + process.exit(reviewConfig.incompleteProfile ? 2 : review.blocking && !has('no-fail') ? 1 : 0) } /** @@ -163,11 +179,11 @@ async function main() { * name resolves to a `@agentskit/adapters` factory and is given * `{ apiKey, model, baseUrl? }`. `--api` is a back-compat alias for `--provider anthropic`. */ -function buildAdapter(): AdapterFactory { - const requestedProvider = flag('provider') ?? (has('api') ? 'anthropic' : undefined) +function buildAdapter(reviewConfig: ResolvedReviewConfig): AdapterFactory { + const requestedProvider = reviewConfig.provider const provider = requestedProvider && resolveProviderId(requestedProvider) if (!provider) throw new Error(requestedProvider ? `unknown --provider "${requestedProvider}" (run --list-providers for common options)` : 'choose a provider with --provider (run --list-providers for common options)') - const model = flag('model') ?? (has('api') ? 'claude-opus-4-8' : undefined) + const model = reviewConfig.model ?? (has('api') ? 'claude-opus-4-8' : undefined) if (provider === 'claude-cli') return claudeCode({ model }) if (provider === 'codex-cli') return codexCli({ model }) if (provider === 'ollama') { @@ -184,16 +200,16 @@ function buildAdapter(): AdapterFactory { return make({ apiKey, model, ...(baseUrl ? { baseUrl } : {}) }) } -async function preflightProvider(): Promise { - const requested = flag('provider') ?? (has('api') ? 'anthropic' : undefined) +async function preflightProvider(reviewConfig: ResolvedReviewConfig): Promise { + const requested = reviewConfig.provider const id = requested && resolveProviderId(requested) const entry = id && providerEntry(id) if (!entry || entry.kind === 'api') return const report = await diagnoseProvider({ provider: entry.id, - model: flag('model'), - transport: flag('transport'), - mode: flag('mode'), + model: reviewConfig.model, + transport: reviewConfig.transport, + mode: flag('mode') ?? reviewConfig.trustMode, apiKey: flag('api-key'), ci: has('ci'), }) diff --git a/src/review-config.ts b/src/review-config.ts new file mode 100644 index 0000000..382eb90 --- /dev/null +++ b/src/review-config.ts @@ -0,0 +1,148 @@ +import { existsSync, readFileSync } from 'node:fs' +import { join } from 'node:path' +import { z } from 'zod' +import type { Category, Severity } from '../agents/code-review/agent.js' + +export const BUILTIN_LENS_KEYS = [ + 'correctness', 'security', 'performance', 'maintainability', 'design', 'tests', 'conventions', +] as const satisfies readonly Category[] + +export type BuiltinLensKey = (typeof BUILTIN_LENS_KEYS)[number] +export type LensPolicy = { enabled: boolean; required: boolean } + +const lensPolicy = z.object({ enabled: z.boolean(), required: z.boolean() }).strict() +const lensOverrides = z.object({ + correctness: lensPolicy.optional(), security: lensPolicy.optional(), performance: lensPolicy.optional(), + maintainability: lensPolicy.optional(), design: lensPolicy.optional(), tests: lensPolicy.optional(), + conventions: lensPolicy.optional(), +}).strict() +const positiveInt = z.number().int().min(1) +const nonNegativeInt = z.number().int().min(0) +const relativePattern = z.string().min(1).refine( + (pattern) => !pattern.startsWith('/') && !pattern.startsWith('\\') && !/^[A-Za-z]:[\\/]/.test(pattern) && !pattern.split('/').includes('..'), + 'must be a repository-relative pattern without .. traversal', +) + +const ReviewConfigSchema = z.object({ + configVersion: z.literal(1), + lenses: lensOverrides.optional(), + incompleteProfile: z.boolean().optional(), + votes: positiveInt.max(25).optional(), + retries: nonNegativeInt.max(1).optional(), + thresholds: z.object({ + minSeverity: z.enum(['blocker', 'high', 'med', 'nit']).optional(), + minConfidence: z.number().min(0).max(1).optional(), + maxPerFile: positiveInt.optional(), + suppressNits: z.boolean().optional(), + }).strict().optional(), + budget: z.object({ + maxFiles: positiveInt.max(500).optional(), maxBytes: positiveInt.max(25 * 1024 * 1024).optional(), + maxCalls: positiveInt.max(1000).optional(), concurrency: positiveInt.max(32).optional(), + }).strict().optional(), + conventions: relativePattern.optional(), + context: z.object({ mode: z.enum(['prompt', 'isolated-snapshot']), patterns: z.array(relativePattern).max(100).optional() }).strict().optional(), + provider: z.string().min(1).max(100).optional(), model: z.string().min(1).max(200).optional(), + transport: z.enum(['api', 'acp', 'headless', 'http']).optional(), + trustMode: z.enum(['isolated', 'trusted-local']).optional(), + redaction: z.enum(['required', 'high-confidence']).optional(), + permissions: z.object({ tools: z.boolean().optional(), write: z.boolean().optional(), shell: z.boolean().optional(), mcp: z.boolean().optional() }).strict().optional(), +}).strict() + +type FileConfig = z.infer + +export interface ReviewConfigOverrides { + provider?: string; model?: string; transport?: string; votes?: number; retries?: number + minSeverity?: Severity; minConfidence?: number; maxFiles?: number; concurrency?: number; conventions?: string +} + +export interface ResolvedReviewConfig { + configVersion: 1 + lenses: Record + incompleteProfile: boolean + votes: number + retries: number + thresholds: { minSeverity?: Severity; minConfidence?: number; maxPerFile?: number; suppressNits?: boolean } + budget: { maxFiles?: number; maxBytes?: number; maxCalls?: number; concurrency: number } + conventions?: string + context: { mode: 'prompt' | 'isolated-snapshot'; patterns: string[] } + provider?: string; model?: string; transport?: 'api' | 'acp' | 'headless' | 'http' + trustMode: 'isolated'; redaction: 'required' | 'high-confidence' + permissions: { tools?: boolean; write?: boolean; shell?: boolean; mcp?: boolean } +} + +export class ReviewConfigError extends Error { + constructor(message: string) { super(message); this.name = 'ReviewConfigError' } +} + +const DEFAULT_LENSES: Record = { + correctness: { enabled: true, required: true }, security: { enabled: true, required: true }, + performance: { enabled: true, required: false }, maintainability: { enabled: true, required: false }, + design: { enabled: true, required: false }, tests: { enabled: true, required: true }, conventions: { enabled: true, required: false }, +} +const TRUSTED_ONLY_KEYS = ['provider', 'model', 'transport', 'context', 'trustMode', 'redaction', 'permissions'] as const + +function diagnostic(error: z.ZodError): string { + return error.issues.map((issue) => { + const path = issue.path.join('.') || 'config' + const label = path === 'budget.maxFiles' ? '--max-files' : path + const message = label === '--max-files' && issue.message.includes('greater than or equal to 1') ? 'must be a positive integer' : issue.message + return label === '--max-files' ? `${label} ${message}` : `${label}: ${message}` + }).join('; ') +} + +function parseFile(raw: unknown): FileConfig { + const result = ReviewConfigSchema.safeParse(raw) + if (!result.success) throw new ReviewConfigError(`invalid .agentskit-review.json: ${diagnostic(result.error)}`) + return result.data +} + +export function resolveReviewConfig( + fileConfig: unknown | undefined, + options: { ci?: boolean; allowIncomplete?: boolean; overrides?: ReviewConfigOverrides } = {}, +): ResolvedReviewConfig { + const file = fileConfig === undefined ? undefined : parseFile(fileConfig) + const overrides = options.overrides ?? {} + if (file) { + const restricted = TRUSTED_ONLY_KEYS.filter((key) => file[key] !== undefined) + if (options.ci && restricted.length) throw new ReviewConfigError(`project config cannot set trusted execution inputs in CI: ${restricted.join(', ')}`) + if (file.trustMode === 'trusted-local') throw new ReviewConfigError('project config cannot enable trusted-local mode; use an explicit trusted CLI invocation') + } + + const lenses = Object.fromEntries(BUILTIN_LENS_KEYS.map((key) => [key, { ...DEFAULT_LENSES[key], ...(file?.lenses?.[key] ?? {}) }])) as Record + const impossibleRequired = BUILTIN_LENS_KEYS.filter((key) => lenses[key].required && !lenses[key].enabled) + if (impossibleRequired.length && !file?.incompleteProfile) throw new ReviewConfigError(`required lens cannot be disabled without incompleteProfile: ${impossibleRequired.join(', ')}`) + const incompleteProfile = Boolean(file?.incompleteProfile || impossibleRequired.length) + if (options.ci && (incompleteProfile || options.allowIncomplete)) throw new ReviewConfigError('--allow-incomplete and incomplete profiles are local-only; CI cannot approve an incomplete review') + if (incompleteProfile && !options.allowIncomplete) throw new ReviewConfigError('incomplete profile requires explicit --allow-incomplete for a local run') + + const thresholds = { ...file?.thresholds, ...(overrides.minSeverity === undefined ? {} : { minSeverity: overrides.minSeverity }), ...(overrides.minConfidence === undefined ? {} : { minConfidence: overrides.minConfidence }) } + const budget = { ...file?.budget, ...(overrides.maxFiles === undefined ? {} : { maxFiles: overrides.maxFiles }), ...(overrides.concurrency === undefined ? {} : { concurrency: overrides.concurrency }) } + const effective = { + configVersion: 1 as const, lenses, incompleteProfile, + votes: overrides.votes ?? file?.votes ?? 3, retries: overrides.retries ?? file?.retries ?? 1, + thresholds, budget: { ...budget, concurrency: budget.concurrency ?? 4 }, + conventions: overrides.conventions ?? file?.conventions, + context: { mode: file?.context?.mode ?? 'prompt', patterns: file?.context?.patterns ?? [] }, + provider: overrides.provider ?? file?.provider, model: overrides.model ?? file?.model, + transport: (overrides.transport ?? file?.transport) as ResolvedReviewConfig['transport'], + trustMode: 'isolated' as const, redaction: file?.redaction ?? 'required', permissions: file?.permissions ?? {}, + } + const validation = z.object({ + votes: positiveInt.max(25), retries: nonNegativeInt.max(1), + thresholds: z.object({ minSeverity: z.enum(['blocker', 'high', 'med', 'nit']).optional(), minConfidence: z.number().min(0).max(1).optional() }), + budget: z.object({ maxFiles: positiveInt.max(500).optional(), concurrency: positiveInt.max(32) }), + }).safeParse(effective) + if (!validation.success) throw new ReviewConfigError(`invalid effective review config: ${diagnostic(validation.error)}`) + return effective +} + +export function loadReviewConfig( + cwd: string, + options: { ci?: boolean; allowIncomplete?: boolean; overrides?: ReviewConfigOverrides } = {}, +): ResolvedReviewConfig { + const file = join(cwd, '.agentskit-review.json') + if (!existsSync(file)) return resolveReviewConfig(undefined, options) + let raw: unknown + try { raw = JSON.parse(readFileSync(file, 'utf8')) as unknown } catch { throw new ReviewConfigError(`invalid ${file}: expected valid JSON`) } + return resolveReviewConfig(raw, options) +} diff --git a/test/review-config.test.mjs b/test/review-config.test.mjs new file mode 100644 index 0000000..8743a50 --- /dev/null +++ b/test/review-config.test.mjs @@ -0,0 +1,104 @@ +import assert from 'node:assert/strict' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { spawnSync } from 'node:child_process' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' +import test from 'node:test' +import { resolveReviewConfig, loadReviewConfig, ReviewConfigError } from '../dist/src/review-config.js' + +const root = resolve(fileURLToPath(new URL('.', import.meta.url)), '..') + +function tempRepo(config) { + const cwd = mkdtempSync(join(tmpdir(), 'agentskit-review-config-')) + if (config !== undefined) writeFileSync(join(cwd, '.agentskit-review.json'), JSON.stringify(config)) + return cwd +} + +test('defaults enable every lens and require correctness, security, and tests', () => { + const config = resolveReviewConfig() + assert.deepEqual(Object.fromEntries(Object.entries(config.lenses).map(([key, value]) => [key, value.enabled])), { + correctness: true, security: true, performance: true, maintainability: true, design: true, tests: true, conventions: true, + }) + assert.deepEqual(Object.entries(config.lenses).filter(([, value]) => value.required).map(([key]) => key), ['correctness', 'security', 'tests']) +}) + +test('merges independent lens policy and flags override file values', () => { + const config = resolveReviewConfig({ + configVersion: 1, + lenses: { performance: { enabled: false, required: false } }, + votes: 3, + thresholds: { minSeverity: 'high' }, + }, { overrides: { votes: 1, minSeverity: 'med' } }) + assert.equal(config.lenses.performance.enabled, false) + assert.equal(config.lenses.security.enabled, true) + assert.equal(config.votes, 1) + assert.equal(config.thresholds.minSeverity, 'med') +}) + +test('rejects unknown fields, unsupported versions, and impossible required lenses', () => { + for (const config of [ + { configVersion: 1, unknown: true }, + { configVersion: 2 }, + { configVersion: 1, lenses: { security: { enabled: false, required: true } } }, + ]) assert.throws(() => resolveReviewConfig(config), ReviewConfigError) +}) + +test('requires an explicit local exception for an incomplete profile and rejects it in CI', () => { + const config = { configVersion: 1, incompleteProfile: true, lenses: { security: { enabled: false, required: true } } } + assert.throws(() => resolveReviewConfig(config), /allow-incomplete/) + assert.equal(resolveReviewConfig(config, { allowIncomplete: true }).incompleteProfile, true) + assert.throws(() => resolveReviewConfig(config, { ci: true, allowIncomplete: true }), /local-only/) +}) + +test('CI cannot accept trusted execution inputs or secrets from the project file', () => { + assert.throws(() => resolveReviewConfig({ configVersion: 1, provider: 'openai' }, { ci: true }), /provider/) + assert.throws(() => resolveReviewConfig({ configVersion: 1, apiKey: 'secret-value' }), /apiKey/) + assert.doesNotThrow(() => resolveReviewConfig({ configVersion: 1, context: { mode: 'prompt' } })) +}) + +test('invalid config exits 2 before provider execution and does not echo secret values', () => { + const cwd = tempRepo({ configVersion: 1, apiKey: 'never-echo-this-key' }) + try { + const run = spawnSync(process.execPath, [join(root, 'dist/src/cli.js'), '--provider', 'codex-cli', '--stdin'], { + cwd, input: 'export const answer = 42\n', encoding: 'utf8', + env: { ...process.env, CI: '', PATH: '/usr/bin:/bin' }, + }) + assert.equal(run.status, 2, run.stdout) + assert.match(run.stderr, /apiKey/) + assert.doesNotMatch(`${run.stdout}\n${run.stderr}`, /never-echo-this-key/) + assert.doesNotMatch(run.stdout, /Code review —/) + } finally { rmSync(cwd, { recursive: true, force: true }) } +}) + +test('CLI applies project lens policy and flags override configuration', () => { + const cwd = tempRepo({ configVersion: 1, lenses: { performance: { enabled: false, required: false } }, votes: 3 }) + const fixtureBin = join(root, 'test/fixtures/bin') + try { + const run = spawnSync(process.execPath, [join(root, 'dist/src/cli.js'), '--provider', 'codex-cli', '--stdin', '--votes', '1', '--no-fail'], { + cwd, input: 'export const answer = 42\n', encoding: 'utf8', + env: { ...process.env, CI: '', PATH: `${fixtureBin}:${process.env.PATH ?? ''}` }, + }) + assert.equal(run.status, 0, run.stderr) + assert.match(run.stdout, /6\/6 lens executions succeeded/) + } finally { rmSync(cwd, { recursive: true, force: true }) } +}) + +test('an explicitly incomplete local profile never reports approval', () => { + const cwd = tempRepo({ configVersion: 1, incompleteProfile: true, lenses: { security: { enabled: false, required: true } } }) + const fixtureBin = join(root, 'test/fixtures/bin') + try { + const run = spawnSync(process.execPath, [join(root, 'dist/src/cli.js'), '--provider', 'codex-cli', '--stdin', '--allow-incomplete'], { + cwd, input: 'export const answer = 42\n', encoding: 'utf8', + env: { ...process.env, CI: '', PATH: `${fixtureBin}:${process.env.PATH ?? ''}` }, + }) + assert.equal(run.status, 2, run.stderr) + assert.match(run.stdout, /Code review — COMMENT/) + assert.match(run.stdout, /not an approval/) + } finally { rmSync(cwd, { recursive: true, force: true }) } +}) + +test('loadReviewConfig reads only the repository config filename', () => { + const cwd = tempRepo({ configVersion: 1, votes: 2 }) + try { assert.equal(loadReviewConfig(cwd).votes, 2) } finally { rmSync(cwd, { recursive: true, force: true }) } +})