feat(doctor): proximity-based DRY detection + --fix auto-repair (v0.14.1) (#254)
* feat(doctor): proximity-based DRY detection + --fix auto-repair
Fixes false-positive DRY violations on skills that properly delegate
notability/filing rules to `skills/_brain-filing-rules.md`. The old
check only accepted `conventions/quality.md` as a valid delegation
target, leaving 9 skills flagged every run even though they delegate
correctly.
- CROSS_CUTTING_PATTERNS.conventions is now an array; notability gate
accepts both `conventions/quality.md` AND `_brain-filing-rules.md`
- New extractDelegationTargets() parses `> **Convention:**`,
`> **Filing rule:**`, and inline backtick references
- DRY suppression is proximity-based (K=40 lines) via DRY_PROXIMITY_LINES
- New src/core/dry-fix.ts module with autoFixDryViolations:
- expanders strategy map (bullet / blockquote / paragraph)
- 5 guards: working-tree-dirty, no-git-backup, inside-code-fence,
already-delegated, ambiguous-multi-match, block-is-callout
- execFileSync array args (no shell-injection surface)
- EOF newline preservation
- `gbrain doctor --fix` and `--dry-run` flags wire in via doctor.ts
- 31 new tests across dry-fix.test.ts (28 unit), check-resolvable.test.ts
(13 DRY detection + extraction), doctor-fix.test.ts (3 CLI integration)
* chore: bump version and changelog (v0.14.1)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs: update project documentation for v0.14.1
CLAUDE.md:
- Added src/core/dry-fix.ts entry under Key files (expanders, guards,
execFileSync safety, EOF newline preservation).
- Updated src/commands/doctor.ts entry to cover --fix/--dry-run flags.
- Updated src/core/check-resolvable.ts entry to reflect array-valued
CROSS_CUTTING_PATTERNS.conventions, extractDelegationTargets(), and
proximity-based DRY suppression via DRY_PROXIMITY_LINES = 40.
- Added test/dry-fix.test.ts and test/doctor-fix.test.ts to the test
list, and annotated test/check-resolvable.test.ts with v0.14.1 cases.
README.md:
- ADMIN block: --fix now names what it actually fixes (DRY violations
via conventions delegation) and documents --dry-run.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
@@ -2,6 +2,7 @@ import type { BrainEngine } from '../core/engine.ts';
|
||||
import * as db from '../core/db.ts';
|
||||
import { LATEST_VERSION } from '../core/migrate.ts';
|
||||
import { checkResolvable } from '../core/check-resolvable.ts';
|
||||
import { autoFixDryViolations, type AutoFixReport, type FixOutcome } from '../core/dry-fix.ts';
|
||||
import { loadCompletedMigrations } from '../core/preferences.ts';
|
||||
import { join } from 'path';
|
||||
import { existsSync, readFileSync, readdirSync } from 'fs';
|
||||
@@ -21,7 +22,10 @@ export interface Check {
|
||||
export async function runDoctor(engine: BrainEngine | null, args: string[]) {
|
||||
const jsonOutput = args.includes('--json');
|
||||
const fastMode = args.includes('--fast');
|
||||
const doFix = args.includes('--fix');
|
||||
const dryRun = args.includes('--dry-run');
|
||||
const checks: Check[] = [];
|
||||
let autoFixReport: AutoFixReport | null = null;
|
||||
|
||||
// --- Filesystem checks (always run, no DB needed) ---
|
||||
|
||||
@@ -29,6 +33,15 @@ export async function runDoctor(engine: BrainEngine | null, args: string[]) {
|
||||
const repoRoot = findRepoRoot();
|
||||
if (repoRoot) {
|
||||
const skillsDir = join(repoRoot, 'skills');
|
||||
|
||||
// --fix: run auto-repair BEFORE checkResolvable so the post-fix scan
|
||||
// reflects the new state. Auto-fix only targets DRY violations today;
|
||||
// other resolver issues are left to human repair.
|
||||
if (doFix) {
|
||||
autoFixReport = autoFixDryViolations(skillsDir, { dryRun });
|
||||
printAutoFixReport(autoFixReport, dryRun, jsonOutput);
|
||||
}
|
||||
|
||||
const report = checkResolvable(skillsDir);
|
||||
if (report.ok && report.issues.length === 0) {
|
||||
checks.push({
|
||||
@@ -351,6 +364,36 @@ export async function runDoctor(engine: BrainEngine | null, args: string[]) {
|
||||
// Helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** Print the auto-fix report in human-readable form. JSON output goes through
|
||||
* outputResults alongside the check list; this is the pretty-print path. */
|
||||
function printAutoFixReport(report: AutoFixReport, dryRun: boolean, jsonOutput: boolean): void {
|
||||
if (jsonOutput) return; // JSON consumers read autoFixReport via the check issues / caller
|
||||
const verb = dryRun ? 'PROPOSED' : 'APPLIED';
|
||||
for (const outcome of report.fixed) {
|
||||
console.log(`[${verb}] ${outcome.skillPath} (${outcome.patternLabel})`);
|
||||
if (outcome.before) {
|
||||
console.log('--- before');
|
||||
console.log(outcome.before);
|
||||
console.log('--- after');
|
||||
console.log(outcome.after ?? '');
|
||||
console.log('');
|
||||
}
|
||||
}
|
||||
const n = report.fixed.length;
|
||||
const s = report.skipped.length;
|
||||
if (n === 0 && s === 0) {
|
||||
console.log('Doctor --fix: no DRY violations to repair.');
|
||||
return;
|
||||
}
|
||||
const label = dryRun ? 'fixes proposed' : 'fixes applied';
|
||||
console.log(`${n} ${label}${s > 0 ? `, ${s} skipped:` : '.'}`);
|
||||
for (const sk of report.skipped) {
|
||||
const hint = sk.reason === 'working_tree_dirty' ? ' (run `git stash` first)' : '';
|
||||
console.log(` - ${sk.skillPath}: ${sk.reason}${hint}`);
|
||||
}
|
||||
if (dryRun && n > 0) console.log('\nRun without --dry-run to apply.');
|
||||
}
|
||||
|
||||
/** Find the GBrain repo root by walking up from cwd looking for skills/RESOLVER.md */
|
||||
function findRepoRoot(): string | null {
|
||||
let dir = process.cwd();
|
||||
|
||||
@@ -136,13 +136,67 @@ function extractTriggers(skillContent: string): string[] {
|
||||
.filter(Boolean);
|
||||
}
|
||||
|
||||
/** Scan for inlined cross-cutting rules that should reference convention files. */
|
||||
const CROSS_CUTTING_PATTERNS = [
|
||||
{ pattern: /iron\s*law.*back-?link/i, convention: 'conventions/quality.md', label: 'Iron Law back-linking' },
|
||||
{ pattern: /citation.*format.*\[Source:/i, convention: 'conventions/quality.md', label: 'citation format rules' },
|
||||
{ pattern: /notability.*gate/i, convention: 'conventions/quality.md', label: 'notability gate' },
|
||||
/**
|
||||
* Scan for inlined cross-cutting rules that should reference convention
|
||||
* files. Each pattern can list multiple valid delegation targets — e.g.,
|
||||
* notability rules live in both `conventions/quality.md` and
|
||||
* `_brain-filing-rules.md`, and referencing either counts as delegation.
|
||||
*/
|
||||
export interface CrossCuttingPattern {
|
||||
pattern: RegExp;
|
||||
conventions: string[];
|
||||
label: string;
|
||||
}
|
||||
|
||||
export const CROSS_CUTTING_PATTERNS: CrossCuttingPattern[] = [
|
||||
{ pattern: /iron\s*law.*back-?link/i,
|
||||
conventions: ['conventions/quality.md'],
|
||||
label: 'Iron Law back-linking' },
|
||||
{ pattern: /citation.*format.*\[Source:/i,
|
||||
conventions: ['conventions/quality.md'],
|
||||
label: 'citation format rules' },
|
||||
{ pattern: /notability.*gate/i,
|
||||
conventions: ['conventions/quality.md', '_brain-filing-rules.md'],
|
||||
label: 'notability gate' },
|
||||
];
|
||||
|
||||
/** Proximity window (lines) within which a delegation reference suppresses
|
||||
* a DRY match. Typical skill section is 20-30 lines; 40 covers header +
|
||||
* section without leaking across document-length files. */
|
||||
export const DRY_PROXIMITY_LINES = 40;
|
||||
|
||||
export interface DelegationRef {
|
||||
convention: string; // normalized relative path, e.g., 'conventions/quality.md'
|
||||
line: number; // 1-indexed line number of the reference
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract delegation references from skill content. Recognizes three shapes:
|
||||
* 1. `> **Convention:** ... \`skills/<path>\` ...`
|
||||
* 2. `> **Filing rule:** ... \`skills/<path>\` ...`
|
||||
* 3. Inline backtick `\`skills/conventions/*.md\`` or
|
||||
* `\`skills/_brain-filing-rules.md\``
|
||||
*
|
||||
* Paths are normalized by stripping the leading `skills/` so they match the
|
||||
* `conventions` field of CROSS_CUTTING_PATTERNS.
|
||||
*/
|
||||
export function extractDelegationTargets(content: string): DelegationRef[] {
|
||||
const refs: DelegationRef[] = [];
|
||||
const lines = content.split('\n');
|
||||
// Match backtick-wrapped skills/ paths that point at a known delegation
|
||||
// target. Scoped to conventions/ subtree and _brain-filing-rules.md.
|
||||
const pathRe = /`skills\/((?:conventions\/[^`]+\.md)|(?:_brain-filing-rules\.md))`/g;
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
pathRe.lastIndex = 0;
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = pathRe.exec(line)) !== null) {
|
||||
refs.push({ convention: m[1], line: i + 1 });
|
||||
}
|
||||
}
|
||||
return refs;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Main function
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -317,24 +371,34 @@ export function checkResolvable(skillsDir: string): ResolvableReport {
|
||||
}
|
||||
}
|
||||
|
||||
// 5. DRY detection — inlined cross-cutting rules
|
||||
// 5. DRY detection — inlined cross-cutting rules.
|
||||
// A match is suppressed when the skill references one of the pattern's
|
||||
// accepted convention files within DRY_PROXIMITY_LINES lines of the match.
|
||||
// This catches the common case where a skill delegates at a section
|
||||
// header but still contains prose mentioning the rule by name.
|
||||
for (const skill of manifest) {
|
||||
const skillPath = join(skillsDir, skill.path);
|
||||
if (!existsSync(skillPath)) continue;
|
||||
try {
|
||||
const content = readFileSync(skillPath, 'utf-8');
|
||||
for (const { pattern, convention, label } of CROSS_CUTTING_PATTERNS) {
|
||||
if (pattern.test(content)) {
|
||||
// Check if the skill also references the convention file
|
||||
if (!content.includes(convention)) {
|
||||
issues.push({
|
||||
type: 'dry_violation',
|
||||
severity: 'warning',
|
||||
skill: skill.name,
|
||||
message: `Skill '${skill.name}' inlines ${label} instead of referencing '${convention}'`,
|
||||
action: `Replace inlined rules with a reference to '${convention}'`,
|
||||
});
|
||||
}
|
||||
const delegations = extractDelegationTargets(content);
|
||||
for (const { pattern, conventions, label } of CROSS_CUTTING_PATTERNS) {
|
||||
const globalRe = new RegExp(pattern.source, pattern.flags.includes('g') ? pattern.flags : pattern.flags + 'g');
|
||||
const matches = [...content.matchAll(globalRe)];
|
||||
for (const m of matches) {
|
||||
const matchLine = content.slice(0, m.index ?? 0).split('\n').length;
|
||||
const suppressed = delegations.some(
|
||||
d => conventions.includes(d.convention) && Math.abs(d.line - matchLine) <= DRY_PROXIMITY_LINES
|
||||
);
|
||||
if (suppressed) continue;
|
||||
issues.push({
|
||||
type: 'dry_violation',
|
||||
severity: 'warning',
|
||||
skill: skill.name,
|
||||
message: `Skill '${skill.name}' inlines ${label} instead of delegating to a convention file`,
|
||||
action: `Replace inlined rules with a reference to one of: ${conventions.join(', ')}`,
|
||||
});
|
||||
break; // one issue per pattern per skill
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
@@ -354,3 +418,7 @@ export function checkResolvable(skillsDir: string): ResolvableReport {
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// Re-export auto-fix so callers have one canonical entry point.
|
||||
export { autoFixDryViolations } from './dry-fix.ts';
|
||||
export type { AutoFixOptions, AutoFixReport, FixOutcome } from './dry-fix.ts';
|
||||
|
||||
382
src/core/dry-fix.ts
Normal file
382
src/core/dry-fix.ts
Normal file
@@ -0,0 +1,382 @@
|
||||
/**
|
||||
* dry-fix.ts — Auto-repair DRY violations surfaced by checkResolvable().
|
||||
*
|
||||
* Called by `gbrain doctor --fix`. Scans every skill in the manifest, locates
|
||||
* matches of CROSS_CUTTING_PATTERNS, expands each match to its block
|
||||
* boundary, and replaces the block with a `> **Convention:** ...` reference
|
||||
* line. Writes are guarded:
|
||||
* - working-tree-dirty → skip (preserves git-as-backup contract)
|
||||
* - inside code fence → skip (don't mangle example prose)
|
||||
* - already delegated → skip (idempotent re-runs)
|
||||
* - multi-match → skip (ambiguous; manual edit required)
|
||||
*
|
||||
* Dry-run mode returns proposed edits without writing to disk.
|
||||
*/
|
||||
|
||||
import { readFileSync, writeFileSync, existsSync } from 'fs';
|
||||
import { join, dirname } from 'path';
|
||||
import { execFileSync } from 'child_process';
|
||||
import {
|
||||
CROSS_CUTTING_PATTERNS,
|
||||
DRY_PROXIMITY_LINES,
|
||||
extractDelegationTargets,
|
||||
type CrossCuttingPattern,
|
||||
} from './check-resolvable.ts';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Types
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export interface AutoFixOptions {
|
||||
dryRun?: boolean;
|
||||
}
|
||||
|
||||
export type FixStatus = 'applied' | 'proposed' | 'skipped' | 'error';
|
||||
|
||||
export type SkipReason =
|
||||
| 'working_tree_dirty'
|
||||
| 'no_git_backup'
|
||||
| 'inside_code_fence'
|
||||
| 'already_delegated'
|
||||
| 'ambiguous_multiple_matches'
|
||||
| 'block_is_callout'
|
||||
| 'file_missing'
|
||||
| 'read_error'
|
||||
| 'write_error';
|
||||
|
||||
export interface FixOutcome {
|
||||
skill: string;
|
||||
skillPath: string; // absolute
|
||||
patternLabel: string;
|
||||
status: FixStatus;
|
||||
reason?: SkipReason | string;
|
||||
before?: string; // snippet (the expanded block)
|
||||
after?: string; // replacement line
|
||||
}
|
||||
|
||||
export interface AutoFixReport {
|
||||
fixed: FixOutcome[]; // applied writes (or proposals in dryRun)
|
||||
skipped: FixOutcome[]; // skips and errors
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Block-expansion strategy map
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export type BlockShape = 'bullet' | 'blockquote' | 'paragraph';
|
||||
|
||||
export interface Block {
|
||||
startLine: number; // 0-indexed inclusive
|
||||
endLine: number; // 0-indexed inclusive
|
||||
}
|
||||
|
||||
/** Detect which block shape the line at `lineIdx` belongs to. */
|
||||
export function detectBlockShape(lines: string[], lineIdx: number): BlockShape {
|
||||
const line = lines[lineIdx] ?? '';
|
||||
if (/^(\s*)(?:[-*]\s|\d+\.\s)/.test(line)) return 'bullet';
|
||||
if (/^>\s/.test(line)) return 'blockquote';
|
||||
return 'paragraph';
|
||||
}
|
||||
|
||||
/** Expand a bullet item: start at the bullet line, end at the next sibling
|
||||
* or shallower bullet (sub-bullets included). */
|
||||
export function expandBullet(lines: string[], lineIdx: number): Block | null {
|
||||
const line = lines[lineIdx] ?? '';
|
||||
const indentMatch = line.match(/^(\s*)(?:[-*]\s|\d+\.\s)/);
|
||||
if (!indentMatch) return null;
|
||||
const baseIndent = indentMatch[1].length;
|
||||
|
||||
// Walk up to find the start of THIS bullet (in case match is on a
|
||||
// continuation line of a multi-line bullet).
|
||||
let start = lineIdx;
|
||||
while (start > 0) {
|
||||
const prev = lines[start - 1];
|
||||
const prevIsBullet = /^(\s*)(?:[-*]\s|\d+\.\s)/.test(prev);
|
||||
const prevIndent = prev.match(/^(\s*)/)?.[1].length ?? 0;
|
||||
if (prevIsBullet && prevIndent <= baseIndent) break;
|
||||
if (prev.trim() === '') break;
|
||||
start--;
|
||||
}
|
||||
|
||||
// Walk down: continue until a bullet at <= baseIndent (sibling or
|
||||
// shallower), a blank line, or end of file.
|
||||
let end = lineIdx;
|
||||
for (let i = lineIdx + 1; i < lines.length; i++) {
|
||||
const l = lines[i];
|
||||
if (l.trim() === '') break;
|
||||
const isBullet = /^(\s*)(?:[-*]\s|\d+\.\s)/.test(l);
|
||||
const indent = l.match(/^(\s*)/)?.[1].length ?? 0;
|
||||
if (isBullet && indent <= baseIndent) break;
|
||||
end = i;
|
||||
}
|
||||
return { startLine: start, endLine: end };
|
||||
}
|
||||
|
||||
/** Expand a blockquote: contiguous `>` lines. Returns null if the block is
|
||||
* itself a `> **Convention:**` or `> **Filing rule:**` callout (don't
|
||||
* rewrite a reference into a reference). */
|
||||
export function expandBlockquote(lines: string[], lineIdx: number): Block | null {
|
||||
if (!/^>\s/.test(lines[lineIdx] ?? '')) return null;
|
||||
let start = lineIdx;
|
||||
while (start > 0 && /^>\s/.test(lines[start - 1])) start--;
|
||||
let end = lineIdx;
|
||||
while (end + 1 < lines.length && /^>\s/.test(lines[end + 1])) end++;
|
||||
|
||||
const firstLine = lines[start] ?? '';
|
||||
if (/\*\*(?:Convention|Filing rule):\*\*/.test(firstLine)) {
|
||||
return null; // this IS a delegation callout already
|
||||
}
|
||||
return { startLine: start, endLine: end };
|
||||
}
|
||||
|
||||
/** Expand a paragraph: previous blank line → next blank line. */
|
||||
export function expandParagraph(lines: string[], lineIdx: number): Block | null {
|
||||
let start = lineIdx;
|
||||
while (start > 0 && lines[start - 1].trim() !== '') start--;
|
||||
let end = lineIdx;
|
||||
while (end + 1 < lines.length && lines[end + 1].trim() !== '') end++;
|
||||
return { startLine: start, endLine: end };
|
||||
}
|
||||
|
||||
export const expanders: Record<BlockShape, (lines: string[], lineIdx: number) => Block | null> = {
|
||||
bullet: expandBullet,
|
||||
blockquote: expandBlockquote,
|
||||
paragraph: expandParagraph,
|
||||
};
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Guards
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** True when the match offset sits inside a fenced code block (``` ... ```).
|
||||
* Counts triple-backtick fences at line starts. Odd count = inside. */
|
||||
export function isInsideCodeFence(content: string, offset: number): boolean {
|
||||
const before = content.slice(0, offset);
|
||||
const fenceRe = /^```/gm;
|
||||
const fenceCount = (before.match(fenceRe) || []).length;
|
||||
return fenceCount % 2 === 1;
|
||||
}
|
||||
|
||||
export type WorkingTreeStatus = 'clean' | 'dirty' | 'not_a_repo';
|
||||
|
||||
/** Check the git state of a skill file. Three distinct outcomes — callers
|
||||
* must NOT conflate "not a repo" with "clean", because the auto-fix
|
||||
* contract is "git is the backup" and writing to a file outside any repo
|
||||
* destroys user data with no recovery path.
|
||||
*
|
||||
* `execFileSync` with array args bypasses the shell entirely, so paths
|
||||
* with odd characters from a manifest can't inject commands. */
|
||||
export function getWorkingTreeStatus(skillPath: string): WorkingTreeStatus {
|
||||
try {
|
||||
const out = execFileSync('git', ['status', '--porcelain', '--', skillPath], {
|
||||
encoding: 'utf-8',
|
||||
stdio: ['ignore', 'pipe', 'ignore'],
|
||||
cwd: dirname(skillPath),
|
||||
});
|
||||
return out.trim().length > 0 ? 'dirty' : 'clean';
|
||||
} catch {
|
||||
// git exits 128 when not inside a repo; treat any non-zero the same.
|
||||
return 'not_a_repo';
|
||||
}
|
||||
}
|
||||
|
||||
/** Legacy wrapper. Callers that need to distinguish not_a_repo from clean
|
||||
* should use getWorkingTreeStatus() directly. */
|
||||
export function isWorkingTreeDirty(skillPath: string): boolean {
|
||||
return getWorkingTreeStatus(skillPath) === 'dirty';
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Manifest loading (duplicated from check-resolvable.ts to avoid exporting
|
||||
// that internal helper — kept in sync via tests)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
interface ManifestEntry {
|
||||
name: string;
|
||||
path: string;
|
||||
}
|
||||
|
||||
function loadManifest(skillsDir: string): ManifestEntry[] {
|
||||
const manifestPath = join(skillsDir, 'manifest.json');
|
||||
if (!existsSync(manifestPath)) return [];
|
||||
try {
|
||||
const content = JSON.parse(readFileSync(manifestPath, 'utf-8'));
|
||||
return content.skills || [];
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Main function
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Auto-repair DRY violations across every skill in the manifest.
|
||||
*
|
||||
* @param skillsDir — path to the `skills/` directory
|
||||
* @param opts.dryRun — if true, do not write; return proposed edits
|
||||
*/
|
||||
export function autoFixDryViolations(
|
||||
skillsDir: string,
|
||||
opts: AutoFixOptions = {}
|
||||
): AutoFixReport {
|
||||
const fixed: FixOutcome[] = [];
|
||||
const skipped: FixOutcome[] = [];
|
||||
const manifest = loadManifest(skillsDir);
|
||||
|
||||
for (const skill of manifest) {
|
||||
const skillPath = join(skillsDir, skill.path);
|
||||
if (!existsSync(skillPath)) {
|
||||
// Manifest-present but file-missing is already reported by
|
||||
// checkResolvable as 'missing_file'; don't double-report here.
|
||||
continue;
|
||||
}
|
||||
|
||||
let content: string;
|
||||
try {
|
||||
content = readFileSync(skillPath, 'utf-8');
|
||||
} catch (e: any) {
|
||||
skipped.push({
|
||||
skill: skill.name,
|
||||
skillPath,
|
||||
patternLabel: '(all)',
|
||||
status: 'error',
|
||||
reason: 'read_error',
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
// Compute delegations fresh per pattern — a prior applied fix inserts
|
||||
// a new Convention callout that should inform later patterns'
|
||||
// idempotency checks.
|
||||
let delegations = extractDelegationTargets(content);
|
||||
|
||||
for (const cut of CROSS_CUTTING_PATTERNS) {
|
||||
const outcome = attemptFix(skill.name, skillPath, content, delegations, cut, opts);
|
||||
if (!outcome) continue;
|
||||
if (outcome.status === 'applied' || outcome.status === 'proposed') {
|
||||
fixed.push(outcome);
|
||||
if (outcome.status === 'applied') {
|
||||
try {
|
||||
content = readFileSync(skillPath, 'utf-8');
|
||||
delegations = extractDelegationTargets(content);
|
||||
} catch {
|
||||
break;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
skipped.push(outcome);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return { fixed, skipped };
|
||||
}
|
||||
|
||||
function attemptFix(
|
||||
skillName: string,
|
||||
skillPath: string,
|
||||
content: string,
|
||||
delegations: ReturnType<typeof extractDelegationTargets>,
|
||||
cut: CrossCuttingPattern,
|
||||
opts: AutoFixOptions
|
||||
): FixOutcome | null {
|
||||
const base = {
|
||||
skill: skillName,
|
||||
skillPath,
|
||||
patternLabel: cut.label,
|
||||
};
|
||||
|
||||
// Find ALL matches first (for multi-match detection).
|
||||
const globalRe = new RegExp(
|
||||
cut.pattern.source,
|
||||
cut.pattern.flags.includes('g') ? cut.pattern.flags : cut.pattern.flags + 'g'
|
||||
);
|
||||
const matches = [...content.matchAll(globalRe)];
|
||||
if (matches.length === 0) return null;
|
||||
|
||||
if (matches.length > 1) {
|
||||
return { ...base, status: 'skipped', reason: 'ambiguous_multiple_matches' };
|
||||
}
|
||||
|
||||
const m = matches[0];
|
||||
const offset = m.index ?? 0;
|
||||
|
||||
if (isInsideCodeFence(content, offset)) {
|
||||
return { ...base, status: 'skipped', reason: 'inside_code_fence' };
|
||||
}
|
||||
|
||||
// Compute match line (1-indexed) to evaluate idempotency.
|
||||
// Use the same proximity window as the detector (DRY_PROXIMITY_LINES)
|
||||
// so the fixer can't re-fire on blocks the detector already suppresses.
|
||||
const matchLine = content.slice(0, offset).split('\n').length;
|
||||
const alreadyDelegated = delegations.some(
|
||||
d => cut.conventions.includes(d.convention) && Math.abs(d.line - matchLine) <= DRY_PROXIMITY_LINES
|
||||
);
|
||||
if (alreadyDelegated) {
|
||||
return { ...base, status: 'skipped', reason: 'already_delegated' };
|
||||
}
|
||||
|
||||
const treeStatus = getWorkingTreeStatus(skillPath);
|
||||
if (treeStatus === 'dirty') {
|
||||
return { ...base, status: 'skipped', reason: 'working_tree_dirty' };
|
||||
}
|
||||
if (treeStatus === 'not_a_repo') {
|
||||
// File isn't tracked by git — writing would destroy the user's only
|
||||
// copy with no rollback path. Refuse.
|
||||
return { ...base, status: 'skipped', reason: 'no_git_backup' };
|
||||
}
|
||||
|
||||
// Expand to block boundary.
|
||||
const lines = content.split('\n');
|
||||
const lineIdx = matchLine - 1; // 0-indexed
|
||||
const shape = detectBlockShape(lines, lineIdx);
|
||||
const expander = expanders[shape];
|
||||
const block = expander(lines, lineIdx);
|
||||
if (!block) {
|
||||
return { ...base, status: 'skipped', reason: 'block_is_callout' };
|
||||
}
|
||||
|
||||
// Build replacement line.
|
||||
const canonical = cut.conventions[0];
|
||||
const replacement = `> **Convention:** See \`skills/${canonical}\` for ${cut.label}.`;
|
||||
|
||||
// Splice: replace lines[startLine..endLine] with [replacement].
|
||||
const before = lines.slice(0, block.startLine).join('\n');
|
||||
const originalBlock = lines.slice(block.startLine, block.endLine + 1).join('\n');
|
||||
const after = lines.slice(block.endLine + 1).join('\n');
|
||||
|
||||
// Preserve structure: one newline between sections, preserve the file's
|
||||
// trailing newline if the original had one (POSIX convention).
|
||||
const parts: string[] = [];
|
||||
if (before.length > 0) parts.push(before);
|
||||
parts.push(replacement);
|
||||
if (after.length > 0) parts.push(after);
|
||||
let next = parts.join('\n');
|
||||
if (content.endsWith('\n') && !next.endsWith('\n')) {
|
||||
next += '\n';
|
||||
}
|
||||
|
||||
if (opts.dryRun) {
|
||||
return {
|
||||
...base,
|
||||
status: 'proposed',
|
||||
before: originalBlock,
|
||||
after: replacement,
|
||||
};
|
||||
}
|
||||
|
||||
try {
|
||||
writeFileSync(skillPath, next, 'utf-8');
|
||||
} catch {
|
||||
return { ...base, status: 'error', reason: 'write_error' };
|
||||
}
|
||||
|
||||
return {
|
||||
...base,
|
||||
status: 'applied',
|
||||
before: originalBlock,
|
||||
after: replacement,
|
||||
};
|
||||
}
|
||||
Reference in New Issue
Block a user