From 87b2e90d6f1ecda384be939cbab6f67592d8c5ef Mon Sep 17 00:00:00 2001 From: Alex Dubois Date: Mon, 9 Nov 2026 14:44:49 +0100 Subject: [PATCH 1/2] Comment to be configured as function --- skills/coderuler-new-rule/reference.md | 17 ++- src/adapter/cli/commands/check.ts | 2 +- src/adapter/cli/reports.ts | 28 +++- src/app/rule.ts | 2 +- src/app/validate.ts | 4 +- tests/cli-auto-comment.test.ts | 150 +++++++++++++++++++++- tests/contracts.test.ts | 51 +++++++- tests/violation-locations.test.ts | 131 +++++++++++++++++-- website/src/content/docs/configuration.md | 6 +- 9 files changed, 360 insertions(+), 31 deletions(-) diff --git a/skills/coderuler-new-rule/reference.md b/skills/coderuler-new-rule/reference.md index f826fa6..407a73a 100644 --- a/skills/coderuler-new-rule/reference.md +++ b/skills/coderuler-new-rule/reference.md @@ -16,7 +16,7 @@ Default-export a constructible class extending `Rule` (`T = JsonValue`). Extr | `scope?: ScopeSelector` | Absent | One whole-file scope per eligible file unless supplied. | | `settings: RuleSettings` | `{}` | Optional backend, model, threshold, and input/output USD-per-million token prices; see AI below. | | `group: Grouping` | `'scope'` | Groups surviving scopes before evaluation. | -| `comment?: string` | Absent | Nonblank Markdown override for each GitHub finding body with `--auto-comment`; see below. | +| `comment?: string | callback` | Absent | Nonblank Markdown body for each GitHub finding; see below. | | `when(context: ProjectContext): Awaitable` | `true` | Whole-rule applicability, once per enabled rule before parsing target sources. | | `select(context: SelectionContext): Awaitable[]>` | Delegates | Calls `context.select({ files, exclude, scope })`; override for custom selection. | | `filter(subject: ScopeSubject, context: ProjectContext): Awaitable` | `true` | Per-scope applicability before grouping. | @@ -317,7 +317,20 @@ Explicit `lines` are absolute file coordinates: positive safe integers with incl With `locations` omitted, a scope violation uses its own source lines; a group violation does **not** automatically blame all members. Explicit `locations: []` disables automatic scope attribution. Without usable source lines (or a matching current PR diff anchor), `--auto-comment` reports the finding in an ordinary PR comment instead of guessing an inline location. -Set `comment` on the rule to replace each generated GitHub finding body with your Markdown. Omit it to keep the generated body. CodeRuler strips terminal controls and redacts credentials from overrides. Inline locations and check results stay unchanged. The fallback comment keeps its shared `CodeRuler violations` heading. +Set `comment` to a nonblank Markdown string to use the same body for each finding. You can also set a callback to build the body from the evaluated subject and its normalized `CheckResult`. + +The callback returns a string or a promise for a string. CodeRuler calls it once for each violating evaluation, after the check finishes. A grouped finding with many locations still gets one callback call. CodeRuler skips callbacks for passing evaluations and runs them only with `--auto-comment`. + +```ts +comment = function (subject, result) { + const target = subject.kind === 'scope' ? subject.filepath : subject.group.path; + return `**${this.group} ${target}**: ${result.message ?? 'Policy violation'}`; +}; +``` + +A normal function receives the rule instance as `this`, typed as `Rule`. An arrow function keeps its lexical `this`. Use it to access subclass-only fields. + +Return a nonblank string. A throw, rejected promise, blank string, or non-string result fails comment rendering. The CLI exits with status `2`. CodeRuler does not publish comments or use a generated body for that run. CodeRuler strips terminal controls and redacts credentials from resolved bodies. Markdown stays unescaped. The callback does not change check results or inline locations. Reports and JSON do not include computed bodies. Scalar AI classification supplies no offending-member attribution: scope AI can use its subject's lines, but grouped AI needs explicit locations from a custom check wrapping `context.ai(...)`; otherwise it uses the fallback comment. diff --git a/src/adapter/cli/commands/check.ts b/src/adapter/cli/commands/check.ts index 7992223..7ee795e 100644 --- a/src/adapter/cli/commands/check.ts +++ b/src/adapter/cli/commands/check.ts @@ -125,7 +125,7 @@ async function runCheck( if (options.autoComment === true) { await services.commentOnPullRequest( root, - renderViolationComments(reports, context.redact, rules), + await renderViolationComments(reports, context.redact, rules), checkout?.pullRequest, ); } diff --git a/src/adapter/cli/reports.ts b/src/adapter/cli/reports.ts index 3d66ee0..e0e69b6 100644 --- a/src/adapter/cli/reports.ts +++ b/src/adapter/cli/reports.ts @@ -671,16 +671,24 @@ export function countViolations(reports: readonly RuleReport[]): number { return total; } -export function renderViolationComments( +export async function renderViolationComments( reports: readonly RuleReport[], redact: TextRedactor, rules?: readonly LoadedRule[], -): readonly ViolationComment[] { - const overrides = new Map(); +): Promise { + const overrides = new Map; + }>(); if (rules !== undefined) { for (const entry of rules) { const comment = entry.rule.comment; - if (comment !== undefined) overrides.set(entry.name, redact(stripVTControlCharacters(comment))); + if (comment !== undefined) { + overrides.set(entry.name, { + rule: entry.rule, + comment: typeof comment === 'string' ? redact(stripVTControlCharacters(comment)) : comment, + }); + } } } const safe = (text: string): string => escapeText(redact(stripVTControlCharacters(text))) @@ -705,9 +713,7 @@ export function renderViolationComments( for (const { subject, result } of report.evaluations) { if (result.status !== 'violation') continue; let body: string; - if (override !== undefined) { - body = override; - } else { + if (override === undefined) { const label = subject.kind === 'scope' ? `scope ${location(subject)} (${safe(subject.id)})` : safe(`group ${subject.group.type} ${subject.group.path} (${subject.members.length} scopes)`); @@ -720,6 +726,14 @@ export function renderViolationComments( } if (result.message !== undefined) lines.push(` - ${safe(result.message)}`); body = `${lines.join('\n')}\n`; + } else if (typeof override.comment === 'string') { + body = override.comment; + } else { + const value: unknown = await override.comment.call(override.rule, subject, result); + if (typeof value !== 'string' || value.trim() === '') { + throw new Error(`Rule "${report.name}" comment result must be a non-blank string`); + } + body = redact(stripVTControlCharacters(value)); } const candidates = result.locations ?? (subject.kind === 'scope' ? [subject] : []); diff --git a/src/app/rule.ts b/src/app/rule.ts index 790a2ca..45090b8 100644 --- a/src/app/rule.ts +++ b/src/app/rule.ts @@ -221,7 +221,7 @@ export abstract class Rule { scope?: ScopeSelector; settings: RuleSettings = {}; group: Grouping = 'scope'; - comment?: string; + comment?: string | ((this: Rule, subject: Subject, result: CheckResult) => Awaitable); when(_context: ProjectContext): Awaitable { return true; diff --git a/src/app/validate.ts b/src/app/validate.ts index 56816ee..04f2c19 100644 --- a/src/app/validate.ts +++ b/src/app/validate.ts @@ -98,8 +98,8 @@ export function validateRule(value: unknown, ruleFile: string): Rule { validateSettings(settings, at); const comment = value['comment']; - if (comment !== undefined && (typeof comment !== 'string' || comment.trim() === '')) { - invalid(at, 'comment must be a non-blank string'); + if (comment !== undefined && typeof comment !== 'function' && (typeof comment !== 'string' || comment.trim() === '')) { + invalid(at, 'comment must be a non-blank string or a function'); } if (!isGrouping(value['group'])) { diff --git a/tests/cli-auto-comment.test.ts b/tests/cli-auto-comment.test.ts index 60a97f5..a467335 100644 --- a/tests/cli-auto-comment.test.ts +++ b/tests/cli-auto-comment.test.ts @@ -29,14 +29,15 @@ interface Fixture extends Sandbox { readonly head: string; } -const scoped = (check: string, options = '') => `import { Rule, pass, violation, regex } from '@alex-dbs/coderuler'; +const scoped = (check: string, options = '', privateData = 'DATA-SECRET') => `import { Rule, pass, violation, regex } from '@alex-dbs/coderuler'; export default class FixtureRule extends Rule { files = ['src/*.txt']; - scope = file => [{ id: '$', source: file.source, data: { private: 'DATA-SECRET' }, captures: { private: 'CAPTURE-SECRET' }, lines: { start: 2, end: 3 } }]; + scope = file => [{ id: '$', source: file.source, data: { private: ${JSON.stringify(privateData)} }, captures: { private: 'CAPTURE-SECRET' }, lines: { start: 2, end: 3 } }]; ${options} check = ${check}; } `; + const implicit = scoped(`subject => subject.source.includes('BAD') ? violation(${JSON.stringify(MESSAGE)}) : pass('PASS-MESSAGE')`); const unlocatedGroup = scoped(`() => violation('unlocated group rejection')`, `group = 'folder';`); @@ -196,6 +197,53 @@ for (const diff of ['--diff', '--diff-only']) { } const CUSTOM = '**Policy** — follow [the guide](https://example.invalid/guide) and keep `snake_case`.\n\n- mention @scope'; +for (const kind of ['sync', 'async'] as const) { + test(`subject-based comments (${kind})`, async context => { + const callback = `${kind === 'async' ? 'async ' : ''}function (subject, result) { + if (result.status !== 'violation') throw new Error('comment called for a passing result'); + ${kind === 'async' ? 'await Promise.resolve();' : ''} + const token = ${JSON.stringify(TOKEN)}; + const apiKey = ${JSON.stringify(API_KEY)}; + return this.guidance + ' ' + subject.filepath + ': ' + subject.data.private + '. ' + result.message + ' ' + + token.slice(0, 5) + ${JSON.stringify('\u001b[31m')} + token.slice(5) + ${JSON.stringify('\u001b[0m')} + ' ' + + apiKey.slice(0, 5) + ${JSON.stringify('\u001b[31m')} + apiKey.slice(5) + ${JSON.stringify('\u001b[0m')}; + }`; + const target = repository(`subject-comment-${kind}`, { + custom: scoped( + `subject => subject.source.includes('BAD') ? violation('Needs trusted data') : pass()`, + `guidance = '**Policy**'; comment = ${callback};`, + 'field-specific guidance', + ), + }); + const requests = await github(context, target); + const result = await check(target, ['--diff', '--base', target.base, '--auto-comment', '--json', '--output-all']); + assert.equal(result.status, 1, result.stderr); + assert.equal(result.stderr, ''); + assert.equal(posts(requests).length, 1); + const payload = review(requests); + assert.deepEqual(payload.comments.map(({ path, body, start_line, line, side, start_side }) => ({ + path, body, start_line, line, side, start_side, + })).sort((left, right) => left.path.localeCompare(right.path)), [ + { + path: 'src/one.txt', + body: '**Policy** src/one.txt: field-specific guidance. Needs trusted data [REDACTED] [REDACTED]', + start_line: 2, line: 3, side: 'RIGHT', start_side: 'RIGHT', + }, + { + path: 'src/two.txt', + body: '**Policy** src/two.txt: field-specific guidance. Needs trusted data [REDACTED] [REDACTED]', + start_line: 2, line: 3, side: 'RIGHT', start_side: 'RIGHT', + }, + ]); + const report = JSON.parse(result.stdout).reports[0]; + const failures = report.evaluations.filter(({ result: finding }: { result: { status: string } }) => finding.status === 'violation'); + assert.deepEqual(failures.map(({ result: finding }: { result: { message: string } }) => finding.message), [ + 'Needs trusted data', 'Needs trusted data', + ]); + assert.ok(failures.every(({ result: finding }: { result: Record }) => !Object.hasOwn(finding, 'comment'))); + assert.equal(Object.hasOwn(report, 'comment'), false); + }); +} test('per-rule comments replace review bodies verbatim and preserve locations', async context => { const target = repository('custom-review', { @@ -247,6 +295,104 @@ test('a commented group rule uses the comment as the whole fallback finding', as assert.equal(fallback(requests), `## CodeRuler violations\n\n${CUSTOM}\n`); }); +test('an async group callback uses group metadata for the fallback finding', async context => { + const target = repository('custom-async-fallback', { + grouped: scoped(`() => violation('group rejection')`, `group = 'folder'; comment = async function (subject, result) { + if (result.status !== 'violation' || subject.kind !== 'group') throw new Error('comment called for a non-group violation'); + await Promise.resolve(); + return '**' + this.group + ' ' + subject.group.path + '**: ' + result.message; + };`), + }); + const requests = await github(context, target); + const result = await check(target, ['--diff', '--base', target.base, '--auto-comment']); + assert.equal(result.status, 1, result.stderr); + assert.equal(result.stderr, ''); + assert.equal(posts(requests).length, 1); + assert.equal(fallback(requests), '## CodeRuler violations\n\n**folder src**: group rejection\n'); +}); + +for (const kind of ['sync-throw', 'async-rejection'] as const) { + test(`${kind} comment failure stops publishing after an earlier result`, async context => { + const diagnostic = kind === 'sync-throw' ? TOKEN : API_KEY; + const failure = kind === 'sync-throw' + ? `throw new Error(${JSON.stringify(diagnostic)});` + : `return Promise.reject(new Error(${JSON.stringify(diagnostic)}));`; + const callback = `${kind === 'async-rejection' ? 'async ' : ''}function (subject, result) { + if (result.status !== 'violation') throw new Error('comment called for a passing result'); + if (subject.filepath === 'src/two.txt') { + if (!this.commentResolved) throw new Error('src/one.txt comment did not resolve first'); + ${failure} + } + this.commentResolved = true; + return 'resolved before the failure'; + }`; + const target = repository(`comment-${kind}`, { + custom: scoped(`subject => subject.source.includes('BAD') ? violation('Needs trusted data') : pass()`, `comment = ${callback};`), + }); + const requests = await github(context, target); + const result = await check(target, ['--diff', '--base', target.base, '--auto-comment', '--json', '--output-all']); + assert.equal(result.status, 2); + assert.match(result.stderr, /\[REDACTED\]/); + assert.equal(result.stderr.includes(TOKEN), false); + assert.equal(result.stderr.includes(API_KEY), false); + assert.ok(JSON.parse(result.stdout).reports[0].evaluations.some(({ result: finding }: { result: { message?: string } }) => finding.message === 'Needs trusted data')); + assert.deepEqual(posts(requests), []); + }); +} + +for (const [kind, invalid] of [ + ['number', '42'], + ['undefined', 'undefined'], + ['empty', "''"], + ['whitespace', "' \\t\\n '"], +] as const) { + test(`async comment rejects ${kind} results after an earlier result`, async context => { + const callback = `async function (subject, result) { + if (result.status !== 'violation') throw new Error('comment called for a passing result'); + if (subject.filepath === 'src/two.txt') { + if (!this.commentResolved) throw new Error('src/one.txt comment did not resolve first'); + return ${invalid}; + } + this.commentResolved = true; + return 'resolved before the invalid result'; + }`; + const target = repository(`comment-invalid-${kind}`, { + custom: scoped(`subject => subject.source.includes('BAD') ? violation('Needs trusted data') : pass()`, `comment = ${callback};`), + }); + const requests = await github(context, target); + const result = await check(target, ['--diff', '--base', target.base, '--auto-comment', '--json', '--output-all']); + assert.equal(result.status, 2); + assert.match(result.stderr, /Rule "custom" comment result must be a non-blank string/); + assert.ok(JSON.parse(result.stdout).reports[0].evaluations.some(({ result: finding }: { result: { message?: string } }) => finding.message === 'Needs trusted data')); + assert.deepEqual(posts(requests), []); + }); +} + +test('a throwing callback is not invoked without --auto-comment', async context => { + const target = repository('comment-without-publish', { + custom: scoped( + `subject => subject.source.includes('BAD') ? violation('Needs trusted data') : pass()`, + `comment = function () { throw new Error(${JSON.stringify(`must not run ${TOKEN}`)}); };`, + ), + }); + const requests = await github(context, target); + const result = await check(target, ['--diff', '--base', target.base, '--json', '--output-all']); + assert.equal(result.status, 1, result.stderr); + assert.equal(result.stderr, ''); + const reports = JSON.parse(result.stdout).reports; + assert.deepEqual(reports.map((report: { name: string; status: string }) => ({ name: report.name, status: report.status })), [ + { name: 'custom', status: 'violation' }, + ]); + const findings: { status?: string; message?: string; comment?: unknown }[] = reports[0].evaluations.map( + ({ result: finding }: { result: { status?: string; message?: string; comment?: unknown } }) => finding, + ); + assert.deepEqual(findings.filter(finding => finding.status === 'violation').map(finding => finding.message), [ + 'Needs trusted data', 'Needs trusted data', + ]); + assert.ok(findings.every(finding => !Object.hasOwn(finding, 'comment'))); + assert.deepEqual(posts(requests), []); +}); + test('commented findings strip terminal controls before redaction and keep Markdown unescaped', async context => { const token = 'supersecretgithubtoken'; const apiKey = 'supersecrettypesafekey'; diff --git a/tests/contracts.test.ts b/tests/contracts.test.ts index ba4c396..5bd6cfe 100644 --- a/tests/contracts.test.ts +++ b/tests/contracts.test.ts @@ -397,6 +397,39 @@ test('a separately defined Rule copy carries the same brand without being an ins }); test('validateRule accepts structurally valid rules in every grouping and setting form', () => { + void (() => { + type RuleComment = NonNullable['comment']>; + const rule = null as unknown as Rule<{ name: string }>; + + const comment: RuleComment = function (this: Rule<{ name: string }>, subject, result) { + let name: string | undefined; + if (subject.kind === 'scope') { + name = subject.data?.name; + } else { + name = subject.members[0]?.data?.name; + } + const message: string | undefined = result.message; + return `${this.group}: ${name ?? ''} ${message ?? ''}`; + }; + rule.comment = comment; + + const asyncComment: RuleComment = async (subject, result) => { + const name = subject.kind === 'scope' ? subject.data?.name : subject.members[0]?.data?.name; + const message: string | undefined = result.message; + return `${name ?? ''} ${message ?? ''}`; + }; + rule.comment = asyncComment; + + // @ts-expect-error comments must return a string + rule.comment = () => 42; + // @ts-expect-error comments must return a string promise + rule.comment = async () => 42; + // @ts-expect-error comments must return a string + rule.comment = () => undefined; + // @ts-expect-error comments must return a string promise + rule.comment = async () => undefined; + }); + const accepted: readonly Record[] = [ brandedRule(), brandedRule({ enabled: false }), @@ -408,6 +441,8 @@ test('validateRule accepts structurally valid rules in every grouping and settin brandedRule({ group: { by: 'folder', level: -3 } }), brandedRule({ exclude: ['old/**'] }), brandedRule({ comment: 'Follow **this** policy.' }), + brandedRule({ comment: () => { throw new Error('comment callback must not run during validation'); } }), + brandedRule({ comment: async () => { throw new Error('comment callback must not run during validation'); } }), brandedRule({ settings: { backend: 'jev', model: 'jev-latest', threshold: 0 } }), brandedRule({ settings: { backend: 'jev', model: 'jev-latest', threshold: 1 } }), brandedRule({ settings: { backend: 'openai', model: 'gpt-6-luna' } }), @@ -490,13 +525,25 @@ test('validateRule rejects malformed rules even when they are disabled', () => { for (const [label, mutate] of malformed) { const rule = brandedRule(); mutate(rule); - expectThrows(label, () => validateRule(rule, RULE_FILE)); + if (label.endsWith('comment')) { + assert.throws(() => validateRule(rule, RULE_FILE), { + message: `Invalid rule at ${RULE_FILE}: comment must be a non-blank string or a function`, + }); + } else { + expectThrows(label, () => validateRule(rule, RULE_FILE)); + } } for (const [label, mutate] of malformed) { const rule = brandedRule({ enabled: false }); mutate(rule); - expectThrows(`disabled ${label}`, () => validateRule(rule, RULE_FILE)); + if (label.endsWith('comment')) { + assert.throws(() => validateRule(rule, RULE_FILE), { + message: `Invalid rule at ${RULE_FILE}: comment must be a non-blank string or a function`, + }); + } else { + expectThrows(`disabled ${label}`, () => validateRule(rule, RULE_FILE)); + } } }); diff --git a/tests/violation-locations.test.ts b/tests/violation-locations.test.ts index a0797e1..3b07004 100644 --- a/tests/violation-locations.test.ts +++ b/tests/violation-locations.test.ts @@ -97,14 +97,121 @@ test('scope violations infer their own range only when locations are omitted', a const implicit = await run(scopedRule(() => violation('Scope policy failed'))); assert.equal(implicit.report.status, 'violation'); assert.ok(implicit.report.evaluations.every(({ result }) => result.locations === undefined)); - assert.deepEqual(renderViolationComments(implicit.reports, redact).map(comment => comment.locations), [ + assert.deepEqual((await renderViolationComments(implicit.reports, redact)).map(comment => comment.locations), [ [{ filepath: 'pkg/a.txt', lines: { start: 1, end: 3 } }], [{ filepath: 'pkg/a.txt', lines: { start: 7, end: 9 } }], ]); const suppressed = await run(scopedRule(() => violation('Do not infer blame', []))); assert.deepEqual(suppressed.report.evaluations.map(({ result }) => result.locations), [[], []]); - assert.deepEqual(renderViolationComments(suppressed.reports, redact).map(comment => comment.locations), [[], []]); + assert.deepEqual((await renderViolationComments(suppressed.reports, redact)).map(comment => comment.locations), [[], []]); +}); + +test('protobuf field comments preserve each field source location', async () => { + const source = 'syntax = "proto3";\nmessage Request {\n string organization_id = 1;\n string user_id = 2;\n}\n'; + const project = fixture({ 'pkg/fields.proto': source }); + + const syncRule = new LocationRule(() => violation('Use trusted token data')); + syncRule.files = ['pkg/*.proto']; + syncRule.scope = scopes.protobuf<{ name: string }>({ path: '$.fields[*]' }) as ScopeSelector; + syncRule.comment = function (subject, result) { + assert.equal(subject.kind, 'scope'); + if (subject.kind !== 'scope') throw new Error('Expected a protobuf field scope'); + const data = subject.data as { name: string }; + return `**${data.name}**: ${result.message}. [${data.name === 'organization_id' ? 'Organization' : 'User'} policy](https://example.invalid/${data.name === 'organization_id' ? 'organization' : 'user'})`; + }; + + const sync = await run(syncRule, project); + assert.deepEqual(sync.report.evaluations.map(({ result }) => result), [ + { status: 'violation', message: 'Use trusted token data' }, + { status: 'violation', message: 'Use trusted token data' }, + ]); + assert.deepEqual(await renderViolationComments(sync.reports, redact, [ + { name: 'location-policy', ruleFile: '.coderuler/location-policy.rule.ts', rule: syncRule as Rule }, + ]), [ + { + body: '**organization_id**: Use trusted token data. [Organization policy](https://example.invalid/organization)', + locations: [{ filepath: 'pkg/fields.proto', lines: { start: 3, end: 3 } }], + }, + { + body: '**user_id**: Use trusted token data. [User policy](https://example.invalid/user)', + locations: [{ filepath: 'pkg/fields.proto', lines: { start: 4, end: 4 } }], + }, + ]); + + const asyncRule = new LocationRule(() => violation('Use trusted token data')); + asyncRule.files = ['pkg/*.proto']; + asyncRule.scope = scopes.protobuf<{ name: string }>({ path: '$.fields[*]' }) as ScopeSelector; + asyncRule.comment = async function (subject, result) { + await Promise.resolve(); + assert.equal(subject.kind, 'scope'); + if (subject.kind !== 'scope') throw new Error('Expected a protobuf field scope'); + const data = subject.data as { name: string }; + return `**${data.name}**: ${result.message}. [${data.name === 'organization_id' ? 'Organization' : 'User'} policy](https://example.invalid/${data.name === 'organization_id' ? 'organization' : 'user'})`; + }; + + const asyncResult = await run(asyncRule, project); + assert.deepEqual(asyncResult.report.evaluations.map(({ result }) => result), [ + { status: 'violation', message: 'Use trusted token data' }, + { status: 'violation', message: 'Use trusted token data' }, + ]); + assert.deepEqual(await renderViolationComments(asyncResult.reports, redact, [ + { name: 'location-policy', ruleFile: '.coderuler/location-policy.rule.ts', rule: asyncRule as Rule }, + ]), [ + { + body: '**organization_id**: Use trusted token data. [Organization policy](https://example.invalid/organization)', + locations: [{ filepath: 'pkg/fields.proto', lines: { start: 3, end: 3 } }], + }, + { + body: '**user_id**: Use trusted token data. [User policy](https://example.invalid/user)', + locations: [{ filepath: 'pkg/fields.proto', lines: { start: 4, end: 4 } }], + }, + ]); +}); + +test('group comments render once with all resolved member locations', async () => { + let callbackCalls = 0; + const rule = scopedRule(subject => { + if (subject.kind !== 'group') throw new Error('Expected a project group'); + return violation('Group rejection', [ + { filepath: subject.members[0]!.filepath, scopeId: 'head' }, + { filepath: subject.members[1]!.filepath, scopeId: 'tail' }, + ]); + }, 'project'); + rule.comment = async function (subject, result) { + callbackCalls += 1; + assert.equal(this.group, 'project'); + assert.equal(subject.kind, 'group'); + if (subject.kind !== 'group') throw new Error('Expected a project group'); + assert.equal(subject.group.path, '.'); + assert.deepEqual(subject.members.map(member => member.id), ['head', 'tail']); + assert.equal(result.locations?.length, 2); + return `**${this.group} ${subject.group.path}**: ${subject.members.map(member => member.id).join(', ')} (${result.locations!.length} locations)`; + }; + + const result = await run(rule); + assert.deepEqual(await renderViolationComments(result.reports, redact, [ + { name: 'location-policy', ruleFile: '.coderuler/location-policy.rule.ts', rule: rule as Rule }, + ]), [{ + body: '**project .**: head, tail (2 locations)', + locations: [ + { filepath: 'pkg/a.txt', lines: { start: 1, end: 3 } }, + { filepath: 'pkg/a.txt', lines: { start: 7, end: 9 } }, + ], + }]); + assert.equal(callbackCalls, 1); + + const passing = scopedRule(() => pass(), 'project'); + let passingCallbackCalls = 0; + passing.comment = () => { + passingCallbackCalls += 1; + throw new Error('Comment callback must not run for a passing evaluation'); + }; + const passingResult = await run(passing); + assert.deepEqual(await renderViolationComments(passingResult.reports, redact, [ + { name: 'location-policy', ruleFile: '.coderuler/location-policy.rule.ts', rule: passing as Rule }, + ]), []); + assert.equal(passingCallbackCalls, 0); }); test('scopeId-only and explicit subranges resolve into consumer-visible result locations', async () => { @@ -130,7 +237,7 @@ test('scopeId-only and explicit subranges resolve into consumer-visible result l [{ filepath: 'pkg/a.txt', scopeId: 'head', lines: { start: 1, end: 2 } }], [{ filepath: 'pkg/a.txt', scopeId: 'tail', lines: { start: 7, end: 8 } }], ]); - assert.deepEqual(renderViolationComments(explicit.reports, redact).map(comment => comment.locations), [ + assert.deepEqual((await renderViolationComments(explicit.reports, redact)).map(comment => comment.locations), [ [{ filepath: 'pkg/a.txt', lines: { start: 1, end: 2 } }], [{ filepath: 'pkg/a.txt', lines: { start: 7, end: 8 } }], ]); @@ -150,7 +257,7 @@ for (const group of ['scope', 'file', 'folder', 'project'] as const) { assert.deepEqual(failures[0]!.result.locations, [ { filepath: 'pkg/a.txt', scopeId: ' ', lines: { start: 3, end: 3 } }, ]); - assert.deepEqual(renderViolationComments(result.reports, redact).map(comment => comment.locations), [ + assert.deepEqual((await renderViolationComments(result.reports, redact)).map(comment => comment.locations), [ [{ filepath: 'pkg/a.txt', lines: { start: 3, end: 3 } }], ]); assert.equal(result.events.some(event => event.type === 'error'), false); @@ -173,7 +280,7 @@ for (const group of ['scope', 'file', 'folder', 'project'] as const) { message: 'Whitespace identity fails', locations: [{ filepath: ' ', scopeId: ' ', lines: { start: 1, end: 1 } }], }]); - assert.deepEqual(renderViolationComments(result.reports, redact).map(comment => comment.locations), [ + assert.deepEqual((await renderViolationComments(result.reports, redact)).map(comment => comment.locations), [ [{ filepath: ' ', lines: { start: 1, end: 1 } }], ]); assert.equal(result.events.some(event => event.type === 'error'), false); @@ -196,7 +303,7 @@ for (const group of ['file', 'folder', 'project'] as const) { assert.deepEqual(result.report.evaluations.map(({ result }) => result.locations), paths.map(filepath => [ { filepath, scopeId: 'tail', lines: { start: 7, end: 9 } }, ])); - assert.deepEqual(renderViolationComments(result.reports, redact).map(comment => comment.locations), paths.map(filepath => [ + assert.deepEqual((await renderViolationComments(result.reports, redact)).map(comment => comment.locations), paths.map(filepath => [ { filepath, lines: { start: 7, end: 9 } }, ])); }); @@ -205,7 +312,7 @@ for (const group of ['file', 'folder', 'project'] as const) { const result = await run(scopedRule(() => violation('Group policy failed'), group)); assert.equal(result.report.status, 'violation'); assert.equal(result.report.evaluations[0]!.result.locations, undefined); - assert.deepEqual(renderViolationComments(result.reports, redact)[0]!.locations, []); + assert.deepEqual((await renderViolationComments(result.reports, redact))[0]!.locations, []); }); } @@ -322,7 +429,7 @@ test('locations without known member lines remain unanchored unless callers supp assert.deepEqual(result.report.evaluations[0]!.result.locations, [ { filepath: 'pkg/a.txt', scopeId: 'virtual' }, ]); - assert.deepEqual(renderViolationComments(result.reports, redact)[0]!.locations, []); + assert.deepEqual((await renderViolationComments(result.reports, redact))[0]!.locations, []); unanchored.check = () => violation('Caller knows source position', [ { filepath: 'pkg/a.txt', scopeId: 'virtual', lines: { start: 50, end: 51 } }, @@ -628,7 +735,7 @@ for (const group of ['scope', 'file', 'folder', 'project'] as const) { ]); assert.ok(result.report.evaluations.filter(({ result }) => result.status === 'pass') .every(({ result }) => result.locations === undefined)); - assert.deepEqual(renderViolationComments(result.reports, redact).flatMap(comment => comment.locations), [ + assert.deepEqual((await renderViolationComments(result.reports, redact)).flatMap(comment => comment.locations), [ { filepath: 'pkg/a.txt', lines: { start: 2, end: 2 } }, { filepath: 'pkg/b.txt', lines: { start: 1, end: 1 } }, ]); @@ -645,7 +752,7 @@ test('regex preserves unanchored failing members without blaming passing ranged const result = await run(rule, fixture({ 'pkg/a.txt': 'ok', 'pkg/b.txt': 'bad' })); assert.deepEqual(result.report.evaluations[0]!.result.locations, [{ filepath: 'pkg/b.txt', scopeId: 'member' }]); assert.equal(result.report.evaluations[0]!.result.message, undefined); - assert.deepEqual(renderViolationComments(result.reports, redact)[0]!.locations, []); + assert.deepEqual((await renderViolationComments(result.reports, redact))[0]!.locations, []); }); test('passing regex evaluations preserve pass behavior without location metadata', async () => { @@ -654,7 +761,7 @@ test('passing regex evaluations preserve pass behavior without location metadata const result = await run(rule, fixture({ 'pkg/a.txt': 'ok', 'pkg/b.txt': 'ok' })); assert.equal(result.report.status, 'pass'); assert.deepEqual(result.report.evaluations[0]!.result, pass()); - assert.deepEqual(renderViolationComments(result.reports, redact), []); + assert.deepEqual(await renderViolationComments(result.reports, redact), []); }); test('AI scalar results use only scope fallback and never invent grouped attribution or extra calls', async () => { @@ -670,7 +777,7 @@ test('AI scalar results use only scope fallback and never invent grouped attribu const result = await run(rule, fixture(), { backend }); assert.equal(calls, result.report.evaluations.length); assert.ok(result.report.evaluations.every(({ result }) => result.locations === undefined)); - const comments = renderViolationComments(result.reports, redact); + const comments = await renderViolationComments(result.reports, redact); assert.equal(comments.length, result.report.evaluations.length); assert.deepEqual(comments.map(comment => comment.locations), group === 'scope' ? [ [{ filepath: 'pkg/a.txt', lines: { start: 1, end: 3 } }], diff --git a/website/src/content/docs/configuration.md b/website/src/content/docs/configuration.md index c8c8bbd..a714a51 100644 --- a/website/src/content/docs/configuration.md +++ b/website/src/content/docs/configuration.md @@ -32,7 +32,7 @@ A shared Git repository is a good way to distribute company-wide rules across pr | `scope?: ScopeSelector` | Absent | Select parts of a file instead of its whole source | | `settings: RuleSettings` | `{}` | AI backend, model, threshold, and token prices | | `group: Grouping` | `'scope'` | Decide which scopes a check sees together | -| `comment?: string` | Absent | Nonblank Markdown override for GitHub finding bodies | +| `comment?: string or callback` | Absent | Nonblank Markdown override for GitHub finding bodies | | `when(context)` | Returns `true` | Whole-rule applicability before file selection | | `select(context)` | Built-in selection | Return selected scope subjects | | `filter(subject, context)` | Returns `true` | Per-scope applicability before grouping | @@ -83,6 +83,8 @@ Group checks receive `subject.kind === 'group'` and `subject.members`. One group `RuleSettings` supports `backend: 'jev'`, `model`, `threshold`, `inputCostPerMillion`, and `outputCostPerMillion`. See [AI checks](/docs/ai-checks/) for defaults and precedence. -`comment` replaces the generated finding body during `check --diff --auto-comment` or `check --pr ... --auto-comment`. It does not change decisions or inline locations. Omit it to keep generated text. CodeRuler strips terminal controls and redacts credentials from overrides. +`comment` can be a nonblank Markdown string or a callback. The callback receives the evaluated subject and normalized check result. It returns a string or a promise for a string, and runs once for each violating evaluation. A normal function receives the rule instance as `this`. + +The comment replaces the generated finding body during `check --diff --auto-comment` or `check --pr ... --auto-comment`. It does not change decisions or inline locations. Omit it to keep generated text. CodeRuler strips terminal controls and redacts credentials from comments. The class contracts are defined in [rule.ts](https://github.com/alex-dbs/coderuler/blob/main/src/app/rule.ts). The [CLI reference](/docs/cli/) lists command-level options. From bf0bb828221cfa3c9965f22c56dd4d108c4cdcbb Mon Sep 17 00:00:00 2001 From: Alex Dubois Date: Mon, 9 Nov 2026 14:54:00 +0100 Subject: [PATCH 2/2] cleanup --- skills/coderuler-new-rule/reference.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/skills/coderuler-new-rule/reference.md b/skills/coderuler-new-rule/reference.md index 407a73a..bcff934 100644 --- a/skills/coderuler-new-rule/reference.md +++ b/skills/coderuler-new-rule/reference.md @@ -16,7 +16,7 @@ Default-export a constructible class extending `Rule` (`T = JsonValue`). Extr | `scope?: ScopeSelector` | Absent | One whole-file scope per eligible file unless supplied. | | `settings: RuleSettings` | `{}` | Optional backend, model, threshold, and input/output USD-per-million token prices; see AI below. | | `group: Grouping` | `'scope'` | Groups surviving scopes before evaluation. | -| `comment?: string | callback` | Absent | Nonblank Markdown body for each GitHub finding; see below. | +| `comment` (optional) | Absent | Nonblank Markdown string or callback for each GitHub finding; see below. | | `when(context: ProjectContext): Awaitable` | `true` | Whole-rule applicability, once per enabled rule before parsing target sources. | | `select(context: SelectionContext): Awaitable[]>` | Delegates | Calls `context.select({ files, exclude, scope })`; override for custom selection. | | `filter(subject: ScopeSubject, context: ProjectContext): Awaitable` | `true` | Per-scope applicability before grouping. | @@ -322,10 +322,18 @@ Set `comment` to a nonblank Markdown string to use the same body for each findin The callback returns a string or a promise for a string. CodeRuler calls it once for each violating evaluation, after the check finishes. A grouped finding with many locations still gets one callback call. CodeRuler skips callbacks for passing evaluations and runs them only with `--auto-comment`. ```ts -comment = function (subject, result) { - const target = subject.kind === 'scope' ? subject.filepath : subject.group.path; - return `**${this.group} ${target}**: ${result.message ?? 'Policy violation'}`; -}; +import { Rule, violation, type CheckResult, type Subject } from '@alex-dbs/coderuler'; + +type PolicyData = { name: string }; + +class NamePolicy extends Rule { + check = () => violation('Policy violation'); + + comment = function (this: Rule, subject: Subject, result: CheckResult) { + const target = subject.kind === 'scope' ? subject.filepath : subject.group.path; + return `**${this.group} ${target}**: ${result.message ?? 'Policy violation'}`; + }; +} ``` A normal function receives the rule instance as `this`, typed as `Rule`. An arrow function keeps its lexical `this`. Use it to access subclass-only fields.