diff --git a/electron/ipc/channel-manifest.json b/electron/ipc/channel-manifest.json index 1d9316aa..8c867272 100644 --- a/electron/ipc/channel-manifest.json +++ b/electron/ipc/channel-manifest.json @@ -112,6 +112,7 @@ "DetectPrForBranch": "detect_pr_for_branch", "RefreshPrChecksWatcher": "refresh_pr_checks_watcher", "PrChecksUpdate": "pr_checks_update", + "GetEslintQualityFindings": "get_eslint_quality_findings", "LogFromRenderer": "log_from_renderer", "CheckForUpdates": "check_for_updates", "DownloadUpdate": "download_update", diff --git a/electron/ipc/eslint-quality-findings.test.ts b/electron/ipc/eslint-quality-findings.test.ts new file mode 100644 index 00000000..5342ba3d --- /dev/null +++ b/electron/ipc/eslint-quality-findings.test.ts @@ -0,0 +1,159 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { describe, expect, it } from 'vitest'; +import { + classifyEslintError, + isLintablePath, + loadEslintQualityFindings, + parseEslintFindings, +} from './eslint-quality-findings.js'; + +describe('ESLint quality findings', () => { + it('parses only changed lintable files and maps rule messages', () => { + const worktreePath = '/tmp/project'; + const stdout = JSON.stringify([ + { + filePath: '/tmp/project/src/a.ts', + messages: [ + { + ruleId: '@typescript-eslint/no-explicit-any', + severity: 2, + message: 'Unexpected any.', + line: 4, + column: 7, + endLine: 4, + endColumn: 10, + }, + { ruleId: null, severity: 2, message: 'Parsing error', line: 1, column: 1 }, + ], + }, + { + filePath: '/tmp/project/README.md', + messages: [{ ruleId: 'markdown/rule', severity: 2, message: 'ignored', line: 1 }], + }, + ]); + + expect(parseEslintFindings(stdout, worktreePath, ['src/a.ts', 'README.md'])).toEqual([ + { + id: 'eslint:src/a.ts:4:7:@typescript-eslint/no-explicit-any', + source: 'eslint', + ruleId: '@typescript-eslint/no-explicit-any', + category: 'maintainability', + severity: 'error', + location: { + filePath: 'src/a.ts', + startLine: 4, + startColumn: 7, + endLine: 4, + endColumn: 10, + }, + explanation: 'Unexpected any.', + }, + ]); + }); + + it('recognizes supported JavaScript and TypeScript paths', () => { + expect(isLintablePath('src/a.ts')).toBe(true); + expect(isLintablePath('src/a.tsx')).toBe(true); + expect(isLintablePath('src/a.js')).toBe(true); + expect(isLintablePath('README.md')).toBe(false); + }); + + it('silently skips projects without an ESLint config', async () => { + const worktreePath = fs.mkdtempSync(path.join(os.tmpdir(), 'parallel-code-eslint-')); + try { + await expect(loadEslintQualityFindings(worktreePath, ['src/a.ts'])).resolves.toEqual({ + status: 'not-applicable', + }); + } finally { + fs.rmSync(worktreePath, { recursive: true, force: true }); + } + }); + + it('silently skips legacy ESLint configs that ESLint 9 does not load', async () => { + const worktreePath = fs.mkdtempSync(path.join(os.tmpdir(), 'parallel-code-eslint-')); + try { + fs.writeFileSync(path.join(worktreePath, '.eslintrc.json'), '{}'); + await expect(loadEslintQualityFindings(worktreePath, ['src/a.ts'])).resolves.toEqual({ + status: 'not-applicable', + }); + } finally { + fs.rmSync(worktreePath, { recursive: true, force: true }); + } + }); + + it('silently skips configured projects without a local ESLint binary', async () => { + const worktreePath = fs.mkdtempSync(path.join(os.tmpdir(), 'parallel-code-eslint-')); + try { + fs.writeFileSync(path.join(worktreePath, 'eslint.config.js'), 'export default [];'); + await expect(loadEslintQualityFindings(worktreePath, ['src/a.ts'])).resolves.toEqual({ + status: 'not-applicable', + }); + } finally { + fs.rmSync(worktreePath, { recursive: true, force: true }); + } + }); + + it('runs the local binary with a path separator and parses its output', async () => { + const worktreePath = fs.mkdtempSync(path.join(os.tmpdir(), 'parallel-code-eslint-')); + try { + const binaryPath = path.join(worktreePath, 'node_modules', '.bin'); + fs.mkdirSync(binaryPath, { recursive: true }); + fs.writeFileSync(path.join(worktreePath, 'eslint.config.js'), 'export default [];'); + fs.writeFileSync(path.join(binaryPath, 'eslint'), ''); + const calls: Array<{ file: string; args: string[]; cwd: string }> = []; + const execImpl = async ( + file: string, + args: string[], + options: { cwd: string; timeout: number; maxBuffer: number }, + ) => { + calls.push({ file, args, cwd: options.cwd }); + return { + stdout: JSON.stringify([ + { + filePath: path.join(worktreePath, 'src/a.ts'), + messages: [ + { + ruleId: 'no-console', + severity: 1, + message: 'Unexpected console statement.', + line: 2, + }, + ], + }, + ]), + stderr: '', + }; + }; + + await expect( + loadEslintQualityFindings(worktreePath, ['src/a.ts'], execImpl), + ).resolves.toEqual({ + status: 'available', + findings: [ + expect.objectContaining({ + ruleId: 'no-console', + severity: 'warning', + }), + ], + }); + expect(calls).toEqual([ + expect.objectContaining({ + file: path.join(worktreePath, 'node_modules', '.bin', 'eslint'), + args: ['--format', 'json', '--no-error-on-unmatched-pattern', '--', 'src/a.ts'], + cwd: worktreePath, + }), + ]); + } finally { + fs.rmSync(worktreePath, { recursive: true, force: true }); + } + }); + + it('maps missing executable and unsupported flat-config errors to not-applicable', () => { + expect(classifyEslintError({ code: 'ENOENT' })).toEqual({ status: 'not-applicable' }); + expect( + classifyEslintError({ stderr: "ESLint couldn't find an eslint.config.(js|mjs|cjs) file." }), + ).toEqual({ status: 'not-applicable' }); + }); +}); diff --git a/electron/ipc/eslint-quality-findings.ts b/electron/ipc/eslint-quality-findings.ts new file mode 100644 index 00000000..afa79a18 --- /dev/null +++ b/electron/ipc/eslint-quality-findings.ts @@ -0,0 +1,243 @@ +import { execFile } from 'child_process'; +import fs from 'fs'; +import path from 'path'; +import { promisify } from 'util'; +import type { EslintQualityFinding, EslintQualityResult } from './shared-types.js'; + +type EslintExec = ( + file: string, + args: string[], + options: { cwd: string; timeout: number; maxBuffer: number }, +) => Promise<{ stdout: string; stderr: string }>; + +const exec: EslintExec = async (file, args, options) => { + const result = await promisify(execFile)(file, args, options); + return { + stdout: String(result.stdout), + stderr: String(result.stderr), + }; +}; +const ESLINT_TIMEOUT_MS = 30_000; +const ESLINT_MAX_BUFFER = 8 * 1024 * 1024; +const ESLINT_BINARY_SEARCH_DEPTH = 3; +const LINTABLE_EXTENSIONS = new Set(['.cjs', '.cts', '.js', '.jsx', '.mjs', '.mts', '.ts', '.tsx']); +const ESLINT_CONFIG_NAMES = [ + 'eslint.config.js', + 'eslint.config.cjs', + 'eslint.config.mjs', + 'eslint.config.ts', + 'eslint.config.cts', + 'eslint.config.mts', +] as const; + +interface EslintMessage { + ruleId?: unknown; + severity?: unknown; + message?: unknown; + line?: unknown; + column?: unknown; + endLine?: unknown; + endColumn?: unknown; +} + +interface EslintFileResult { + filePath?: unknown; + messages?: unknown; +} + +function record(value: unknown): Record | null { + return value !== null && typeof value === 'object' ? (value as Record) : null; +} + +function positiveInteger(value: unknown): number | undefined { + return Number.isInteger(value) && Number(value) > 0 ? Number(value) : undefined; +} + +function nonEmptyString(value: unknown): string | undefined { + return typeof value === 'string' && value.trim() ? value.trim() : undefined; +} + +function normalizePath(filePath: string): string { + return filePath.split(path.sep).join('/'); +} + +function parseJson(stdout: string): unknown[] { + if (!stdout.trim()) return []; + let parsed: unknown; + try { + parsed = JSON.parse(stdout); + } catch { + throw new Error('ESLint returned malformed JSON output.'); + } + if (!Array.isArray(parsed)) throw new Error('ESLint returned an unexpected JSON shape.'); + return parsed; +} + +function parseFilePath(rawPath: unknown, worktreePath: string): string | undefined { + const filePath = nonEmptyString(rawPath); + if (!filePath) return undefined; + const absolutePath = path.isAbsolute(filePath) ? filePath : path.resolve(worktreePath, filePath); + const relativePath = path.relative(worktreePath, absolutePath); + if (!relativePath || relativePath.startsWith('..') || path.isAbsolute(relativePath)) { + return undefined; + } + return normalizePath(relativePath); +} + +function parseMessage(filePath: string, rawMessage: unknown): EslintQualityFinding | undefined { + const message = record(rawMessage) as EslintMessage | null; + const ruleId = nonEmptyString(message?.ruleId); + const explanation = nonEmptyString(message?.message); + const startLine = positiveInteger(message?.line); + const severity = + message?.severity === 2 ? 'error' : message?.severity === 1 ? 'warning' : undefined; + + // ESLint parse errors have no ruleId. The issue mapping intentionally leaves + // those out because they do not identify a stable rule-backed finding. + if (!ruleId || !explanation || !startLine || !severity) return undefined; + + const startColumn = positiveInteger(message?.column); + const endLine = positiveInteger(message?.endLine); + const endColumn = positiveInteger(message?.endColumn); + return { + id: `eslint:${filePath}:${startLine}:${startColumn ?? 0}:${ruleId}`, + source: 'eslint', + ruleId, + category: 'maintainability', + severity, + location: { + filePath, + startLine, + ...(startColumn ? { startColumn } : {}), + ...(endLine ? { endLine } : {}), + ...(endColumn ? { endColumn } : {}), + }, + explanation, + }; +} + +export function isLintablePath(filePath: string): boolean { + return LINTABLE_EXTENSIONS.has(path.extname(filePath).toLowerCase()); +} + +export function parseEslintFindings( + stdout: string, + worktreePath: string, + changedPaths: readonly string[], +): EslintQualityFinding[] { + const changed = new Set( + changedPaths.filter(isLintablePath).map((filePath) => normalizePath(filePath)), + ); + const findings: EslintQualityFinding[] = []; + for (const rawFile of parseJson(stdout)) { + const file = record(rawFile) as EslintFileResult | null; + const filePath = parseFilePath(file?.filePath, worktreePath); + if (!filePath || !changed.has(filePath) || !Array.isArray(file?.messages)) continue; + for (const rawMessage of file.messages) { + const finding = parseMessage(filePath, rawMessage); + if (finding) findings.push(finding); + } + } + return findings; +} + +function hasEslintConfig(worktreePath: string): boolean { + if (ESLINT_CONFIG_NAMES.some((name) => fs.existsSync(path.join(worktreePath, name)))) { + return true; + } + try { + const packageJson = JSON.parse( + fs.readFileSync(path.join(worktreePath, 'package.json'), 'utf8'), + ) as { + eslintConfig?: unknown; + }; + return record(packageJson.eslintConfig) !== null; + } catch { + return false; + } +} + +function findLocalEslintBinary(worktreePath: string): string | undefined { + let currentPath = path.resolve(worktreePath); + for (let depth = 0; depth <= ESLINT_BINARY_SEARCH_DEPTH; depth += 1) { + const binaryNames = + process.platform === 'win32' ? ['eslint.cmd', 'eslint.exe', 'eslint'] : ['eslint']; + for (const binaryName of binaryNames) { + const binaryPath = path.join(currentPath, 'node_modules', '.bin', binaryName); + if (fs.existsSync(binaryPath)) return binaryPath; + } + const parentPath = path.dirname(currentPath); + if (parentPath === currentPath) break; + currentPath = parentPath; + } + return undefined; +} + +function errorText(error: unknown): string { + const value = record(error); + return [value?.message, value?.stderr, value?.stdout] + .filter((part): part is string => typeof part === 'string') + .join('\n'); +} + +function unavailable(message: string): EslintQualityResult { + return { status: 'unavailable', message }; +} + +export function classifyEslintError(error: unknown): EslintQualityResult { + const code = (error as NodeJS.ErrnoException | undefined)?.code; + const text = errorText(error); + if ( + code === 'ENOENT' || + /eslint.*not found|could not determine executable|no such file|couldn't find an eslint\.config\./i.test( + text, + ) + ) { + return { status: 'not-applicable' }; + } + if (/configuration.*invalid|failed to load config|cannot find module/i.test(text)) { + return unavailable('ESLint could not load this project configuration.'); + } + return unavailable( + 'ESLint findings could not be loaded. Try again after checking the project lint command.', + ); +} + +export async function loadEslintQualityFindings( + worktreePath: string, + changedPaths: readonly string[], + execImpl: EslintExec = exec, +): Promise { + if (!hasEslintConfig(worktreePath)) return { status: 'not-applicable' }; + const lintablePaths = changedPaths.filter(isLintablePath); + if (lintablePaths.length === 0) return { status: 'available', findings: [] }; + const eslintBinary = findLocalEslintBinary(worktreePath); + if (!eslintBinary) return { status: 'not-applicable' }; + + try { + const { stdout } = await execImpl( + eslintBinary, + ['--format', 'json', '--no-error-on-unmatched-pattern', '--', ...lintablePaths], + { cwd: worktreePath, timeout: ESLINT_TIMEOUT_MS, maxBuffer: ESLINT_MAX_BUFFER }, + ); + return { + status: 'available', + findings: parseEslintFindings(stdout, worktreePath, lintablePaths), + }; + } catch (error) { + const stdout = (error as { stdout?: unknown })?.stdout; + if (typeof stdout === 'string' && stdout.trim()) { + try { + return { + status: 'available', + findings: parseEslintFindings(stdout, worktreePath, lintablePaths), + }; + } catch (parseError) { + return unavailable( + parseError instanceof Error ? parseError.message : 'ESLint output could not be parsed.', + ); + } + } + return classifyEslintError(error); + } +} diff --git a/electron/ipc/register.ts b/electron/ipc/register.ts index ab8d213f..da52433f 100644 --- a/electron/ipc/register.ts +++ b/electron/ipc/register.ts @@ -38,6 +38,7 @@ import { isPrUrl, } from './pr-checks.js'; import { readCoverageSummary } from './coverage.js'; +import { loadEslintQualityFindings } from './eslint-quality-findings.js'; import { startRemoteServer, getMCPLogs, type RemoteProject } from '../remote/server.js'; import type { RemoteAttentionState } from '../remote/protocol.js'; import { atomicWriteFileSync } from '../mcp/atomic.js'; @@ -880,6 +881,14 @@ export function registerAllHandlers(win: BrowserWindow): void { refreshPrChecksWatcher(args.taskId); }); + // --- Local ESLint quality findings --- + ipcMain.handle(IPC.GetEslintQualityFindings, (_e, args) => { + validatePath(args.worktreePath, 'worktreePath'); + assertStringArray(args.filePaths, 'filePaths'); + for (const filePath of args.filePaths) validateRelativePath(filePath, 'filePath'); + return loadEslintQualityFindings(args.worktreePath, args.filePaths); + }); + // --- Steps content (one-shot read) --- ipcMain.handle(IPC.ReadStepsContent, (_e, args) => { validatePath(args.worktreePath, 'worktreePath'); diff --git a/electron/ipc/shared-types.ts b/electron/ipc/shared-types.ts index 616a334e..f458121e 100644 --- a/electron/ipc/shared-types.ts +++ b/electron/ipc/shared-types.ts @@ -136,6 +136,27 @@ export interface BranchPrDetectionResult { unavailable?: 'missing' | 'auth'; } +export interface EslintQualityFinding { + id: string; + source: 'eslint'; + ruleId: string; + category: 'maintainability'; + severity: 'error' | 'warning'; + location: { + filePath: string; + startLine: number; + startColumn?: number; + endLine?: number; + endColumn?: number; + }; + explanation: string; +} + +export type EslintQualityResult = + | { status: 'available'; findings: EslintQualityFinding[] } + | { status: 'not-applicable' } + | { status: 'unavailable'; message: string }; + export interface StepEntry { summary: string; detail?: string; diff --git a/electron/preload.cjs b/electron/preload.cjs index 1ce97c81..11de5023 100644 --- a/electron/preload.cjs +++ b/electron/preload.cjs @@ -116,6 +116,7 @@ const ALLOWED_CHANNELS = new Set([ 'detect_pr_for_branch', 'refresh_pr_checks_watcher', 'pr_checks_update', + 'get_eslint_quality_findings', 'log_from_renderer', 'check_for_updates', 'download_update', diff --git a/src/components/DiffViewerDialog.tsx b/src/components/DiffViewerDialog.tsx index 2184481f..c92c919f 100644 --- a/src/components/DiffViewerDialog.tsx +++ b/src/components/DiffViewerDialog.tsx @@ -21,7 +21,11 @@ import { isCommitHashSelection, isUncommittedSelection, } from './CommitNavBar'; -import { ReviewCommentsButton, ReviewSidebarPanel } from './ReviewSidebarPanel'; +import { + ReviewCommentsButton, + ReviewFindingsRefreshButton, + ReviewSidebarPanel, +} from './ReviewSidebarPanel'; import { ReviewProvider, useReview } from './ReviewProvider'; import { ChangedFilesList } from './ChangedFilesList'; import { CloseIcon } from './icons'; @@ -127,6 +131,7 @@ export function DiffViewerDialog(props: DiffViewerDialogProps) { selectedCommit={props.selectedCommit} onCommitNavigate={props.onCommitNavigate} gitIsolation={props.gitIsolation} + findingProvider={props.findingProvider} /> @@ -386,6 +391,9 @@ function DiffViewerContent(props: DiffViewerDialogProps) { + + + diff --git a/src/components/ReviewProvider.tsx b/src/components/ReviewProvider.tsx index 04c378c9..82b5baac 100644 --- a/src/components/ReviewProvider.tsx +++ b/src/components/ReviewProvider.tsx @@ -72,6 +72,7 @@ export interface ReviewContextValue { findingsLoading: () => boolean; findingsError: () => string; clearFindingsError: () => void; + refreshFindings: () => void; beginDiffLoad: () => void; completeDiffLoad: (diffIdentity: string, files: FileDiff[]) => void; @@ -333,6 +334,17 @@ export function ReviewProvider(props: ReviewProviderProps) { }); } + function refreshFindings() { + const current = activeReviewDiff; + if (!current || !props.findingProvider) return; + invalidateFindingLoad(); + setFindings([]); + setSelectedFindingIds(new Set()); + findingsLoadedFor = null; + findingsLoadedProvider = undefined; + loadFindingsForDiff(current, current.files); + } + function completeDiffLoad(diffIdentity: string, files: FileDiff[]) { const next: ReviewDiffSnapshot = { reviewIdentity: props.reviewIdentity ?? '', @@ -495,6 +507,7 @@ export function ReviewProvider(props: ReviewProviderProps) { findingsLoading, findingsError, clearFindingsError: () => setFindingsError(''), + refreshFindings, beginDiffLoad, completeDiffLoad, suspendDiffLoad, diff --git a/src/components/ReviewSidebarPanel.tsx b/src/components/ReviewSidebarPanel.tsx index c2cec042..5a7dd881 100644 --- a/src/components/ReviewSidebarPanel.tsx +++ b/src/components/ReviewSidebarPanel.tsx @@ -55,6 +55,32 @@ export function ReviewCommentsButton() { ); } +/** Explicitly rerun the configured quality-finding provider for the current diff. */ +export function ReviewFindingsRefreshButton() { + const review = useReview(); + return ( + + ); +} + /** Sidebar column with human comments and provider findings. */ export function ReviewSidebarPanel() { const review = useReview(); diff --git a/src/components/TaskPanel.tsx b/src/components/TaskPanel.tsx index ecb967c4..4650eeca 100644 --- a/src/components/TaskPanel.tsx +++ b/src/components/TaskPanel.tsx @@ -43,6 +43,7 @@ import type { CommitInfo } from '../ipc/types'; import { isLandedTaskState } from '../store/landing'; import { shouldPollTaskCommits } from './task-commit-polling'; import { devQualityFindingProvider } from './dev-quality-finding-fixture'; +import { createEslintQualityFindingProvider } from '../lib/eslint-quality-findings'; interface TaskPanelProps { task: Task; @@ -57,6 +58,9 @@ const CHANGED_FILES_PANEL_AUTO_MAX = 'min(300px, 33vh)'; const NOTES_PANEL_AUTO_MAX = 'min(400px, 33vh)'; export function TaskPanel(props: TaskPanelProps) { + const eslintQualityFindingProvider = createEslintQualityFindingProvider( + () => props.task.worktreePath, + ); const [showCloseConfirm, setShowCloseConfirm] = createSignal(false); const [planFullscreen, setPlanFullscreen] = createSignal(false); @@ -657,7 +661,7 @@ export function TaskPanel(props: TaskPanelProps) { selectedCommit={selectedCommit()} onCommitNavigate={setSelectedCommit} gitIsolation={props.task.gitIsolation} - findingProvider={devQualityFindingProvider} + findingProvider={devQualityFindingProvider ?? eslintQualityFindingProvider} /> setEditingProjectId(null)} /> diff --git a/src/ipc/types.ts b/src/ipc/types.ts index 00cfeddb..ff3aea67 100644 --- a/src/ipc/types.ts +++ b/src/ipc/types.ts @@ -7,6 +7,8 @@ export type { CoverageMetricSummary, CoverageSummary, CreateTaskResult, + EslintQualityFinding, + EslintQualityResult, FileDiffResult, GitIgnoredEntry, ImportableWorktree, diff --git a/src/lib/eslint-quality-findings.ts b/src/lib/eslint-quality-findings.ts new file mode 100644 index 00000000..6f232011 --- /dev/null +++ b/src/lib/eslint-quality-findings.ts @@ -0,0 +1,26 @@ +import { IPC } from '../../electron/ipc/channels'; +import type { EslintQualityResult } from '../ipc/types'; +import { invoke } from './ipc'; +import type { QualityFinding, QualityFindingProvider } from './quality-findings'; + +export function createEslintQualityFindingProvider( + getWorktreePath: () => string, +): QualityFindingProvider { + return { + async loadFindings({ files }): Promise { + const result = await invoke(IPC.GetEslintQualityFindings, { + worktreePath: getWorktreePath(), + filePaths: files + .filter((file) => file.status !== 'D' && !file.binary) + .map((file) => file.path), + }); + if (result.status === 'not-applicable') return []; + if (result.status === 'unavailable') throw new Error(result.message); + return result.findings.map((finding) => ({ + ...finding, + state: 'open' as const, + freshness: 'pending' as const, + })); + }, + }; +}