feat(code-review): integrate fallow structural pre-pass for /gsd-code-review (#3424)

* feat(code-review): add optional fallow structural pre-pass

* fix(ci): sync lockfile for fallow optional binaries

* fix(test): make fallow integration tests cross-platform

* fix(review): require executable fallow binary paths

* docs(review): clarify structural findings usage and size guard

* fix(fallow): preserve line:0, prefer node_modules/.bin, sync SDK twin (H1, M2, N1 from #3424 review)

* fix(workflow): harden fallow pre-pass — exit check, timeout, atomic write, size-guard order (B1, H2-H4, M1, M3 from #3424 review)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(deps): pin fallow floor to ^2.70.0 matching lockfile (H7 from #3424 review)

* fix(config): enum-validate fallow.scope/profile + group code_quality.* contiguously (H5, N3 from #3424 review)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs(fallow): label mcp gate reserved, version-pin install, expand context schema (B3, H8, M4, M8, L1 from #3424 review)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(fallow): replace source-grep with behavioral tests, expand fixtures, fail-loud tmpdir (B4, H6, L2, L3, M5, M6, N2 from #3424 review)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(workflow): escape closing structural_findings tag in JSON payload (CR #3424 inline finding)

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-13 21:19:47 -04:00
committed by GitHub
parent 1e091d2bcb
commit 75d5ca5875
20 changed files with 1131 additions and 2 deletions

View File

@@ -45,6 +45,10 @@ export const VALID_CONFIG_KEYS: ReadonlySet<string> = new Set([
'workflow.security_block_on',
'workflow.drift_threshold',
'workflow.drift_action',
'code_quality.fallow.enabled',
'code_quality.fallow.scope',
'code_quality.fallow.profile',
'code_quality.fallow.mcp',
'ship.pr_body_sections',
'git.branching_strategy', 'git.base_branch', 'git.phase_branch_template', 'git.milestone_branch_template', 'git.quick_branch_template',
'planning.commit_docs', 'planning.search_gitignored', 'planning.sub_repos',

View File

@@ -0,0 +1,88 @@
export interface FallowUnusedExport {
file?: string;
symbol?: string;
line?: number | null;
}
export interface FallowDuplicateBlock {
left?: { file?: string; start?: number | null; end?: number | null };
right?: { file?: string; start?: number | null; end?: number | null };
similarity?: number;
}
export interface FallowCircularDependency {
cycle?: string[];
}
export interface FallowReport {
unusedExports?: FallowUnusedExport[];
duplicates?: FallowDuplicateBlock[];
circularDependencies?: FallowCircularDependency[];
}
export interface NormalizedFallowFinding {
type: 'unused_export' | 'duplicate_block' | 'circular_dependency';
message: string;
file: string;
line: number | null;
related_file?: string;
}
export interface NormalizedFallowReport {
summary: {
unused_exports: number;
duplicates: number;
circular_dependencies: number;
total: number;
};
findings: NormalizedFallowFinding[];
}
// TODO(parity): SDK lacks a TS test harness for normalizeFallowReport. CJS parity tests live in
// tests/feat-3210-fallow-integration.test.cjs (H1 suite). When a TS test runner is added,
// mirror those cases here — especially the line:0 preservation assertions.
export function normalizeFallowReport(report: FallowReport | null | undefined): NormalizedFallowReport {
const unused = Array.isArray(report?.unusedExports) ? report.unusedExports : [];
const duplicates = Array.isArray(report?.duplicates) ? report.duplicates : [];
const circular = Array.isArray(report?.circularDependencies) ? report.circularDependencies : [];
const findings: NormalizedFallowFinding[] = [];
for (const item of unused) {
findings.push({
type: 'unused_export',
message: `Unused export ${item.symbol || '<unknown>'}`,
file: item.file || '',
line: item.line ?? null,
});
}
for (const item of duplicates) {
findings.push({
type: 'duplicate_block',
message: `Duplicate block (${Math.round((item.similarity || 0) * 100)}% similarity)`,
file: item.left?.file || '',
line: item.left?.start ?? null,
related_file: item.right?.file || '',
});
}
for (const item of circular) {
findings.push({
type: 'circular_dependency',
message: `Circular dependency: ${(item.cycle || []).join(' -> ')}`,
file: Array.isArray(item.cycle) && item.cycle.length > 0 ? item.cycle[0] : '',
line: null,
});
}
return {
summary: {
unused_exports: unused.length,
duplicates: duplicates.length,
circular_dependencies: circular.length,
total: findings.length,
},
findings,
};
}