diff --git a/.env.example b/.env.example index 7680b99..62ccb15 100644 --- a/.env.example +++ b/.env.example @@ -28,3 +28,9 @@ BACHS_API_KEY="your-bachs-secret-key" BACHS_API_BASE="https://sandbox-api.bachs.io" # live: https://api.bachs.io BACHS_PRO_PRODUCT_ID="your-bachs-pro-product-id" BACHS_WEBHOOK_SECRET="your-bachs-webhook-signing-secret" + +# --- Feature flags --- +# Comma-separated GitHub logins that can see the codebase architecture map. +# Unset falls back to the accounts trialling it; "*" opens it to everyone; +# empty disables it for all. Gates both building the map and returning it. +ARCHITECTURE_MAP_LOGINS="devtofunmi,xt42io" diff --git a/src/components/scans/architecture-layout.test.ts b/src/components/scans/architecture-layout.test.ts new file mode 100644 index 0000000..a944d3b --- /dev/null +++ b/src/components/scans/architecture-layout.test.ts @@ -0,0 +1,196 @@ +import { describe, expect, it } from 'vitest' + +import { + GAP_X, + GAP_Y, + NODE_HEIGHT, + NODE_WIDTH, + PADDING, + assignLayers, + layoutArchitecture, + moduleName, + moduleParent, +} from './architecture-layout' +import type { ScanArchitectureModule } from '../../server/scan-engine/summary' + +const asModule = (id: string): ScanArchitectureModule => ({ + id, + label: null, + files: 1, + findings: 0, + counts: { critical: 0, high: 0, medium: 0, low: 0, note: 0 }, + topSeverity: null, +}) + +describe('assignLayers', () => { + it('puts a module one column right of what imports it', () => { + const layers = assignLayers(['routes', 'server', 'db'].map(asModule), [ + { from: 'routes', to: 'server', weight: 1 }, + { from: 'server', to: 'db', weight: 1 }, + ]) + + expect(layers.get('routes')).toBe(0) + expect(layers.get('server')).toBe(1) + expect(layers.get('db')).toBe(2) + }) + + it('uses the longest path when a module has two importers', () => { + const layers = assignLayers(['a', 'b', 'c'].map(asModule), [ + { from: 'a', to: 'b', weight: 1 }, + { from: 'b', to: 'c', weight: 1 }, + { from: 'a', to: 'c', weight: 1 }, + ]) + + // Reachable at depth 1 directly and depth 2 via b; the deeper one wins so + // the arrow never points backwards. + expect(layers.get('c')).toBe(2) + }) + + it('ignores the edge that closes a cycle when placing columns', () => { + const layers = assignLayers(['a', 'b', 'c'].map(asModule), [ + { from: 'a', to: 'b', weight: 1 }, + { from: 'b', to: 'c', weight: 1 }, + { from: 'c', to: 'a', weight: 1 }, + ]) + + // Without dropping c -> a from the maths, the cycle would push every module + // to the last column and the diagram would collapse into one stack. + expect(layers.get('a')).toBe(0) + expect(layers.get('b')).toBe(1) + expect(layers.get('c')).toBe(2) + }) + + it('keeps a module out of a column its own dependents pushed it past', () => { + // A two-module cycle plus a third importer: the cycle must not drag `hub` + // rightwards past everything that depends on it. + const layers = assignLayers(['app', 'hub', 'util'].map(asModule), [ + { from: 'app', to: 'hub', weight: 1 }, + { from: 'hub', to: 'util', weight: 1 }, + { from: 'util', to: 'hub', weight: 1 }, + ]) + + expect(layers.get('app')).toBe(0) + expect(layers.get('hub')).toBe(1) + expect(layers.get('util')).toBe(2) + }) + + it('still terminates when every module is in one cycle', () => { + const ids = ['a', 'b', 'c', 'd'] + const layers = assignLayers( + ids.map(asModule), + ids.map((id, index) => ({ + from: id, + to: ids[(index + 1) % ids.length], + weight: 1, + })), + ) + + for (const id of ids) { + expect(layers.get(id)).toBeLessThanOrEqual(ids.length - 1) + expect(layers.get(id)).toBeGreaterThanOrEqual(0) + } + }) + + it('ignores an edge naming a module that is not present', () => { + const layers = assignLayers( + [asModule('a')], + [ + { from: 'a', to: 'ghost', weight: 1 }, + { from: 'ghost', to: 'a', weight: 1 }, + ], + ) + + expect(layers.get('a')).toBe(0) + expect(layers.has('ghost')).toBe(false) + }) +}) + +describe('layoutArchitecture', () => { + it('lays out nothing for an empty map', () => { + expect(layoutArchitecture([], [])).toEqual({ + nodes: [], + edges: [], + width: 0, + height: 0, + }) + }) + + it('sizes the canvas from the column count and the tallest column', () => { + const layout = layoutArchitecture(['a', 'b', 'c'].map(asModule), [ + { from: 'a', to: 'c', weight: 1 }, + { from: 'b', to: 'c', weight: 1 }, + ]) + + // Two columns: [a, b] then [c]. + expect(layout.width).toBe(2 * NODE_WIDTH + GAP_X + PADDING * 2) + expect(layout.height).toBe(2 * NODE_HEIGHT + GAP_Y + PADDING * 2) + }) + + it('centres a short column against the tallest one', () => { + const layout = layoutArchitecture(['a', 'b', 'c'].map(asModule), [ + { from: 'a', to: 'c', weight: 1 }, + { from: 'b', to: 'c', weight: 1 }, + ]) + + const c = layout.nodes.find((node) => node.id === 'c')! + const a = layout.nodes.find((node) => node.id === 'a')! + const b = layout.nodes.find((node) => node.id === 'b')! + + expect(c.y).toBeCloseTo((a.y + b.y) / 2) + expect(c.x).toBeGreaterThan(a.x) + }) + + it('draws an edge between the two boxes it connects', () => { + const layout = layoutArchitecture(['a', 'b'].map(asModule), [ + { from: 'a', to: 'b', weight: 3 }, + ]) + + const a = layout.nodes.find((node) => node.id === 'a')! + const b = layout.nodes.find((node) => node.id === 'b')! + + expect(layout.edges).toHaveLength(1) + expect(layout.edges[0].weight).toBe(3) + expect(layout.edges[0].path).toContain( + `M ${a.x + NODE_WIDTH} ${a.y + NODE_HEIGHT / 2}`, + ) + expect(layout.edges[0].path).toContain(`${b.x} ${b.y + NODE_HEIGHT / 2}`) + }) + + it('drops an edge whose endpoint is not on the map', () => { + const layout = layoutArchitecture( + [asModule('a')], + [{ from: 'a', to: 'ghost', weight: 1 }], + ) + + expect(layout.edges).toEqual([]) + }) + + it('draws the same picture for the same map', () => { + const modules = ['a', 'b', 'c'].map(asModule) + const edges = [{ from: 'a', to: 'b', weight: 1 }] + + expect(layoutArchitecture(modules, edges)).toEqual( + layoutArchitecture(modules, edges), + ) + }) +}) + +describe('moduleName', () => { + it('emphasises the segment that distinguishes the module', () => { + expect(moduleName('src/server/scan-engine')).toBe('scan-engine') + expect(moduleParent('src/server/scan-engine')).toBe('src/server') + }) + + it('has no parent for a top-level module', () => { + expect(moduleName('src')).toBe('src') + expect(moduleParent('src')).toBeNull() + }) + + it('shortens a parent path that would not fit', () => { + const parent = moduleParent( + 'packages/platform/services/internal/deep/nested/leaf', + ) + expect(parent!.length).toBeLessThanOrEqual(24) + expect(parent).toContain('…') + }) +}) diff --git a/src/components/scans/architecture-layout.ts b/src/components/scans/architecture-layout.ts new file mode 100644 index 0000000..369e2d0 --- /dev/null +++ b/src/components/scans/architecture-layout.ts @@ -0,0 +1,229 @@ +// Geometry for the architecture map. Pure and React-free so the layout can be +// unit-tested without rendering: the part worth testing is the arithmetic, not +// the SVG. + +import type { + ScanArchitectureEdge, + ScanArchitectureModule, +} from '../../server/scan-engine/summary' + +export const NODE_WIDTH = 176 +export const NODE_HEIGHT = 66 +export const GAP_X = 76 +export const GAP_Y = 22 +export const PADDING = 24 + +export type PositionedModule = ScanArchitectureModule & { + layer: number + x: number + y: number +} + +export type PositionedEdge = ScanArchitectureEdge & { + // An SVG cubic path from the right edge of the source to the left edge of the + // target. + path: string +} + +export type ArchitectureLayout = { + nodes: PositionedModule[] + edges: PositionedEdge[] + width: number + height: number +} + +// Edges that close a cycle, found by depth-first search: an edge onto a module +// still open on the search stack. +// +// Real codebases have import cycles, and letting one drive the column maths +// pushes every module in the cycle to the far right and drags everything it +// touches with it — the whole graph collapses into one tall column with arrows +// looping back across the diagram. Cycle-closing edges are still drawn; they +// just do not get a say in where the boxes go. +// +// Indexes rather than composite keys, since a repository path may contain any +// character and there is no separator safe to join on. +function backEdgeIndexes( + modules: ScanArchitectureModule[], + edges: ScanArchitectureEdge[], +): Set { + const adjacency = new Map>() + edges.forEach((edge, index) => { + const list = adjacency.get(edge.from) ?? [] + list.push({ to: edge.to, index }) + adjacency.set(edge.from, list) + }) + + const OPEN = 1 + const DONE = 2 + const state = new Map() + const backEdges = new Set() + + // Iterative, and started from every module in the order given, so the result + // does not depend on which module happens to come first in a cycle. + for (const module of modules) { + if (state.has(module.id)) continue + + state.set(module.id, OPEN) + const stack = [{ id: module.id, next: 0 }] + + while (stack.length > 0) { + const frame = stack[stack.length - 1] + const neighbours = adjacency.get(frame.id) ?? [] + + if (frame.next >= neighbours.length) { + state.set(frame.id, DONE) + stack.pop() + continue + } + + const { to, index } = neighbours[frame.next] + frame.next += 1 + + if (state.get(to) === OPEN) { + backEdges.add(index) + } else if (!state.has(to) && modules.some((m) => m.id === to)) { + state.set(to, OPEN) + stack.push({ id: to, next: 0 }) + } + } + } + + return backEdges +} + +// Push every module one column to the right of everything that imports it, so +// dependencies read left to right. +// +// Relaxed rather than walked, and bounded by the module count: cycle-closing +// edges are already excluded, so the bound is a guard rather than the mechanism. +export function assignLayers( + modules: ScanArchitectureModule[], + edges: ScanArchitectureEdge[], +): Map { + const layers = new Map( + modules.map((module) => [module.id, 0]), + ) + const backEdges = backEdgeIndexes(modules, edges) + + // One pass per module is the worst case: a chain that long cannot need more, + // and the counter caps the work if an edge set ever fails to settle. + let passesLeft = modules.length + + while (passesLeft > 0) { + passesLeft -= 1 + let moved = false + + for (const [index, edge] of edges.entries()) { + if (backEdges.has(index)) continue + const from = layers.get(edge.from) + const to = layers.get(edge.to) + if (from === undefined || to === undefined) continue + const wanted = Math.min(from + 1, modules.length - 1) + if (wanted > to) { + layers.set(edge.to, wanted) + moved = true + } + } + + if (!moved) break + } + + return layers +} + +function truncateMiddle(value: string, max: number): string { + if (value.length <= max) return value + return `${value.slice(0, max - 1)}…` +} + +// Shown inside the box. The trailing segment is what distinguishes one module +// from another, so it gets the emphasis and the parent path sits above it. +export function moduleName(id: string): string { + const cut = id.lastIndexOf('/') + return cut === -1 ? id : id.slice(cut + 1) +} + +export function moduleParent(id: string): string | null { + const cut = id.lastIndexOf('/') + return cut === -1 ? null : truncateMiddle(id.slice(0, cut), 24) +} + +export function layoutArchitecture( + modules: ScanArchitectureModule[], + edges: ScanArchitectureEdge[], +): ArchitectureLayout { + if (modules.length === 0) { + return { nodes: [], edges: [], width: 0, height: 0 } + } + + const layers = assignLayers(modules, edges) + + // Modules arrive sorted by id and stay that way within a column, so the same + // scan always draws the same picture. + const columns = new Map() + for (const module of modules) { + const layer = layers.get(module.id) ?? 0 + const column = columns.get(layer) ?? [] + column.push(module) + columns.set(layer, column) + } + + const layerIndexes = [...columns.keys()].sort((a, b) => a - b) + const tallest = Math.max( + ...layerIndexes.map((layer) => columns.get(layer)!.length), + ) + + const contentHeight = tallest * NODE_HEIGHT + (tallest - 1) * GAP_Y + const height = contentHeight + PADDING * 2 + const width = + layerIndexes.length * NODE_WIDTH + + (layerIndexes.length - 1) * GAP_X + + PADDING * 2 + + const nodes: PositionedModule[] = [] + const positions = new Map() + + layerIndexes.forEach((layer, columnIndex) => { + const column = columns.get(layer)! + const columnHeight = + column.length * NODE_HEIGHT + (column.length - 1) * GAP_Y + // Centre each column against the tallest one, so a sparse column does not + // hug the top edge. + const startY = PADDING + (contentHeight - columnHeight) / 2 + + column.forEach((module, rowIndex) => { + const positioned: PositionedModule = { + ...module, + layer, + x: PADDING + columnIndex * (NODE_WIDTH + GAP_X), + y: startY + rowIndex * (NODE_HEIGHT + GAP_Y), + } + nodes.push(positioned) + positions.set(module.id, positioned) + }) + }) + + const positionedEdges = edges.flatMap((edge): PositionedEdge[] => { + const from = positions.get(edge.from) + const to = positions.get(edge.to) + if (!from || !to) return [] + + const startX = from.x + NODE_WIDTH + const startY = from.y + NODE_HEIGHT / 2 + const endX = to.x + const endY = to.y + NODE_HEIGHT / 2 + // Enough horizontal pull that an edge inside one column, or one pointing + // back to an earlier column, still reads as a curve rather than a spike. + const pull = Math.max(GAP_X / 2, Math.abs(endX - startX) / 2) + + return [ + { + ...edge, + path: `M ${startX} ${startY} C ${startX + pull} ${startY}, ${endX - pull} ${endY}, ${endX} ${endY}`, + }, + ] + }) + + return { nodes, edges: positionedEdges, width, height } +} diff --git a/src/components/scans/architecture-map.test.tsx b/src/components/scans/architecture-map.test.tsx new file mode 100644 index 0000000..d6a32da --- /dev/null +++ b/src/components/scans/architecture-map.test.tsx @@ -0,0 +1,108 @@ +// @vitest-environment jsdom +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it } from 'vitest' + +import { ArchitectureMap } from './architecture-map' +import type { + ScanArchitecture, + ScanArchitectureModule, +} from '../../server/scan-engine/summary' + +// The map is the one part of a scan a reader is likely to screenshot, so what +// matters is that it never overstates itself: the caption has to admit how much +// of the repository the arrows actually cover. +afterEach(cleanup) + +const asModule = ( + id: string, + overrides: Partial = {}, +): ScanArchitectureModule => ({ + id, + label: null, + files: 4, + findings: 0, + counts: { critical: 0, high: 0, medium: 0, low: 0, note: 0 }, + topSeverity: null, + ...overrides, +}) + +const architecture = ( + overrides: Partial = {}, +): ScanArchitecture => ({ + modules: [asModule('src/routes'), asModule('src/server')], + edges: [{ from: 'src/routes', to: 'src/server', weight: 2 }], + omittedModules: 0, + graphedFiles: 180, + totalFiles: 940, + ...overrides, +}) + +describe('ArchitectureMap', () => { + it('names every module on the map', () => { + render() + + expect(screen.getByText('routes')).toBeTruthy() + expect(screen.getByText('server')).toBeTruthy() + }) + + it('says how much of the repository the arrows came from', () => { + render() + + const caption = screen.getByText(/dependencies read from/i) + expect(caption.textContent).toContain('940') + expect(caption.textContent).toContain('180') + }) + + it('admits when modules were left off the map', () => { + render( + , + ) + + expect(screen.getByText(/5 smaller modules not shown/i)).toBeTruthy() + }) + + it('says nothing about omitted modules when none were', () => { + render() + + expect(screen.queryByText(/not shown/i)).toBeNull() + }) + + it('shows the model-written label under the module name', () => { + render( + , + ) + + expect(screen.getByText('GitHub webhook intake')).toBeTruthy() + }) + + it('legends only the severities actually on the map', () => { + render( + , + ) + + expect(screen.getByText('critical')).toBeTruthy() + expect(screen.queryByText('medium')).toBeNull() + }) + + it('renders nothing when there is no module to draw', () => { + const { container } = render( + , + ) + + expect(container.innerHTML).toBe('') + }) +}) diff --git a/src/components/scans/architecture-map.tsx b/src/components/scans/architecture-map.tsx new file mode 100644 index 0000000..f06d24f --- /dev/null +++ b/src/components/scans/architecture-map.tsx @@ -0,0 +1,212 @@ +import { Network } from 'lucide-react' + +import { + NODE_HEIGHT, + NODE_WIDTH, + layoutArchitecture, + moduleName, + moduleParent, +} from './architecture-layout' +import type { Severity } from '../../lib/severity' +import type { ScanArchitecture } from '../../server/scan-engine/summary' + +// Boxes are directories, arrows are resolved imports, and colour is the most +// severe finding in the module. Only the label under each name was written by a +// model — everything with a shape was derived from the code. + +type Tone = { fill: string; stroke: string; text: string } + +const NEUTRAL: Tone = { + fill: 'rgba(255,255,255,0.035)', + stroke: 'rgba(255,255,255,0.10)', + text: '#a1a1aa', +} + +const SEVERITY_TONES: Record = { + critical: { + fill: 'rgba(252,165,165,0.10)', + stroke: 'rgba(252,165,165,0.42)', + text: '#fecaca', + }, + high: { + fill: 'rgba(253,186,116,0.09)', + stroke: 'rgba(253,186,116,0.36)', + text: '#fdba74', + }, + medium: { + fill: 'rgba(252,211,77,0.08)', + stroke: 'rgba(252,211,77,0.32)', + text: '#fcd34d', + }, + low: { + fill: 'rgba(125,211,252,0.07)', + stroke: 'rgba(125,211,252,0.28)', + text: '#7dd3fc', + }, + note: NEUTRAL, +} + +function toneFor(severity: Severity | null): Tone { + return severity ? SEVERITY_TONES[severity] : NEUTRAL +} + +function truncate(value: string, max: number): string { + return value.length <= max ? value : `${value.slice(0, max - 1)}…` +} + +export function ArchitectureMap({ + architecture, +}: { + architecture: ScanArchitecture +}) { + const layout = layoutArchitecture(architecture.modules, architecture.edges) + + if (layout.nodes.length === 0) { + return null + } + + const legend = (['critical', 'high', 'medium', 'low'] as const).filter( + (severity) => + architecture.modules.some((module) => module.topSeverity === severity), + ) + + return ( +
+
+ +

Architecture

+
+ +

+ {architecture.modules.length} modules across{' '} + {architecture.totalFiles.toLocaleString()} files · dependencies read + from {architecture.graphedFiles.toLocaleString()} of them + {architecture.omittedModules > 0 + ? ` · ${architecture.omittedModules} smaller modules not shown` + : null} +

+ + {/* Bounded in both directions: a map whose modules barely depend on each + other lays out as one tall column, and without a ceiling that card + would push the findings list off the page. */} +
+ + + + + + + + {layout.edges.map((edge) => ( + + ))} + + {layout.nodes.map((node) => { + const tone = toneFor(node.topSeverity) + const parent = moduleParent(node.id) + + return ( + + + {node.id} — {node.files} files, {node.findings} findings + + + {parent ? ( + + {parent}/ + + ) : null} + + {truncate(moduleName(node.id), 20)} + + {node.label ? ( + + {truncate(node.label, 32)} + + ) : null} + + {node.files} files + {node.findings > 0 ? ` · ${node.findings} findings` : ''} + + + ) + })} + +
+ + {legend.length > 0 ? ( +
+ {legend.map((severity) => ( + + + {severity} + + ))} + + module colour is its most severe finding + +
+ ) : null} +
+ ) +} diff --git a/src/routes/api.scans.start.tsx b/src/routes/api.scans.start.tsx index 3724fef..808bab9 100644 --- a/src/routes/api.scans.start.tsx +++ b/src/routes/api.scans.start.tsx @@ -1,6 +1,7 @@ import { createFileRoute } from '@tanstack/react-router' import { loadDb } from '../db/load' +import { isArchitectureMapEnabled } from '../server/flags' import { getCurrentUserFromRequest } from '../server/github-auth' export const Route = createFileRoute('/api/scans/start')({ @@ -106,6 +107,9 @@ export const Route = createFileRoute('/api/scans/start')({ owner: repository.owner, repo: repository.name, branch: repository.defaultBranch, + // Decided here rather than inside the engine: the flag is about who + // asked for the scan, and the engine only knows the repository. + architectureMap: isArchitectureMapEnabled(currentUser.username), }), ) diff --git a/src/routes/app.scans.$scanId.index.tsx b/src/routes/app.scans.$scanId.index.tsx index 0f2942b..e888deb 100644 --- a/src/routes/app.scans.$scanId.index.tsx +++ b/src/routes/app.scans.$scanId.index.tsx @@ -11,6 +11,7 @@ import { import { useEffect, useState } from 'react' import { FindingCard } from '../components/issues/finding-card' +import { ArchitectureMap } from '../components/scans/architecture-map' import { DetailPageSkeleton } from '../components/skeletons' import { timeAgo } from '../lib/format' import { getCodebaseScan } from '../server/scans' @@ -129,6 +130,10 @@ function ScanDetailPage() { ) : null} + {scan.architecture ? ( + + ) : null} +
diff --git a/src/server/env.ts b/src/server/env.ts index 07ff544..e54a59d 100644 --- a/src/server/env.ts +++ b/src/server/env.ts @@ -20,6 +20,7 @@ type OptionalEnvKey = | 'BACHS_API_KEY' | 'BACHS_API_BASE' | 'BACHS_PRO_PRODUCT_ID' + | 'ARCHITECTURE_MAP_LOGINS' export function getEnv(key: RequiredEnvKey) { const value = process.env[key] diff --git a/src/server/flags.test.ts b/src/server/flags.test.ts new file mode 100644 index 0000000..aa83c32 --- /dev/null +++ b/src/server/flags.test.ts @@ -0,0 +1,65 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { isArchitectureMapEnabled, isLoginAllowed } from './flags' + +afterEach(() => { + vi.unstubAllEnvs() +}) + +describe('isLoginAllowed', () => { + it('allows a login on the list and nobody else', () => { + expect(isLoginAllowed('devtofunmi', 'devtofunmi,xt42io')).toBe(true) + expect(isLoginAllowed('xt42io', 'devtofunmi,xt42io')).toBe(true) + expect(isLoginAllowed('someone-else', 'devtofunmi,xt42io')).toBe(false) + }) + + it('compares case-insensitively, since GitHub logins are', () => { + expect(isLoginAllowed('DevToFunmi', 'devtofunmi')).toBe(true) + expect(isLoginAllowed('devtofunmi', 'DevToFunmi')).toBe(true) + }) + + it('tolerates spacing and empty entries in the list', () => { + expect(isLoginAllowed('xt42io', ' devtofunmi , xt42io , ')).toBe(true) + }) + + it('denies a signed-out or nameless caller', () => { + expect(isLoginAllowed(null, 'devtofunmi')).toBe(false) + expect(isLoginAllowed(undefined, 'devtofunmi')).toBe(false) + expect(isLoginAllowed('', 'devtofunmi')).toBe(false) + }) + + it('denies everyone when the list is empty', () => { + expect(isLoginAllowed('devtofunmi', '')).toBe(false) + expect(isLoginAllowed('devtofunmi', ' , ')).toBe(false) + }) + + it('opens the flag to everyone on the wildcard', () => { + expect(isLoginAllowed('anyone-at-all', '*')).toBe(true) + expect(isLoginAllowed('anyone-at-all', 'devtofunmi,*')).toBe(true) + // Still not a way in for a signed-out caller by accident. + expect(isLoginAllowed('someone-else', 'devtofunmi')).toBe(false) + }) +}) + +describe('isArchitectureMapEnabled', () => { + it('defaults to the two accounts trialling the feature', () => { + expect(isArchitectureMapEnabled('devtofunmi')).toBe(true) + expect(isArchitectureMapEnabled('xt42io')).toBe(true) + expect(isArchitectureMapEnabled('octocat')).toBe(false) + }) + + it('takes the list from the environment when one is set', () => { + vi.stubEnv('ARCHITECTURE_MAP_LOGINS', 'octocat') + + expect(isArchitectureMapEnabled('octocat')).toBe(true) + // The default no longer applies once the variable is set, so rolling the + // trial forward cannot silently leave the original accounts enabled. + expect(isArchitectureMapEnabled('devtofunmi')).toBe(false) + }) + + it('can be switched off entirely without a deploy', () => { + vi.stubEnv('ARCHITECTURE_MAP_LOGINS', '') + + expect(isArchitectureMapEnabled('devtofunmi')).toBe(false) + }) +}) diff --git a/src/server/flags.ts b/src/server/flags.ts new file mode 100644 index 0000000..53dba42 --- /dev/null +++ b/src/server/flags.ts @@ -0,0 +1,46 @@ +// Feature flags gated to named GitHub logins. +// +// Deliberately not stored per-workspace in the database: these gate work that +// is still being proven out, and the set of people who should see it changes by +// editing an environment variable rather than by a migration. + +import { getOptionalEnv } from './env' + +// The architecture map runs an extra LLM call and a few hundred extra file +// reads per scan, so it is gated on both sides: it is not built for anyone +// outside this list, and it is not returned to them either. +const ARCHITECTURE_MAP_DEFAULT_LOGINS = 'devtofunmi,xt42io' + +// Opens a flag to everyone. Set the variable to this once the feature is ready +// to ship broadly, instead of listing logins forever. +const EVERYONE = '*' + +function allowedLogins(value: string): Set { + return new Set( + value + .split(',') + .map((login) => login.trim().toLowerCase()) + .filter(Boolean), + ) +} + +// Exported for tests and for callers that want the raw list; comparison is +// case-insensitive because GitHub logins are. +export function isLoginAllowed( + username: string | null | undefined, + value: string, +): boolean { + const allowed = allowedLogins(value) + if (allowed.has(EVERYONE)) return true + if (!username) return false + return allowed.has(username.trim().toLowerCase()) +} + +export function isArchitectureMapEnabled( + username: string | null | undefined, +): boolean { + return isLoginAllowed( + username, + getOptionalEnv('ARCHITECTURE_MAP_LOGINS', ARCHITECTURE_MAP_DEFAULT_LOGINS), + ) +} diff --git a/src/server/github-app/client.test.ts b/src/server/github-app/client.test.ts new file mode 100644 index 0000000..ca2c8b2 --- /dev/null +++ b/src/server/github-app/client.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from 'vitest' + +import { tokenIsFresh } from './client' + +// The margin arithmetic is the part of the token cache worth pinning: too +// generous and a scan reuses a token that expires mid-request, which surfaces +// as a spurious GitHub 401 rather than as a cache bug. +describe('tokenIsFresh', () => { + const now = 1_700_000_000_000 + const minute = 60_000 + + it('reuses a token with plenty of life left', () => { + expect(tokenIsFresh(now + 30 * minute, now)).toBe(true) + }) + + it('gives a token up a clear margin before GitHub expires it', () => { + expect(tokenIsFresh(now + minute, now)).toBe(false) + }) +}) diff --git a/src/server/github-app/client.ts b/src/server/github-app/client.ts index de24712..2e366c0 100644 --- a/src/server/github-app/client.ts +++ b/src/server/github-app/client.ts @@ -96,7 +96,34 @@ export async function getInstallation(installationId: string) { return (await response.json()) as GitHubInstallation } -export async function createInstallationAccessToken(installationId: string) { +// An installation token is good for an hour, so minting one per API call is +// pure waste — it doubles the request count of anything that reads more than a +// single file, and the codebase scan reads hundreds. Cached per installation +// until shortly before it expires. +// +// Stop trusting a token this long before GitHub does, so one already in flight +// cannot expire mid-request. +const TOKEN_EXPIRY_MARGIN_MS = 60_000 + +// Used when GitHub does not send `expires_at`. Comfortably inside the hour +// GitHub actually grants. +const TOKEN_FALLBACK_TTL_MS = 55 * 60_000 + +type CachedToken = { token: string; expiresAt: number } + +const tokenCache = new Map() + +// Mints in progress, so a burst of concurrent readers shares one exchange +// instead of each racing to mint its own. +const tokenRequests = new Map>() + +export function tokenIsFresh(expiresAt: number, now: number): boolean { + return expiresAt - TOKEN_EXPIRY_MARGIN_MS > now +} + +async function mintInstallationAccessToken( + installationId: string, +): Promise { const jwt = await createGitHubAppJwt() const response = await fetch( `https://api.github.com/app/installations/${installationId}/access_tokens`, @@ -110,13 +137,51 @@ export async function createInstallationAccessToken(installationId: string) { throw new Error('Unable to create GitHub installation access token.') } - const token = (await response.json()) as { token?: string } + const token = (await response.json()) as { + token?: string + expires_at?: string + } if (!token.token) { throw new Error('GitHub did not return an installation token.') } - return token.token + const expiresAt = token.expires_at ? Date.parse(token.expires_at) : Number.NaN + + return { + token: token.token, + expiresAt: Number.isNaN(expiresAt) + ? Date.now() + TOKEN_FALLBACK_TTL_MS + : expiresAt, + } +} + +// Named for what callers want rather than what it does: it returns a usable +// installation token, minting one only when there is no fresh one to hand. +export async function createInstallationAccessToken( + installationId: string, +): Promise { + const cached = tokenCache.get(installationId) + if (cached && tokenIsFresh(cached.expiresAt, Date.now())) { + return cached.token + } + + const pending = tokenRequests.get(installationId) + if (pending) return pending + + const request = mintInstallationAccessToken(installationId) + .then((minted) => { + tokenCache.set(installationId, minted) + return minted.token + }) + .finally(() => { + // Cleared either way: a failed mint must not pin every later caller to + // the same rejection. + tokenRequests.delete(installationId) + }) + + tokenRequests.set(installationId, request) + return request } // Uninstall the GitHub App from an account (removes the installation on diff --git a/src/server/scan-engine/candidates.test.ts b/src/server/scan-engine/candidates.test.ts new file mode 100644 index 0000000..711e9e2 --- /dev/null +++ b/src/server/scan-engine/candidates.test.ts @@ -0,0 +1,146 @@ +import { describe, expect, it } from 'vitest' + +import { + MAX_GRAPH_FILES, + scorePath, + selectGraphCandidates, + withinCharBudget, +} from './candidates' + +describe('scorePath', () => { + it('ranks application source above everything else', () => { + expect(scorePath('src/server/scans.ts')).toBeGreaterThan( + scorePath('rollup.config.ts'), + ) + }) + + it('ranks a test below the module it tests', () => { + expect(scorePath('src/server/scans.test.ts')).toBeLessThan( + scorePath('src/server/scans.ts'), + ) + }) + + it('recognises test files across language conventions', () => { + const plain = scorePath('internal/db/store.go') + expect(scorePath('internal/db/store_test.go')).toBeLessThan(plain) + expect(scorePath('app/__tests__/main.py')).toBeLessThan(plain) + expect(scorePath('src/components/button.spec.tsx')).toBeLessThan(plain) + }) + + it('ranks generated code below hand-written code', () => { + expect(scorePath('src/routeTree.gen.ts')).toBeLessThan( + scorePath('src/routes/index.tsx'), + ) + expect(scorePath('src/db/migrations/0001_init.ts')).toBeLessThan( + scorePath('src/db/schema.ts'), + ) + }) + + it('matches a marker directory at the top level as well as nested', () => { + expect(scorePath('tests/smoke.ts')).toBeLessThan(scorePath('other.ts')) + }) +}) + +describe('selectGraphCandidates', () => { + it('reads application source before supporting material', () => { + const selected = selectGraphCandidates( + [ + 'scripts/release.ts', + 'src/server/scans.test.ts', + 'src/server/scans.ts', + 'vite.config.ts', + ], + 2, + ) + + expect(selected).toEqual(['src/server/scans.ts', 'scripts/release.ts']) + }) + + it('caps how many files are ever read', () => { + const paths = Array.from({ length: 500 }, (_, i) => `src/mod-${i}.ts`) + expect(selectGraphCandidates(paths)).toHaveLength(MAX_GRAPH_FILES) + expect(selectGraphCandidates(paths, 10)).toHaveLength(10) + }) + + it('returns the same list for the same repository', () => { + const paths = ['src/b.ts', 'src/a.ts', 'src/c.ts'] + expect(selectGraphCandidates(paths, 2)).toEqual( + selectGraphCandidates([...paths].reverse(), 2), + ) + }) + + it('breaks ties on path so the order never depends on the tree', () => { + expect(selectGraphCandidates(['src/b.ts', 'src/a.ts'], 2)).toEqual([ + 'src/a.ts', + 'src/b.ts', + ]) + }) + + it('does not mutate the paths it was given', () => { + const paths = ['src/b.ts', 'src/a.ts'] + selectGraphCandidates(paths) + expect(paths).toEqual(['src/b.ts', 'src/a.ts']) + }) + + it('reads nothing when the cap is zero or negative', () => { + expect(selectGraphCandidates(['src/a.ts'], 0)).toEqual([]) + expect(selectGraphCandidates(['src/a.ts'], -5)).toEqual([]) + }) +}) + +describe('withinCharBudget', () => { + const file = (path: string, length: number) => ({ + path, + content: 'x'.repeat(length), + }) + + it('does not let one large file starve the rest', () => { + // The top-ranked file is the most-imported one, so it is exactly the file + // most likely to be huge. Spending the budget in rank order would leave the + // other three unread. + const kept = withinCharBudget( + [ + file('huge.ts', 5000), + file('b.ts', 100), + file('c.ts', 100), + file('d.ts', 100), + ], + 1000, + ) + + expect(kept.map((f) => f.path)).toEqual(['huge.ts', 'b.ts', 'c.ts', 'd.ts']) + expect(kept[0].content.length).toBe(700) + }) + + it('never exceeds the budget', () => { + const kept = withinCharBudget( + [file('a.ts', 900), file('b.ts', 900), file('c.ts', 900)], + 1000, + ) + + const total = kept.reduce((sum, f) => sum + f.content.length, 0) + expect(total).toBeLessThanOrEqual(1000) + }) + + it('leaves small files whole and spends the remainder on the ranked leader', () => { + const kept = withinCharBudget( + [file('big.ts', 800), file('small.ts', 10)], + 1000, + ) + + expect(kept[1].content.length).toBe(10) + // 500 share, plus the 490 left unspent by the small file. + expect(kept[0].content.length).toBe(800) + }) + + it('keeps every file whole when they all fit', () => { + const kept = withinCharBudget([file('a.ts', 10), file('b.ts', 20)], 1000) + + expect(kept.map((f) => f.content.length)).toEqual([10, 20]) + }) + + it('reads nothing with no files or no budget', () => { + expect(withinCharBudget([], 1000)).toEqual([]) + expect(withinCharBudget([file('a.ts', 10)], 0)).toEqual([]) + }) +}) diff --git a/src/server/scan-engine/candidates.ts b/src/server/scan-engine/candidates.ts new file mode 100644 index 0000000..3bd13b4 --- /dev/null +++ b/src/server/scan-engine/candidates.ts @@ -0,0 +1,142 @@ +// The budgeting rules for one scan: which files get read, and how much of them +// reaches the prompt. +// +// The graph wants to see as much of the repository as possible; reading a +// repository does not come free. So the tree is ranked rather than truncated: +// application source first, supporting material last, and a hard cap on how +// many files are ever read. Nothing outside this list is fetched. + +// How many files the graph pass will read. Each one is a GitHub request, so +// this is the knob that decides what a scan costs. +export const MAX_GRAPH_FILES = 200 + +// Directories that hold the code a reader would call the application. A path +// under one of these is what the map is actually about. +const SOURCE_ROOTS = [ + 'src/', + 'app/', + 'lib/', + 'server/', + 'client/', + 'internal/', + 'pkg/', + 'cmd/', + 'components/', + 'services/', +] + +// Tests describe the code rather than forming its structure. Their imports +// would wire every module to every other one, which is exactly the hairball +// the map is trying not to be. +const TEST_MARKERS = [ + '.test.', + '.spec.', + '_test.', + '/__tests__/', + '/__mocks__/', + '/tests/', + '/test/', + '/spec/', + '/e2e/', + '/fixtures/', + '/mocks/', +] + +// Machine-written files. Real imports, but nobody designed them, and a large +// generated file can crowd out the modules a reader came to see. +const GENERATED_MARKERS = [ + '.gen.', + '.generated.', + '.pb.', + '_pb2.', + '.d.ts', + '/migrations/', + '/generated/', + '/.storybook/', +] + +// Real code, lower stakes: it sits beside the application rather than in it. +const PERIPHERAL_MARKERS = [ + '/examples/', + '/example/', + '/scripts/', + '/docs/', + '/demo/', + '/benchmarks/', + '.config.', + '.setup.', +] + +function includesAny(path: string, markers: Array): boolean { + // Leading slash so a `/tests/` style marker can match a top-level directory + // as well as a nested one. + const padded = `/${path}` + return markers.some((marker) => padded.includes(marker)) +} + +// Higher scores are read first. The bands are deliberately coarse: this only +// has to be better than tree order, and a fine-grained score would be a +// guess dressed up as a measurement. +// +// The penalties outweigh the source-root bonus on purpose. Tests and generated +// files usually live in a source root, and their edges are the ones that hurt +// most — a test imports half the codebase, and a generated route tree imports +// every route, so either one becomes a false hub in the middle of the map. +// Being under `src/` should not rescue them. +export function scorePath(path: string): number { + let score = 0 + + if (includesAny(path, SOURCE_ROOTS)) score += 3 + if (includesAny(path, TEST_MARKERS)) score -= 6 + if (includesAny(path, GENERATED_MARKERS)) score -= 5 + if (includesAny(path, PERIPHERAL_MARKERS)) score -= 2 + + return score +} + +// The files to read, best first, capped. Ties break on path so two scans of an +// unchanged repository always read the same files and draw the same map. +export function selectGraphCandidates( + paths: string[], + limit: number = MAX_GRAPH_FILES, +): string[] { + return [...paths] + .map((path) => ({ path, score: scorePath(path) })) + .sort((a, b) => b.score - a.score || a.path.localeCompare(b.path)) + .slice(0, Math.max(0, limit)) + .map((entry) => entry.path) +} + +// Fit the chosen files into the prompt budget. +// +// Every file gets an equal share first, and only then is the unspent remainder +// handed out — highest-ranked file first. Spending the budget strictly in rank +// order instead would let one large file swallow all of it: the top-ranked file +// is the most-imported one, which is exactly the kind of file that runs long, so +// a single 50KB module could leave nineteen others entirely unread. +export function withinCharBudget( + files: T[], + maxTotalChars: number, +): Array<{ path: string; content: string }> { + if (files.length === 0 || maxTotalChars <= 0) return [] + + const share = Math.max(1, Math.floor(maxTotalChars / files.length)) + const kept = files.map((file) => ({ + path: file.path, + content: file.content.slice(0, share), + })) + + let used = kept.reduce((total, file) => total + file.content.length, 0) + + for (let index = 0; index < kept.length && used < maxTotalChars; index += 1) { + const full = files[index].content + const taken = kept[index].content.length + if (taken >= full.length) continue + + const extra = Math.min(full.length - taken, maxTotalChars - used) + kept[index].content = full.slice(0, taken + extra) + used += extra + } + + return kept.filter((file) => file.content.length > 0) +} diff --git a/src/server/scan-engine/github.ts b/src/server/scan-engine/github.ts index f3152dd..8aaabeb 100644 --- a/src/server/scan-engine/github.ts +++ b/src/server/scan-engine/github.ts @@ -40,6 +40,11 @@ const SKIP_DIRS = [ export type RepoFile = { path: string; content: string } +// How many file reads run at once. The installation token is cached, so these +// are one request each; the ceiling is about not hammering GitHub rather than +// about rate-limit arithmetic. +const FETCH_CONCURRENCY = 8 + export async function fetchRepoTree({ installationId, owner, @@ -113,3 +118,64 @@ export async function fetchFileContent({ return Buffer.from(data.content, 'base64').toString('utf8') } + +// Read an explicit list of files, a few at a time. Only these paths are +// fetched — the scan never pulls a whole repository, so what it reads is always +// a list something decided on rather than everything that happened to be there. +// +// Results keep the order of `paths` regardless of which read finishes first, so +// a scan of an unchanged repository always produces the same input. A file that +// cannot be read is skipped rather than failing the run: one unreadable file +// should cost its own edges, not the entire map. +export async function fetchFiles({ + installationId, + owner, + repo, + branch, + paths, + maxChars, +}: { + installationId: string + owner: string + repo: string + branch: string + paths: string[] + // Per-file cap. The graph pass only needs the head of a file, since that is + // where import statements live. + maxChars?: number +}): Promise { + const results: Array = new Array(paths.length).fill(null) + let next = 0 + + const worker = async () => { + for (;;) { + const index = next + next += 1 + if (index >= paths.length) return + + const path = paths[index] + try { + const content = await fetchFileContent({ + installationId, + owner, + repo, + branch, + path, + }) + if (content === null) continue + results[index] = { + path, + content: maxChars ? content.slice(0, maxChars) : content, + } + } catch { + // Unreadable file: leave the slot empty and keep going. + } + } + } + + await Promise.all( + Array.from({ length: Math.min(FETCH_CONCURRENCY, paths.length) }, worker), + ) + + return results.filter((file): file is RepoFile => file !== null) +} diff --git a/src/server/scan-engine/graph.test.ts b/src/server/scan-engine/graph.test.ts new file mode 100644 index 0000000..957d7d2 --- /dev/null +++ b/src/server/scan-engine/graph.test.ts @@ -0,0 +1,230 @@ +import { describe, expect, it } from 'vitest' + +import { + MAX_MODULES, + ROOT_MODULE, + buildModuleGraph, + fanInCounts, + moduleOf, + rankFilesByFanIn, +} from './graph' + +describe('moduleOf', () => { + it('uses the directory holding the file', () => { + expect(moduleOf('src/server/scan-engine/graph.ts')).toBe( + 'src/server/scan-engine', + ) + }) + + it('gives repository-root files a module of their own', () => { + expect(moduleOf('vite.config.ts')).toBe(ROOT_MODULE) + }) +}) + +describe('fanInCounts', () => { + it('counts how many files import each target', () => { + const counts = fanInCounts([ + { from: 'a.ts', to: 'shared.ts' }, + { from: 'b.ts', to: 'shared.ts' }, + { from: 'a.ts', to: 'leaf.ts' }, + ]) + + expect(counts.get('shared.ts')).toBe(2) + expect(counts.get('leaf.ts')).toBe(1) + expect(counts.get('a.ts')).toBeUndefined() + }) +}) + +describe('rankFilesByFanIn', () => { + it('reads the most-depended-upon file first', () => { + const ranked = rankFilesByFanIn( + ['entry.ts', 'shared.ts', 'leaf.ts'], + [ + { from: 'entry.ts', to: 'shared.ts' }, + { from: 'leaf.ts', to: 'shared.ts' }, + { from: 'entry.ts', to: 'leaf.ts' }, + ], + ) + + expect(ranked).toEqual(['shared.ts', 'leaf.ts', 'entry.ts']) + }) + + it('leaves files nothing imports in the order it was given', () => { + expect(rankFilesByFanIn(['b.ts', 'a.ts', 'c.ts'], [])).toEqual([ + 'b.ts', + 'a.ts', + 'c.ts', + ]) + }) + + it('does not mutate the paths it was given', () => { + const paths = ['entry.ts', 'shared.ts'] + rankFilesByFanIn(paths, [{ from: 'entry.ts', to: 'shared.ts' }]) + expect(paths).toEqual(['entry.ts', 'shared.ts']) + }) +}) + +describe('buildModuleGraph', () => { + const paths = [ + 'src/routes/index.tsx', + 'src/routes/scans.tsx', + 'src/server/scans.ts', + 'src/db/load.ts', + ] + + it('counts every file in the repository, not only the ones read', () => { + const graph = buildModuleGraph({ paths, edges: [], findings: [] }) + + expect(graph.modules).toEqual([ + { + id: 'src/db', + files: 1, + findings: 0, + counts: { critical: 0, high: 0, medium: 0, low: 0, note: 0 }, + topSeverity: null, + }, + { + id: 'src/routes', + files: 2, + findings: 0, + counts: { critical: 0, high: 0, medium: 0, low: 0, note: 0 }, + topSeverity: null, + }, + { + id: 'src/server', + files: 1, + findings: 0, + counts: { critical: 0, high: 0, medium: 0, low: 0, note: 0 }, + topSeverity: null, + }, + ]) + }) + + it('collapses file imports into one weighted arrow per module pair', () => { + const graph = buildModuleGraph({ + paths, + edges: [ + { from: 'src/routes/index.tsx', to: 'src/server/scans.ts' }, + { from: 'src/routes/scans.tsx', to: 'src/server/scans.ts' }, + { from: 'src/server/scans.ts', to: 'src/db/load.ts' }, + ], + findings: [], + }) + + expect(graph.edges).toEqual([ + { from: 'src/routes', to: 'src/server', weight: 2 }, + { from: 'src/server', to: 'src/db', weight: 1 }, + ]) + }) + + it('drops an edge that stays inside one module', () => { + const graph = buildModuleGraph({ + paths, + edges: [{ from: 'src/routes/index.tsx', to: 'src/routes/scans.tsx' }], + findings: [], + }) + + expect(graph.edges).toEqual([]) + }) + + it('resolves an edge that targets a directory rather than a file', () => { + const graph = buildModuleGraph({ + paths: ['cmd/main.go', 'internal/db/store.go'], + edges: [{ from: 'cmd/main.go', to: 'internal/db' }], + findings: [], + }) + + expect(graph.edges).toEqual([{ from: 'cmd', to: 'internal/db', weight: 1 }]) + }) + + it('colours a module by its most severe finding', () => { + const graph = buildModuleGraph({ + paths, + edges: [], + findings: [ + { filePath: 'src/server/scans.ts', severity: 'low' }, + { filePath: 'src/server/scans.ts', severity: 'critical' }, + { filePath: 'src/db/load.ts', severity: 'medium' }, + ], + }) + + const server = graph.modules.find((m) => m.id === 'src/server') + expect(server?.topSeverity).toBe('critical') + expect(server?.findings).toBe(2) + expect(graph.modules.find((m) => m.id === 'src/db')?.topSeverity).toBe( + 'medium', + ) + expect(graph.modules.find((m) => m.id === 'src/routes')?.topSeverity).toBe( + null, + ) + }) + + it('counts a finding whose file is not in the tree instead of dropping it', () => { + const graph = buildModuleGraph({ + paths, + edges: [], + findings: [{ filePath: 'imagined/place.ts', severity: 'high' }], + }) + + expect(graph.unattributedFindings).toBe(1) + expect(graph.modules.every((m) => m.findings === 0)).toBe(true) + }) + + it('collapses deep modules into their parents to fit the cap', () => { + const deep = [ + 'src/a/one.ts', + 'src/b/two.ts', + 'src/c/three.ts', + 'src/d/four.ts', + ] + const graph = buildModuleGraph({ + paths: deep, + edges: [], + findings: [], + maxNodes: 1, + }) + + expect(graph.modules).toHaveLength(1) + expect(graph.modules[0].id).toBe('src') + // Collapsing loses a level of detail, never a file. + expect(graph.modules[0].files).toBe(4) + expect(graph.omittedModules).toBe(0) + }) + + it('re-points edges at the surviving ancestor after a collapse', () => { + const graph = buildModuleGraph({ + paths: ['src/web/page.ts', 'src/api/route.ts', 'core/db/load.ts'], + edges: [{ from: 'src/web/page.ts', to: 'core/db/load.ts' }], + findings: [], + maxNodes: 2, + }) + + // Three modules do not fit, so each side of the edge folds up one level. + expect(graph.modules.map((m) => m.id)).toEqual(['core', 'src']) + expect(graph.edges).toEqual([{ from: 'src', to: 'core', weight: 1 }]) + }) + + it('keeps the modules that matter and reports the rest as omitted', () => { + const graph = buildModuleGraph({ + paths: ['a/one.ts', 'b/two.ts', 'c/three.ts'], + edges: [], + findings: [{ filePath: 'c/three.ts', severity: 'critical' }], + maxNodes: 1, + }) + + expect(graph.modules.map((m) => m.id)).toEqual(['c']) + expect(graph.omittedModules).toBe(2) + }) + + it('defaults to a cap a reader can actually take in', () => { + const manyPaths = Array.from({ length: 60 }, (_, i) => `mod${i}/file.ts`) + const graph = buildModuleGraph({ + paths: manyPaths, + edges: [], + findings: [], + }) + + expect(graph.modules.length).toBeLessThanOrEqual(MAX_MODULES) + expect(graph.modules.length + graph.omittedModules).toBe(60) + }) +}) diff --git a/src/server/scan-engine/graph.ts b/src/server/scan-engine/graph.ts new file mode 100644 index 0000000..1da390f --- /dev/null +++ b/src/server/scan-engine/graph.ts @@ -0,0 +1,294 @@ +// Turns file-level import edges into the two things the scan needs from them: +// an order to read files in, and a module-level graph to draw. +// +// Everything here is derived from code. No shape in the output is a model's +// opinion — that is the whole point. A wrong label on a module is a shrug; a +// wrong arrow is a false dependency diagram someone pastes into a design doc. + +import { severityRank } from '../../lib/severity' +import type { Severity, SeverityCounts } from '../../lib/severity' +import type { ImportEdge } from './imports' + +// How many boxes the map may show. Past roughly this many, a dependency +// diagram stops being read and starts being squinted at. +export const MAX_MODULES = 24 + +// Files at the repository root belong to a module too, and it needs a name. +export const ROOT_MODULE = '.' + +// The module a path belongs to: the directory holding it. +export function moduleOf(path: string): string { + const cut = path.lastIndexOf('/') + return cut === -1 ? ROOT_MODULE : path.slice(0, cut) +} + +function depthOf(moduleId: string): number { + return moduleId === ROOT_MODULE ? 1 : moduleId.split('/').length +} + +function parentOf(moduleId: string): string | null { + if (moduleId === ROOT_MODULE) return null + const cut = moduleId.lastIndexOf('/') + return cut === -1 ? null : moduleId.slice(0, cut) +} + +// How many distinct files import each path. This is the blast radius of a bug: +// a defect in a module twenty other files depend on is worth more attention +// than one in a leaf nothing imports. +export function fanInCounts(edges: ImportEdge[]): Map { + const counts = new Map() + for (const edge of edges) { + counts.set(edge.to, (counts.get(edge.to) ?? 0) + 1) + } + return counts +} + +// Reorder already-ranked candidates so the most-depended-upon files come +// first. Sorting on fan-in alone is deliberate: Array.prototype.sort is stable, +// so files nothing imports keep the order the caller gave them rather than +// being reshuffled by a second guess. +export function rankFilesByFanIn( + paths: string[], + edges: ImportEdge[], +): string[] { + const counts = fanInCounts(edges) + return [...paths].sort((a, b) => (counts.get(b) ?? 0) - (counts.get(a) ?? 0)) +} + +export type ModuleNode = { + id: string + files: number + findings: number + counts: SeverityCounts + // The most severe finding in the module, which is what colours its box. + topSeverity: Severity | null +} + +export type ModuleEdge = { + from: string + to: string + // How many file-level imports this one arrow stands for. + weight: number +} + +export type ModuleGraph = { + modules: ModuleNode[] + edges: ModuleEdge[] + // Set when the repository had more top-level modules than the map can show, + // so the UI can say so instead of implying it drew everything. + omittedModules: number + // Findings whose file did not match anything in the tree. Should be zero; + // counted rather than silently dropped so a drift shows up. + unattributedFindings: number +} + +const emptyModuleCounts = (): SeverityCounts => ({ + critical: 0, + high: 0, + medium: 0, + low: 0, + note: 0, +}) + +type Accumulator = { + files: number + counts: SeverityCounts +} + +function blank(): Accumulator { + return { files: 0, counts: emptyModuleCounts() } +} + +function absorb(into: Accumulator, from: Accumulator) { + into.files += from.files + for (const severity of Object.keys(from.counts) as Array) { + into.counts[severity] += from.counts[severity] + } +} + +function totalFindings(counts: SeverityCounts): number { + return Object.values(counts).reduce((sum, value) => sum + value, 0) +} + +function mostSevere(counts: SeverityCounts): Severity | null { + const present = (Object.keys(counts) as Array).filter( + (severity) => counts[severity] > 0, + ) + if (present.length === 0) return null + return present.sort((a, b) => severityRank(a) - severityRank(b))[0] +} + +// Merge the deepest modules into their parents until the map fits. Collapsing +// rather than dropping keeps every file accounted for: `src/server/llm` folding +// into `src/server` loses a level of detail, not a number. +// +// Modules are bucketed by depth once and each bucket is sorted once, so the +// whole pass is O(n log n). The obvious version — rescan and re-sort every +// module to pick the next single victim — is quadratic, and a repository with a +// couple of thousand directories spent over half a minute of blocking CPU in +// here before this was bucketed. +// +// Terminates because merging only ever moves a module to a strictly shallower +// depth, and the walk goes deepest-first and never revisits a depth. +function collapseToFit( + accumulators: Map, + maxNodes: number, +): void { + if (accumulators.size <= maxNodes) return + + const byDepth = new Map() + let deepest = 0 + + for (const id of accumulators.keys()) { + const depth = depthOf(id) + deepest = Math.max(deepest, depth) + const bucket = byDepth.get(depth) ?? [] + bucket.push(id) + byDepth.set(depth, bucket) + } + + // Depth 1 is the floor: a top-level module has no parent to fold into. + for (let depth = deepest; depth > 1; depth -= 1) { + if (accumulators.size <= maxNodes) return + + // Least interesting first: fewest findings, then fewest files, then path + // order so the choice is never arbitrary. + const bucket = (byDepth.get(depth) ?? []) + .filter((id) => accumulators.has(id)) + .sort((a, b) => { + const left = accumulators.get(a)! + const right = accumulators.get(b)! + return ( + totalFindings(left.counts) - totalFindings(right.counts) || + left.files - right.files || + a.localeCompare(b) + ) + }) + + for (const id of bucket) { + if (accumulators.size <= maxNodes) return + + const parent = parentOf(id) + if (!parent) continue + + const existing = accumulators.get(parent) + const target = existing ?? blank() + absorb(target, accumulators.get(id)!) + accumulators.set(parent, target) + accumulators.delete(id) + + if (!existing) { + // A parent that did not exist as a module of its own joins its own + // bucket, so a later, shallower pass can fold it further if needed. + const parentDepth = depthOf(parent) + const parentBucket = byDepth.get(parentDepth) ?? [] + parentBucket.push(parent) + byDepth.set(parentDepth, parentBucket) + } + } + } +} + +// Map an original module id onto whichever surviving module now contains it, by +// walking up until a kept ancestor is found. +function survivorOf(moduleId: string, kept: Set): string | null { + let current: string | null = moduleId + while (current) { + if (kept.has(current)) return current + current = parentOf(current) + } + return null +} + +export function buildModuleGraph({ + paths, + edges, + findings, + maxNodes = MAX_MODULES, +}: { + // Every code path in the repository, so file counts and structure are + // complete even though edges only cover the files that were read. + paths: string[] + edges: ImportEdge[] + findings: Array<{ filePath: string; severity: Severity }> + maxNodes?: number +}): ModuleGraph { + const fileSet = new Set(paths) + const accumulators = new Map() + + for (const path of paths) { + const id = moduleOf(path) + const entry = accumulators.get(id) ?? blank() + entry.files += 1 + accumulators.set(id, entry) + } + + let unattributedFindings = 0 + for (const finding of findings) { + const entry = accumulators.get(moduleOf(finding.filePath)) + if (!entry) { + // The path came back from the model, so it can drift from the tree. + unattributedFindings += 1 + continue + } + entry.counts[finding.severity] += 1 + } + + collapseToFit(accumulators, maxNodes) + + // Only reachable when the repository has more top-level directories than the + // map can show, since collapsing cannot merge depth-1 modules any further. + let omittedModules = 0 + if (accumulators.size > maxNodes) { + const ordered = [...accumulators.entries()].sort( + ([aId, a], [bId, b]) => + totalFindings(b.counts) - totalFindings(a.counts) || + b.files - a.files || + aId.localeCompare(bId), + ) + omittedModules = ordered.length - maxNodes + for (const [id] of ordered.slice(maxNodes)) accumulators.delete(id) + } + + const kept = new Set(accumulators.keys()) + + // A resolved target is either a file whose directory is the module, or — for + // import systems that name packages rather than files — already a directory. + const moduleForTarget = (target: string) => + fileSet.has(target) ? moduleOf(target) : target + + // Nested rather than a joined string key: a repository path may contain any + // character, so there is no separator that is safe to split back on. + const weights = new Map>() + for (const edge of edges) { + const from = survivorOf(moduleOf(edge.from), kept) + const to = survivorOf(moduleForTarget(edge.to), kept) + if (!from || !to || from === to) continue + const targets = weights.get(from) ?? new Map() + targets.set(to, (targets.get(to) ?? 0) + 1) + weights.set(from, targets) + } + + const modules: ModuleNode[] = [...accumulators.entries()] + .map(([id, entry]) => ({ + id, + files: entry.files, + findings: totalFindings(entry.counts), + counts: entry.counts, + topSeverity: mostSevere(entry.counts), + })) + .sort((a, b) => a.id.localeCompare(b.id)) + + const moduleEdges: ModuleEdge[] = [...weights.entries()] + .flatMap(([from, targets]) => + [...targets.entries()].map(([to, weight]) => ({ from, to, weight })), + ) + .sort((a, b) => a.from.localeCompare(b.from) || a.to.localeCompare(b.to)) + + return { + modules, + edges: moduleEdges, + omittedModules, + unattributedFindings, + } +} diff --git a/src/server/scan-engine/imports.test.ts b/src/server/scan-engine/imports.test.ts new file mode 100644 index 0000000..1c7b798 --- /dev/null +++ b/src/server/scan-engine/imports.test.ts @@ -0,0 +1,186 @@ +import { describe, expect, it } from 'vitest' + +import { + buildImportEdges, + createPathIndex, + extractSpecifiers, + resolveSpecifier, +} from './imports' + +describe('extractSpecifiers', () => { + it('reads every JavaScript import shape', () => { + const source = [ + "import { a } from './a'", + "import type { B } from '../types/b'", + "import './side-effect'", + "export { c } from './c'", + "const d = require('./d')", + "const e = await import('./e')", + ].join('\n') + + expect(extractSpecifiers('src/index.ts', source).sort()).toEqual([ + '../types/b', + './a', + './c', + './d', + './e', + './side-effect', + ]) + }) + + it('does not let a bare import swallow the file up to the next quote', () => { + const source = [ + "import './side-effect'", + '', + "import { x } from './x'", + ].join('\n') + + expect(extractSpecifiers('src/index.ts', source).sort()).toEqual([ + './side-effect', + './x', + ]) + }) + + it('reads Python import forms', () => { + const source = [ + 'import os', + 'from app.db import session', + 'from . import util', + ].join('\n') + + expect(extractSpecifiers('app/main.py', source).sort()).toEqual([ + '.', + 'app.db', + 'os', + ]) + }) + + it('reads grouped and single Go imports without picking up other strings', () => { + const source = [ + 'import "fmt"', + 'import (', + ' "net/http"', + ' db "github.com/me/app/internal/db"', + ')', + 'var greeting = "not an import"', + ].join('\n') + + expect(extractSpecifiers('cmd/main.go', source).sort()).toEqual([ + 'fmt', + 'github.com/me/app/internal/db', + 'net/http', + ]) + }) + + it('yields nothing for an extension it does not understand', () => { + expect(extractSpecifiers('README.md', "import { a } from './a'")).toEqual( + [], + ) + }) +}) + +describe('resolveSpecifier', () => { + const index = createPathIndex([ + 'src/index.ts', + 'src/db/load.ts', + 'src/lib/format.ts', + 'src/server/scans/index.ts', + 'app/db/session.py', + 'app/util.py', + 'internal/db/store.go', + ]) + + it('resolves a relative specifier against the importing file', () => { + expect(resolveSpecifier('src/index.ts', './db/load', index)).toBe( + 'src/db/load.ts', + ) + expect(resolveSpecifier('src/db/load.ts', '../lib/format', index)).toBe( + 'src/lib/format.ts', + ) + }) + + it('resolves a directory specifier through its index file', () => { + expect(resolveSpecifier('src/index.ts', './server/scans', index)).toBe( + 'src/server/scans/index.ts', + ) + }) + + it('resolves a project-root alias', () => { + expect(resolveSpecifier('src/index.ts', '#/db/load', index)).toBe( + 'src/db/load.ts', + ) + }) + + it('drops package specifiers', () => { + expect(resolveSpecifier('src/index.ts', 'react', index)).toBeNull() + expect(resolveSpecifier('cmd/main.go', 'net/http', index)).toBeNull() + }) + + it('resolves a dotted Python module', () => { + expect(resolveSpecifier('app/main.py', 'app.db.session', index)).toBe( + 'app/db/session.py', + ) + }) + + it('counts leading dots in a relative Python import as package levels', () => { + expect(resolveSpecifier('app/db/session.py', '..util', index)).toBe( + 'app/util.py', + ) + }) + + it('strips a Go module prefix to find the package in the repository', () => { + expect( + resolveSpecifier('cmd/main.go', 'github.com/me/app/internal/db', index), + ).toBe('internal/db') + }) + + it('prefers the shortest path when a suffix is ambiguous', () => { + const ambiguous = createPathIndex([ + 'src/db/load.ts', + 'packages/legacy/src/db/load.ts', + ]) + expect(resolveSpecifier('src/index.ts', '#/db/load', ambiguous)).toBe( + 'src/db/load.ts', + ) + }) +}) + +describe('buildImportEdges', () => { + const paths = ['src/index.ts', 'src/db/load.ts', 'src/lib/format.ts'] + const index = createPathIndex(paths) + + it('keeps only edges that land inside the repository', () => { + const edges = buildImportEdges( + [ + { + path: 'src/index.ts', + content: [ + "import React from 'react'", + "import { load } from './db/load'", + ].join('\n'), + }, + ], + index, + ) + + expect(edges).toEqual([{ from: 'src/index.ts', to: 'src/db/load.ts' }]) + }) + + it('collapses duplicates and drops self-edges', () => { + const edges = buildImportEdges( + [ + { + path: 'src/index.ts', + content: [ + "import { a } from './db/load'", + "import { b } from './db/load'", + "import { c } from './index'", + ].join('\n'), + }, + ], + index, + ) + + expect(edges).toEqual([{ from: 'src/index.ts', to: 'src/db/load.ts' }]) + }) +}) diff --git a/src/server/scan-engine/imports.ts b/src/server/scan-engine/imports.ts new file mode 100644 index 0000000..d4194f7 --- /dev/null +++ b/src/server/scan-engine/imports.ts @@ -0,0 +1,258 @@ +// Reads intra-repository dependency edges out of source text. Deliberately +// regex-based rather than a real parser: the map only needs to know which +// files reach which other files, a target no import statement hides from, and +// a parser per accepted extension would cost far more than the answer is +// worth. +// +// Only specifiers that land inside the repository survive resolution. Package +// imports (`react`, `net/http`) are dropped — they are not modules the map can +// draw, and keeping them would make every node look connected to everything. + +// Tried in order when a specifier does not already name a file exactly. +// Mirrors how the respective toolchains resolve a module path. +const RESOLUTION_SUFFIXES = [ + '', + '.ts', + '.tsx', + '.js', + '.jsx', + '.mjs', + '.cjs', + '.vue', + '.svelte', + '.py', + '.go', + '.rb', + '/index.ts', + '/index.tsx', + '/index.js', + '/index.jsx', + '/index.mjs', + '/__init__.py', +] + +// A bare specifier starting with one of these is a project-root alias +// (`#/db/load`, `@/lib/x`), not a package name. +const ALIAS_PREFIXES = ['#/', '@/', '~/'] + +const JS_EXTENSIONS = new Set([ + 'ts', + 'tsx', + 'js', + 'jsx', + 'mjs', + 'cjs', + 'vue', + 'svelte', +]) + +// One pattern per import shape rather than one clever pattern, so a missed +// form is obvious. Forbidding quotes in the lazy run between the keyword and +// `from` keeps it from spanning half the file when a bare `import 'x'` sits +// above a normal import. +const JS_PATTERNS = [ + /\bimport\s[^;'"]*?\bfrom\s*['"]([^'"]+)['"]/g, + /\bimport\s*['"]([^'"]+)['"]/g, + /\bexport\s[^;'"]*?\bfrom\s*['"]([^'"]+)['"]/g, + /\brequire\s*\(\s*['"]([^'"]+)['"]\s*\)/g, + /\bimport\s*\(\s*['"]([^'"]+)['"]\s*\)/g, +] + +const PY_PATTERNS = [ + /^[ \t]*from[ \t]+([.\w]+)[ \t]+import\b/gm, + /^[ \t]*import[ \t]+([.\w]+)/gm, +] + +const RUBY_PATTERNS = [/\brequire_relative\s+['"]([^'"]+)['"]/g] + +// The grouped Go form is matched as a block first, so quoted strings elsewhere +// in the file are never mistaken for imports. +const GO_SINGLE = /^[ \t]*import[ \t]+(?:[\w.]+[ \t]+)?"([^"]+)"/gm +const GO_BLOCK = /\bimport\s*\(([\s\S]*?)\)/g +const GO_BLOCK_ENTRY = /(?:[\w.]+[ \t]+)?"([^"]+)"/g + +function collect(patterns: Array, text: string, into: Set) { + for (const pattern of patterns) { + // Module-level regexes carry `lastIndex` between calls, so every pass has + // to start from a known position. + pattern.lastIndex = 0 + let match = pattern.exec(text) + while (match) { + if (match[1]) into.add(match[1]) + match = pattern.exec(text) + } + } +} + +// Raw module specifiers written in one file, before any resolution. An +// unrecognised extension yields nothing rather than guessing. +export function extractSpecifiers(path: string, content: string): string[] { + const extension = path.split('.').pop()?.toLowerCase() ?? '' + const specifiers = new Set() + + if (JS_EXTENSIONS.has(extension)) { + collect(JS_PATTERNS, content, specifiers) + } else if (extension === 'py') { + collect(PY_PATTERNS, content, specifiers) + } else if (extension === 'rb') { + collect(RUBY_PATTERNS, content, specifiers) + } else if (extension === 'go') { + collect([GO_SINGLE], content, specifiers) + GO_BLOCK.lastIndex = 0 + let block = GO_BLOCK.exec(content) + while (block) { + collect([GO_BLOCK_ENTRY], block[1], specifiers) + block = GO_BLOCK.exec(content) + } + } + + return [...specifiers] +} + +// A lookup over every path in the repository, indexed by all of its trailing +// segment runs. That is what lets an alias (`#/db/load`) or a Go module path +// (`github.com/me/app/internal/db`) resolve without the scan knowing the +// project alias config or its Go module prefix. +export type PathIndex = { + files: Set + directories: Set + bySuffix: Map +} + +// Every trailing run of segments: `a/b/c.ts` gives `a/b/c.ts`, `b/c.ts`, +// `c.ts`. +function suffixesOf(path: string): Array { + const segments = path.split('/') + return segments.map((_, index) => segments.slice(index).join('/')) +} + +export function createPathIndex(paths: string[]): PathIndex { + const files = new Set(paths) + const directories = new Set() + const bySuffix = new Map() + + // A shorter repository path is the better answer for an ambiguous suffix: a + // specifier ending `db/load` should resolve to `src/db/load.ts`, not to + // `packages/legacy/src/db/load.ts`. + const offer = (key: string, path: string) => { + if (!key) return + const existing = bySuffix.get(key) + if (!existing || path.length < existing.length) bySuffix.set(key, path) + } + + for (const path of paths) { + const segments = path.split('/') + for (let i = 1; i < segments.length; i += 1) { + directories.add(segments.slice(0, i).join('/')) + } + for (const suffix of suffixesOf(path)) { + offer(suffix, path) + offer(suffix.replace(/\.[^./]+$/, ''), path) + } + } + + for (const directory of directories) { + for (const suffix of suffixesOf(directory)) offer(suffix, directory) + } + + return { files, directories, bySuffix } +} + +// Resolve `./foo` against the directory holding `fromPath`, collapsing `.` and +// `..` segments. +function joinRelative(fromPath: string, specifier: string): string { + const segments = fromPath.split('/').slice(0, -1) + + for (const part of specifier.split('/')) { + if (part === '.' || part === '') continue + if (part === '..') segments.pop() + else segments.push(part) + } + + return segments.join('/') +} + +function matchExact(base: string, index: PathIndex): string | null { + for (const suffix of RESOLUTION_SUFFIXES) { + const candidate = `${base}${suffix}` + if (index.files.has(candidate)) return candidate + } + return index.directories.has(base) ? base : null +} + +// Try the trailing runs of the specifier itself, longest first. This is what +// strips a Go module prefix or an alias root the scan does not know about. +function matchBySuffix(base: string, index: PathIndex): string | null { + for (const suffix of suffixesOf(base)) { + const hit = index.bySuffix.get(suffix) + if (hit) return hit + } + return null +} + +// The repository path a specifier points at — a file, or a directory for +// import systems that name packages rather than files. Null means it points +// outside the repository, or at a file the scan never listed. +export function resolveSpecifier( + fromPath: string, + specifier: string, + index: PathIndex, +): string | null { + if (specifier.startsWith('.')) { + // Relative Python imports count leading dots as package levels, not path + // segments: `from ..pkg import y` climbs two directories. + if (fromPath.endsWith('.py') && /^\.+\w/.test(specifier)) { + const levels = specifier.match(/^\.+/)?.[0].length ?? 1 + const base = fromPath.split('/').slice(0, -levels).join('/') + const rest = specifier.slice(levels).split('.').join('/') + return matchExact(base ? `${base}/${rest}` : rest, index) + } + return matchExact(joinRelative(fromPath, specifier), index) + } + + const alias = ALIAS_PREFIXES.find((prefix) => specifier.startsWith(prefix)) + if (alias) { + const base = specifier.slice(alias.length) + return matchExact(base, index) ?? matchBySuffix(base, index) + } + + // A dotted, non-relative Python module (`app.services.billing`). + if (fromPath.endsWith('.py') && specifier.includes('.')) { + const base = specifier.split('.').join('/') + return matchExact(base, index) ?? matchBySuffix(base, index) + } + + // Anything else is only ours if a file or directory actually sits there. + // `react` and `net/http` fall out here; `github.com/me/app/internal/db` + // survives on its trailing `internal/db`. + return matchExact(specifier, index) ?? matchBySuffix(specifier, index) +} + +export type ImportEdge = { from: string; to: string } + +// Every resolved edge across the files whose contents were read. Self-edges +// are dropped and duplicates collapse. +export function buildImportEdges( + files: Array<{ path: string; content: string }>, + index: PathIndex, +): ImportEdge[] { + // Nested rather than a joined string key: a repository path may contain any + // character, so there is no separator that is safe to join on. + const seen = new Map>() + const edges: ImportEdge[] = [] + + for (const file of files) { + for (const specifier of extractSpecifiers(file.path, file.content)) { + const target = resolveSpecifier(file.path, specifier, index) + if (!target || target === file.path) continue + + const targets = seen.get(file.path) ?? new Set() + if (targets.has(target)) continue + targets.add(target) + seen.set(file.path, targets) + edges.push({ from: file.path, to: target }) + } + } + + return edges +} diff --git a/src/server/scan-engine/llm.ts b/src/server/scan-engine/llm.ts index 07e1e66..c19bffe 100644 --- a/src/server/scan-engine/llm.ts +++ b/src/server/scan-engine/llm.ts @@ -90,3 +90,134 @@ function userPrompt(repository: string, files: RepoFile[]) { ...blocks, ].join('\n') } + +// The architecture map asks the model for exactly one thing: what to call each +// module. Boxes, arrows and colours are all derived from the code, so a bad +// answer here costs a confusing caption — not a false dependency someone then +// treats as fact. +export type ModuleLabelInput = { + id: string + files: number + // A few file names from the module. Enough to tell `server/llm` apart from + // `server/billing` without sending any file contents. + sampleFiles: string[] +} + +export type ModuleLabelResult = { + labels: Record + model: string + inputTokens: number + outputTokens: number + costUsd: number +} + +const MAX_SAMPLE_FILES = 12 +const MAX_LABEL_LENGTH = 48 + +const MODULE_LABEL_SCHEMA = { + type: 'OBJECT', + properties: { + modules: { + type: 'ARRAY', + items: { + type: 'OBJECT', + properties: { + id: { type: 'STRING' }, + label: { type: 'STRING' }, + }, + required: ['id', 'label'], + }, + }, + }, + required: ['modules'], +} as const + +function moduleSystemPrompt() { + return [ + 'You are Jargons, labelling the modules of a codebase for an architecture diagram.', + 'For each module you are given, write a short label saying what it is responsible for: 2 to 6 words, no trailing period.', + 'Base the label only on the directory path and the file names given. Do not guess at behaviour the names do not support.', + 'If the names do not say enough to be specific, fall back to a plain reading of the directory name.', + 'Return every module id exactly as it was given, and do not invent module ids.', + ].join('\n') +} + +function moduleUserPrompt(repository: string, modules: ModuleLabelInput[]) { + const blocks = modules.map((module) => + [ + `--- MODULE: ${module.id} (${module.files} files) ---`, + module.sampleFiles.slice(0, MAX_SAMPLE_FILES).join('\n'), + ].join('\n'), + ) + return [ + `Repository: ${repository}`, + `Label these ${modules.length} modules.`, + '', + ...blocks, + ].join('\n') +} + +// Only ids that were asked about survive, so a hallucinated module cannot add a +// box to the map. Labels are trimmed and length-capped because the diagram has +// a fixed amount of room for them. +function parseLabels(text: string, asked: Set): Record { + let payload: unknown + try { + payload = JSON.parse(text) + } catch { + return {} + } + + // JSON.parse happily returns null or a bare string, neither of which has a + // `modules` property to read. + if (!payload || typeof payload !== 'object') return {} + + const raw = (payload as { modules?: unknown }).modules + if (!Array.isArray(raw)) return {} + + const labels: Record = {} + for (const item of raw) { + if (!item || typeof item !== 'object') continue + const { id, label } = item as { id?: unknown; label?: unknown } + if (typeof id !== 'string' || typeof label !== 'string') continue + if (!asked.has(id)) continue + const trimmed = label.trim().slice(0, MAX_LABEL_LENGTH) + if (trimmed) labels[id] = trimmed + } + + return labels +} + +export async function labelModules({ + repository, + modules, +}: { + repository: string + modules: ModuleLabelInput[] +}): Promise { + const provider = getOptionalEnv('LLM_PROVIDER', 'gemini') + const model = getOptionalEnv('LLM_MODEL', 'gemini-2.5-flash') + + if (provider !== 'gemini') { + throw new Error(`Unsupported LLM_PROVIDER: ${provider}`) + } + + const result = await callGemini({ + model, + systemPrompt: moduleSystemPrompt(), + userPrompt: moduleUserPrompt(repository, modules), + responseSchema: MODULE_LABEL_SCHEMA, + maxOutputTokens: 2048, + }) + + return { + labels: parseLabels( + result.text, + new Set(modules.map((module) => module.id)), + ), + model, + inputTokens: result.inputTokens, + outputTokens: result.outputTokens, + costUsd: result.costUsd, + } +} diff --git a/src/server/scan-engine/run-scan.ts b/src/server/scan-engine/run-scan.ts index 563a154..a83905e 100644 --- a/src/server/scan-engine/run-scan.ts +++ b/src/server/scan-engine/run-scan.ts @@ -3,22 +3,43 @@ // failure log says which step broke. import { loadDb } from '../../db/load' +import { NO_USAGE, addUsage } from '../llm/usage' import type { LlmUsage } from '../llm/usage' import type { LlmFinding, ReviewSeverity } from '../review-engine/llm' -import { fetchFileContent, fetchRepoTree } from './github' -import type { RepoFile } from './github' -import { scanCodebase } from './llm' +import { selectGraphCandidates, withinCharBudget } from './candidates' +import { fetchFiles, fetchRepoTree } from './github' +import { buildModuleGraph, rankFilesByFanIn } from './graph' +import type { ModuleGraph } from './graph' +import { buildImportEdges, createPathIndex } from './imports' +import type { ImportEdge } from './imports' +import { labelModules, scanCodebase } from './llm' +import type { ModuleLabelInput } from './llm' // Budgets keep a scan fast + free-tier friendly. const MAX_FILES = 20 const MAX_TOTAL_CHARS = 50_000 +// The graph pass only needs the head of each file: that is where import +// statements live, and reading less of each of 200 files keeps the memory +// profile flat. +const GRAPH_HEAD_CHARS = 4_000 + +// File names sent per module when asking for labels. Names only, never +// contents. +const SAMPLE_FILES_PER_MODULE = 8 + export type RunScanInput = { scanId: string installationId: string owner: string repo: string branch: string + // Whether to build the architecture map for this run. Off means the module + // graph and its labelling call are skipped entirely, so a scan for someone + // without the flag costs no extra LLM spend. The import graph itself is still + // built either way — it decides which files get scanned, which is an + // improvement everyone gets. + architectureMap: boolean } export async function runScan(input: RunScanInput): Promise { @@ -38,11 +59,57 @@ export async function runScan(input: RunScanInput): Promise { branch: input.branch, }) + // Which files deserve the 20-file budget is itself a question about the + // codebase, so the dependency graph is built first and gets to answer it. + // Only the ranked candidate list is ever read — never the whole repository. + stage = 'read_graph' + const candidates = selectGraphCandidates(paths) + const heads = await fetchFiles({ + installationId: input.installationId, + owner: input.owner, + repo: input.repo, + branch: input.branch, + paths: candidates, + maxChars: GRAPH_HEAD_CHARS, + }) + + stage = 'build_graph' + const edges = buildImportEdges(heads, createPathIndex(paths)) + + // Fan-in first: a defect in a module twenty files depend on is worth more of + // the budget than one in a leaf nothing imports. stage = 'fetch_files' - const files = await collectFiles(input, paths) + const selected = rankFilesByFanIn(candidates, edges).slice(0, MAX_FILES) + const files = withinCharBudget( + await fetchFiles({ + installationId: input.installationId, + owner: input.owner, + repo: input.repo, + branch: input.branch, + paths: selected, + }), + MAX_TOTAL_CHARS, + ) stage = 'llm_scan' const result = await scanCodebase({ repository, files }) + let usage: LlmUsage = { + inputTokens: result.inputTokens, + outputTokens: result.outputTokens, + costUsd: result.costUsd, + } + + stage = 'architecture_map' + const mapped = input.architectureMap + ? await buildArchitecture({ + repository, + paths, + edges, + findings: result.findings, + graphedFiles: heads.length, + }) + : { architecture: null, usage: NO_USAGE } + usage = addUsage(usage, mapped.usage) stage = 'write_summary' const counts = countBySeverity(result.findings) @@ -54,20 +121,21 @@ export async function runScan(input: RunScanInput): Promise { counts, scannedFiles: files.length, model: result.model, + architecture: mapped.architecture, }, - { - inputTokens: result.inputTokens, - outputTokens: result.outputTokens, - costUsd: result.costUsd, - }, + usage, ) stage = 'complete' console.log('scan.run complete', { scanId: input.scanId, repository, + repoFiles: paths.length, + graphedFiles: heads.length, + importEdges: edges.length, scannedFiles: files.length, findingsCount: result.findings.length, + modules: mapped.architecture?.modules.length ?? 0, }) } catch (error) { const message = error instanceof Error ? error.message : 'Scan run failed' @@ -82,34 +150,107 @@ export async function runScan(input: RunScanInput): Promise { } } -async function collectFiles( - input: RunScanInput, - paths: string[], -): Promise { - const files: RepoFile[] = [] - let totalChars = 0 +// What gets stored on the scan summary for the architecture map. `graphedFiles` +// and `totalFiles` travel with it so the UI can say how much of the repository +// the arrows actually cover, instead of implying they cover all of it. +export type ArchitectureSummary = { + modules: Array + edges: ModuleGraph['edges'] + omittedModules: number + graphedFiles: number + totalFiles: number +} - for (const path of paths) { - if (files.length >= MAX_FILES || totalChars >= MAX_TOTAL_CHARS) { - break +// The one place a model touches the map, and only to name things. A failure here +// costs the labels, never the diagram. +async function buildArchitecture({ + repository, + paths, + edges, + findings, + graphedFiles, +}: { + repository: string + paths: string[] + edges: ImportEdge[] + findings: LlmFinding[] + graphedFiles: number +}): Promise<{ architecture: ArchitectureSummary | null; usage: LlmUsage }> { + const graph = buildModuleGraph({ paths, edges, findings }) + + if (graph.modules.length === 0) { + return { architecture: null, usage: NO_USAGE } + } + + if (graph.unattributedFindings > 0) { + // Every scanned path came from the tree, so a finding that does not map back + // to it means the model rewrote the path it was given. + console.log('scan.architecture unattributed findings', { + repository, + count: graph.unattributedFindings, + }) + } + + let labels: Record = {} + let usage: LlmUsage = NO_USAGE + + try { + const labelled = await labelModules({ + repository, + modules: graph.modules.map( + (module): ModuleLabelInput => ({ + id: module.id, + files: module.files, + sampleFiles: sampleFilesFor(module.id, paths), + }), + ), + }) + labels = labelled.labels + usage = { + inputTokens: labelled.inputTokens, + outputTokens: labelled.outputTokens, + costUsd: labelled.costUsd, } - const content = await fetchFileContent({ - installationId: input.installationId, - owner: input.owner, - repo: input.repo, - branch: input.branch, - path, + } catch (error) { + console.error('scan.architecture labelling failed', { + repository, + error: error instanceof Error ? error.message : String(error), }) - if (!content) continue + } + + return { + architecture: { + modules: graph.modules.map((module) => ({ + ...module, + label: labels[module.id] ?? null, + })), + edges: graph.edges, + omittedModules: graph.omittedModules, + graphedFiles, + totalFiles: paths.length, + }, + usage, + } +} + +// File names inside a module, relative to it, so a label can be written from +// them without sending any file contents. Works for a collapsed module too, +// since a collapsed id is still a prefix of everything it absorbed. +function sampleFilesFor(moduleId: string, paths: string[]): string[] { + const names: string[] = [] - const remaining = MAX_TOTAL_CHARS - totalChars - const slice = - content.length > remaining ? content.slice(0, remaining) : content - files.push({ path, content: slice }) - totalChars += slice.length + for (const path of paths) { + if (names.length >= SAMPLE_FILES_PER_MODULE) break + if (moduleId === '.') { + if (!path.includes('/')) names.push(path) + continue + } + if (path.startsWith(`${moduleId}/`)) { + names.push(path.slice(moduleId.length + 1)) + } } - return files + return names } function countBySeverity( diff --git a/src/server/scan-engine/summary.test.ts b/src/server/scan-engine/summary.test.ts index 0a08251..6dcb469 100644 --- a/src/server/scan-engine/summary.test.ts +++ b/src/server/scan-engine/summary.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' -import { emptyCounts, summaryToCounts } from './summary' +import { emptyCounts, summaryToArchitecture, summaryToCounts } from './summary' describe('summaryToCounts', () => { it('reads a null/undefined summary as all-zero', () => { @@ -52,3 +52,118 @@ describe('summaryToCounts', () => { expect(emptyCounts.critical).toBe(0) }) }) + +describe('summaryToArchitecture', () => { + const module = (id: string) => ({ + id, + label: `${id} does things`, + files: 3, + findings: 0, + counts: { critical: 0, high: 0, medium: 0, low: 0, note: 0 }, + topSeverity: null, + }) + + it('reads a scan that predates the map as having none', () => { + expect(summaryToArchitecture(null)).toBeNull() + expect(summaryToArchitecture({ counts: { high: 1 } })).toBeNull() + }) + + it('reads a non-object summary or architecture as having none', () => { + expect(summaryToArchitecture('nonsense')).toBeNull() + expect(summaryToArchitecture({ architecture: 'nonsense' })).toBeNull() + }) + + it('reads a map with no usable modules as having none', () => { + expect(summaryToArchitecture({ architecture: { modules: [] } })).toBeNull() + expect( + summaryToArchitecture({ architecture: { modules: [{ files: 2 }] } }), + ).toBeNull() + }) + + it('keeps modules and the edges between them', () => { + const architecture = summaryToArchitecture({ + architecture: { + modules: [module('src/routes'), module('src/server')], + edges: [{ from: 'src/routes', to: 'src/server', weight: 4 }], + omittedModules: 2, + graphedFiles: 180, + totalFiles: 940, + }, + }) + + expect(architecture?.modules.map((m) => m.id)).toEqual([ + 'src/routes', + 'src/server', + ]) + expect(architecture?.edges).toEqual([ + { from: 'src/routes', to: 'src/server', weight: 4 }, + ]) + expect(architecture?.omittedModules).toBe(2) + expect(architecture?.graphedFiles).toBe(180) + expect(architecture?.totalFiles).toBe(940) + }) + + it('drops an edge pointing at a module that is not on the map', () => { + const architecture = summaryToArchitecture({ + architecture: { + modules: [module('src/routes')], + edges: [ + { from: 'src/routes', to: 'src/gone', weight: 1 }, + { from: 'src/gone', to: 'src/routes', weight: 1 }, + { from: 'src/routes', to: 'src/routes', weight: 1 }, + ], + }, + }) + + expect(architecture?.edges).toEqual([]) + }) + + it('coerces a missing or malformed weight to a drawable one', () => { + const architecture = summaryToArchitecture({ + architecture: { + modules: [module('a'), module('b')], + edges: [ + { from: 'a', to: 'b' }, + { from: 'b', to: 'a', weight: -7 }, + ], + }, + }) + + expect(architecture?.edges.map((e) => e.weight)).toEqual([1, 1]) + }) + + it('fills in missing counts rather than trusting the stored shape', () => { + const architecture = summaryToArchitecture({ + architecture: { + modules: [{ id: 'src', counts: { high: 2, bogus: 9 } }], + }, + }) + + expect(architecture?.modules[0].counts).toEqual({ + critical: 0, + high: 2, + medium: 0, + low: 0, + note: 0, + }) + expect(architecture?.modules[0].label).toBeNull() + expect(architecture?.modules[0].files).toBe(0) + }) + + it('rejects a severity it does not recognise', () => { + const architecture = summaryToArchitecture({ + architecture: { + modules: [{ id: 'src', topSeverity: 'catastrophic' }], + }, + }) + + expect(architecture?.modules[0].topSeverity).toBeNull() + }) + + it('does not mutate the shared emptyCounts baseline', () => { + summaryToArchitecture({ + architecture: { modules: [{ id: 'src', counts: { critical: 4 } }] }, + }) + expect(emptyCounts.critical).toBe(0) + }) +}) diff --git a/src/server/scan-engine/summary.ts b/src/server/scan-engine/summary.ts index 8f138d6..fddb31f 100644 --- a/src/server/scan-engine/summary.ts +++ b/src/server/scan-engine/summary.ts @@ -2,7 +2,8 @@ // from the DB) into per-severity counts for display. No I/O — unit-tested in // summary.test.ts. -import type { SeverityCounts } from '../../lib/severity' +import { SEVERITIES } from '../../lib/severity' +import type { Severity, SeverityCounts } from '../../lib/severity' export type ScanFindingCounts = SeverityCounts @@ -11,6 +12,7 @@ export type ScanFindingCounts = SeverityCounts export type ScanSummary = { counts?: Partial findings?: Array + architecture?: unknown } export const emptyCounts: ScanFindingCounts = { @@ -34,3 +36,112 @@ export function summaryToCounts(summary: unknown): { : counts.critical + counts.high + counts.medium + counts.low + counts.note return { counts, findingsCount } } + +// The architecture map as stored on a scan summary: modules are directories, +// edges are resolved imports between them, and the label is the only part a +// model wrote. +export type ScanArchitectureModule = { + id: string + label: string | null + files: number + findings: number + counts: ScanFindingCounts + topSeverity: Severity | null +} + +export type ScanArchitectureEdge = { + from: string + to: string + weight: number +} + +export type ScanArchitecture = { + modules: ScanArchitectureModule[] + edges: ScanArchitectureEdge[] + // Modules the map could not show, so the UI can say so rather than implying + // it drew the whole repository. + omittedModules: number + // How many files the arrows were derived from, against the repository total. + graphedFiles: number + totalFiles: number +} + +function readInt(value: unknown): number { + return typeof value === 'number' && Number.isFinite(value) + ? Math.max(0, Math.trunc(value)) + : 0 +} + +function readModule(value: unknown): ScanArchitectureModule[] { + if (!value || typeof value !== 'object') return [] + const raw = value as Record + if (typeof raw.id !== 'string' || !raw.id) return [] + + const counts: ScanFindingCounts = { ...emptyCounts } + const storedCounts = + raw.counts && typeof raw.counts === 'object' + ? (raw.counts as Record) + : {} + for (const severity of SEVERITIES) { + counts[severity] = readInt(storedCounts[severity]) + } + + return [ + { + id: raw.id, + label: typeof raw.label === 'string' && raw.label ? raw.label : null, + files: readInt(raw.files), + findings: readInt(raw.findings), + counts, + topSeverity: SEVERITIES.includes(raw.topSeverity as Severity) + ? (raw.topSeverity as Severity) + : null, + }, + ] +} + +// Scans that ran before the map existed simply have no `architecture` key, and +// read back as null rather than as an empty diagram. +export function summaryToArchitecture( + summary: unknown, +): ScanArchitecture | null { + const parsed = (summary ?? {}) as ScanSummary + const raw = parsed.architecture + if (!raw || typeof raw !== 'object') return null + + const source = raw as Record + const modules = Array.isArray(source.modules) + ? source.modules.flatMap(readModule) + : [] + + if (modules.length === 0) return null + + // An edge pointing at a module that is not on the map would render as an arrow + // into empty space, so both ends have to exist. + const ids = new Set(modules.map((module) => module.id)) + const edges = (Array.isArray(source.edges) ? source.edges : []).flatMap( + (value): ScanArchitectureEdge[] => { + if (!value || typeof value !== 'object') return [] + const edge = value as Record + if (typeof edge.from !== 'string' || typeof edge.to !== 'string') + return [] + if (!ids.has(edge.from) || !ids.has(edge.to)) return [] + if (edge.from === edge.to) return [] + return [ + { + from: edge.from, + to: edge.to, + weight: Math.max(1, readInt(edge.weight)), + }, + ] + }, + ) + + return { + modules, + edges, + omittedModules: readInt(source.omittedModules), + graphedFiles: readInt(source.graphedFiles), + totalFiles: readInt(source.totalFiles), + } +} diff --git a/src/server/scans.ts b/src/server/scans.ts index 546d637..ef69fdf 100644 --- a/src/server/scans.ts +++ b/src/server/scans.ts @@ -3,11 +3,16 @@ import { createServerFn } from '@tanstack/react-start' import { SEVERITIES, severityRank } from '../lib/severity' import type { Severity } from '../lib/severity' import { loadDb } from '../db/load' +import { isArchitectureMapEnabled } from './flags' import { getCurrentUserFromCookie } from './github-auth' -import { summaryToCounts } from './scan-engine/summary' -import type { ScanFindingCounts, ScanSummary } from './scan-engine/summary' +import { summaryToArchitecture, summaryToCounts } from './scan-engine/summary' +import type { + ScanArchitecture, + ScanFindingCounts, + ScanSummary, +} from './scan-engine/summary' -export type { ScanFindingCounts } +export type { ScanArchitecture, ScanFindingCounts } export type CodebaseScanItem = { id: string @@ -74,6 +79,9 @@ export type CodebaseScanFinding = { export type CodebaseScanDetail = CodebaseScanItem & { findings: CodebaseScanFinding[] + // Null for a scan that ran before the map existed, or one whose repository + // had no modules to draw. + architecture: ScanArchitecture | null } const uuidPattern = @@ -158,6 +166,11 @@ export const getCodebaseScan = createServerFn({ method: 'GET' }) startedAt: scan.startedAt?.toISOString() ?? null, completedAt: scan.completedAt?.toISOString() ?? null, findings: summaryToFindings(scan.summary), + // Gated on read as well as on write, so taking someone off the flag hides + // the map on scans that already stored one. + architecture: isArchitectureMapEnabled(currentUser.username) + ? summaryToArchitecture(scan.summary) + : null, } })