diff --git a/.dependency-cruiser.cjs b/.dependency-cruiser.cjs index 894ea29..e633cd3 100644 --- a/.dependency-cruiser.cjs +++ b/.dependency-cruiser.cjs @@ -12,6 +12,46 @@ * recorded here, not an accident that compiles. */ +const { readdirSync, readFileSync, existsSync } = require("node:fs"); +const path = require("node:path"); + +/** + * What "published entry point" MEANS, read from the packages themselves. + * + * Both deep-import rules below used to hardcode `src/index.ts`. That was a proxy for the real + * rule and it was accurate only while every package had exactly one entry — the moment one + * needed a second, the guard reported a violation for something the package had deliberately + * published, and the tempting fix is to bolt the new path into a regex by hand. + * + * `exports` is already the declaration of what a package publishes; Node enforces it at + * resolution time. Reading it here makes the guard agree with the manifest by construction, + * so a package still controls exactly what apps may import — the boundary is unchanged, only + * the place it is written down. Same reasoning as `ALLOWED` above: state it once, mechanically. + * + * A package with no `exports` map falls back to `src/index.ts`, which is what it would have + * been held to before. + */ +function publishedEntryPoints() { + const root = path.join(__dirname, "packages"); + const out = []; + for (const name of readdirSync(root)) { + const manifest = path.join(root, name, "package.json"); + if (!existsSync(manifest)) continue; + const map = JSON.parse(readFileSync(manifest, "utf8")).exports ?? { ".": "./src/index.ts" }; + for (const target of Object.values(map)) { + // Only string targets. A conditional export object would need resolving per condition, + // and no package here uses one — if that changes this must grow, not silently pass. + if (typeof target === "string") out.push(`packages/${name}/${target.replace(/^\.\//, "")}`); + } + } + return out; +} + +/** The entry points as one anchored alternation, for `pathNot`. */ +const ENTRY_POINT_RE = `^(${publishedEntryPoints() + .map((p) => p.replace(/\./g, "\\.")) + .join("|")})$`; + /** @type {Record} */ const ALLOWED = { // Bottom of the stack: pure types, zero deps, zero I/O (LLD §2). @@ -195,7 +235,7 @@ module.exports = { { name: "no-deep-import-across-packages", comment: - "src/index.ts is a package's ONLY public surface (LLD §1.1). Reaching " + + "A package's `exports` map is its ONLY public surface (LLD §1.1). Reaching " + "past it freezes another package's internals into your contract, which " + "is exactly what this refactor exists to undo. Relative imports inside " + "a package are unaffected — the $1 back-reference exempts self.", @@ -203,20 +243,21 @@ module.exports = { from: { path: "^packages/([^/]+)/" }, to: { path: "^packages/[^/]+/src/", - pathNot: ["^packages/$1/", "^packages/[^/]+/src/index\\.ts$"], + pathNot: ["^packages/$1/", ENTRY_POINT_RE], }, }, { name: "no-deep-import-from-app", comment: - "An app consumes a package through its published entry point only " + - "(LLD §1.1).", + "An app consumes a package through an entry point that package DECLARES " + + "in its `exports` map (LLD §1.1). Publishing a second one is a deliberate " + + "act recorded in the manifest, not a path an app may reach for.", severity: "error", from: { path: "^apps/" }, to: { path: "^packages/[^/]+/src/", - pathNot: "^packages/[^/]+/src/index\\.ts$", + pathNot: ENTRY_POINT_RE, }, }, @@ -269,6 +310,28 @@ module.exports = { // `cloneRepo` produces one. What `indexer.ts` does is bulk read-only // enumeration of an already-validated root. It writes nothing. "^packages/analysis/src/indexer\\.ts$", + // `advisories.ts`, 2026-08-09. Same shape and the same argument as `indexer.ts` + // above, restated rather than pointed at, because a waiver nobody can re-derive is + // the kind that outlives its reason: + // + // It READS ONLY — `existsSync`, `readFileSync`, `readdirSync`, `statSync`, and + // nothing else. Checked by grep, not assumed: no write, no mkdir, no unlink. The + // failure this rule exists to prevent is a WRITE that escapes a root, and there is + // no write here to escape with. + // + // The root is validated before it arrives — `vcs.resolveLocalDir` or `cloneRepo` + // produced it — so this is bulk read-only enumeration of a directory the boundary + // already accepted, exactly as `indexer.ts` is. + // + // It cannot route through `fsx` for the reason `indexer.ts` cannot: the + // WorkspaceHandle is async by design (§10.1) and this scan is synchronous + // throughout, so the conversion changes the pipeline's execution shape rather than + // moving a call. + // + // It bounds what it reads — walk depth, manifest count, lockfile bytes — and skips + // symlinks via `withFileTypes`, so a crafted repository cannot use it to read + // outside the tree or to stall an index. + "^packages/analysis/src/advisories\\.ts$", ], }, to: { path: "^(node:)?fs(/promises)?$", dependencyTypes: ["core"] }, @@ -330,11 +393,11 @@ module.exports = { to: { circular: true }, }, - // `no-orphans` is deliberately NOT enabled. It fired on nine modules that + // `no-orphans` is deliberately NOT enabled. It fired on eight modules that // are all genuinely imported (urlSafety, localAccess, basicAuth, colors, - // layout, editorLang, anthropicKeyCheck, GithubMark, postcss.config) — - // false positives caused by unresolved path aliases, plus config files that - // are legitimately never imported. A warn-level rule that cries wolf nine + // layout, editorLang, GithubMark, postcss.config) — false positives caused + // by unresolved path aliases, plus config files that + // are legitimately never imported. A warn-level rule that cries wolf eight // times teaches everyone to ignore the tool, which costs more than the // dead code it would find. Unreferenced-export detection is knip's job // (LLD §13.1 already uses it to find dead shims). diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 88ca91a..5bc9ff2 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -33,7 +33,7 @@ rather than by convention. Browser (Next.js client pages, apps/web/src/app/*) │ fetch ▼ -API routes (apps/web/src/app/api/*/route.ts) ← 29 routes, thin HTTP glue +API routes (apps/web/src/app/api/*/route.ts) ← 30 routes, thin HTTP glue │ ▼ apps/web/src/lib/* web-only concerns: session, authz, store, agents, editor, timeline diff --git a/README.md b/README.md index af7ee05..06ffb94 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ flowchart TB UI["Dashboard · Report tabs · Built-in Editor"] end - subgraph API["API routes — src/app/api/* (14 routes, thin HTTP glue)"] + subgraph API["API routes — src/app/api/* (30 routes, thin HTTP glue)"] IDX["/api/index"] REPOS["/api/repos · /api/repos/:id"] INTEL["/api/repos/:id/intel"] @@ -157,6 +157,61 @@ verified, `1` for a gate that rejected the patch. `--rule ` narrows to one rule, `--file ` to one file, `--json` for machine output. +### CI — fail a change on findings, not on a score + +A score threshold fails a pull request for debt its author did not write. `codegraph ci` gates on +**unaccepted findings at or above a confidence tier**, each carrying one line of evidence you can +check without opening the file. The exit code is the verdict. + +```bash +node apps/cli/bin.mjs ci . --fail-on high --sarif codegraph.sarif --json codegraph-summary.json +``` + +``` +Health 77/100 /path/to/repo +Findings 200 active · 0 accepted +Tiers high 73 · medium 10 · low 117 + +Top rules + 99 security/detect-non-literal-fs-filename low + 36 hardcoded-local-url high + 19 hardcoded-secret high + +Gate --fail-on high + ✗ apps/web/tests/security.test.ts:222 hardcoded-secret high + assigned to `PASSWORD`, 6 chars, generated-looking + +FAIL — 73 unaccepted finding(s) at or above high confidence. +``` + +Adopting it on a codebase that already has findings does not require a thousand-line PR: + +```bash +node apps/cli/bin.mjs baseline . # 3 entries accepting 7 finding(s) +node apps/cli/bin.mjs ci . # exit 0 — the gate now fires on what you add next +``` + +A baseline entry is *rule + file*, deliberately not line — a line-keyed baseline expires on the +next commit that adds an import. Accepted findings are **still reported**: they stay in the +summary, they stay in the SARIF marked `suppressions: external` (code scanning shows them as +dismissed), and they stay out of the Health Score. A baseline that makes findings vanish is an +allowlist nobody reviews. `codegraph-ignore — reason` on the offending line is the +per-finding escape hatch. + +#### In GitHub Actions + +The gate is a CLI, so a workflow is three lines around `codegraph ci`: + +```yaml +- run: npx codegraph ci . --fail-on high --sarif codegraph.sarif +- uses: github/codeql-action/upload-sarif@v3 + with: { sarif_file: codegraph.sarif } +``` + +A packaged composite action lives on `feat/codegraph-action` — it is not merged, because it +runs green locally and produced no artifacts on a hosted runner, and a gate whose own CI cannot +be reproduced is not one to hand anybody else. + ## Installation & deployment | Mode | Command | Notes | @@ -198,12 +253,12 @@ quietly, and a README is the last place that should happen. | What | Result | Source | |---|---|---| -| **Symbol graph extraction** (`expressjs/express@a371447`) | 174 symbols across 159 files plus 113 synthetic module nodes; **299 resolved call edges**, up from 38. Symbols with no inbound edge: **72 of 174 (41%)**, down from 155 (89%). Two defects caused that: call-site attribution required a named enclosing function, discarding 46% of already-resolved calls, and the extractor recorded only `CallExpression`, so rendering a component or passing a callback produced no reference. Precision is measured separately against the TypeScript checker as ground truth — **98.4%** over 2,159 calls where both resolvers answered. Of the symbols still unreferenced on *this* repository, the compiler finds a real call site for only **2%**: the rest are exports, entry points and dynamically-dispatched handlers, not missing edges | `npm run bench` | -| **Agent swarm** (`expressjs/express@a371447`) | 66 findings across 6 active specialists (P0:24 · P1:2 · P2:40 · P3:0); Health Score 89, *simulated* **89 → 90** if P0+P1 are fixed. The projection re-runs the real scorer over the issues that would remain, so it simulates the shipped model rather than estimating — but it is a simulation, not a measurement. The measured result is the row below | `npm run bench` | -| **Verified remediation** (`expressjs/express@a371447`) | 31 fixes across 27 files; Health Score **89 → 96** and issues **62 → 31**, both from an actual re-index of the fixed tree rather than a projection. Verification level **`partial`** — syntax and re-analysis passed, types and tests skipped (express ships no `tsconfig.json`, and gate 3 runs only under `--verify`). Valid, applyable unified git diff | `npm run bench` | +| **Symbol graph extraction** (`expressjs/express@a371447`) | 174 symbols across 159 files plus 112 synthetic module nodes; **301 resolved call edges**, up from 38 — 5 of them self-edges, which the graph used to mis-attribute to module scope. Symbols with no inbound edge: **73 of 174 (42%)**, down from 155 (89%). Two defects caused that: call-site attribution required a named enclosing function, discarding 46% of already-resolved calls, and the extractor recorded only `CallExpression`, so rendering a component or passing a callback produced no reference. Precision is measured separately against the TypeScript checker as ground truth — **98.4%** over 2,159 calls where both resolvers answered. Of the symbols still unreferenced on *this* repository, the compiler finds a real call site for only **2%**: the rest are exports, entry points and framework callbacks, which is the honest ceiling for a resolver with no runtime information | `npm run bench` | +| **Agent swarm** (`expressjs/express@a371447`) | 54 findings after the critic dedupes, across **7 of 7 active specialists** (P0:21 · P1:1 · P2:24 · P3:8); Health Score 89, *simulated* **89 → 90** if P0+P1 are fixed. The architecture specialist reported 0 here until the recursion fix above — express's 5 self-recursive functions were real call cycles the graph could not see. The projection re-runs the real scorer over the issues that would remain, so it simulates the shipped model rather than estimating — but it is a simulation, not a measurement | `npm run bench` | +| **Batch remediation** (`expressjs/express@a371447`) | **0 edits.** The batch fixer ships exactly one codemod — `annotate-empty-catch` — and express contains no empty catch block, so there is nothing for it to patch and it says so rather than manufacturing a diff. Two earlier codemods (`remove-debug-output`, `remove-todo-marker`) were deleted after one deleted a CLI script's only output line; the bar in `remediate-engine/src/types.ts` is that a fix cannot change behaviour AND must remove an issue the scorer counts, and nothing else has cleared it yet. This row published "31 fixes, 89 → 96" for a while after those removals — a stale number is exactly what `npm run bench` exists to catch, and it only catches it when someone runs it | `npm run bench` | | **Graph-RAG context generation** | Query *"render a view template"* → 5 seeds, 11 slices, ~647 tokens, structured prompt | [`apps/web/CODE_INTELLIGENCE.md`](./apps/web/CODE_INTELLIGENCE.md) | | **Memory ceiling under Render's real constraints** | Full pipeline survives indexing `octocat/Hello-World` **and** `expressjs/express` end-to-end inside a container capped at `--memory=512m --cpus=0.5` — the exact config that OOM-killed the server before the fix in [`docs/postmortems/2026-07-10-tree-sitter-oom.md`](./docs/postmortems/2026-07-10-tree-sitter-oom.md) | CI `docker-smoke-test` job, runs on every push | -| **Test suite** | **94 test files** in the workspace (security, indexer, scoring, pillars, coverage, dependencies, codeintel, graph scope, wheel-zoom policy, anonymous-indexing consent, incremental indexing, executor, verify gates, orchestrator, specialists, migrations, tenant-isolation, workspace containment, timeline hash validation, clone redirect refusal, CLI, README claims, and more). 1,286 cases as of 2026-08-08 — the file count is asserted by a test, the case count is a point-in-time figure that moves with every commit | `npm run test` | +| **Test suite** | **120 test files** in the workspace (security, indexer, scoring, pillars, coverage, dependencies, codeintel, graph scope, wheel-zoom policy, anonymous-indexing consent, incremental indexing, executor, verify gates, orchestrator, specialists, migrations, tenant-isolation, workspace containment, timeline hash validation, clone redirect refusal, ask recall, dashboard triage, CLI, README claims, and more). 2,206 cases as of 2026-08-09 — the file count is asserted by a test, the case count is a point-in-time figure that moves with every commit | `npm run test` | | **Security posture (self-audited, tracked openly)** | Baseline **3/10 → 9.1/10**. Phases 0–3 hardening (SSRF guard, local-access gate, security headers, auth gate, cross-tenant isolation fix), then Phase 7 closed **17 of 27** findings from a follow-up deep audit that surfaced **99 issues (5 critical)** across the full stack. Remaining items are tracked, not hidden — plus an independent pen-test pass that verified every control live and fixed a rate-limit `X-Forwarded-For` bypass | [`docs/PROGRESS_TRACKER.md`](./docs/PROGRESS_TRACKER.md), [`docs/AUDIT_2026-07-12.md`](./docs/AUDIT_2026-07-12.md) | ## Comparison with existing tools @@ -232,6 +287,22 @@ Tracked live in [`docs/IMPROVEMENT_PLAN.md`](./docs/IMPROVEMENT_PLAN.md) (the pl - [x] **Phase 0.6 — Multi-tenant isolation** *(pulled forward, was live-severity)*: per-repo ownership, cross-tenant data leak closed - [~] **Phase 4 — Close the agent loop** *(partly shipped)*: the **explicit confirmation gate exists** — every remote mutation now requires `PublishConsent { confirmed: true }`, and no route constructs one, so nothing publishes as shipped. What remains is the endpoint that takes that consent and performs the branch → commit → push → PR, plus the audit trail - [ ] **Phase 5 — Scale & domains** *(stretch)*: a second Tree-sitter language extractor (Python) for AST-grade precision beyond regex, runtime/observability domain (OTel ingestion) +- [x] **Phase 6 — Code-intelligence breadth**: ownership (developer → commit → file → symbol, reviewer recommendation, stale/orphaned areas), APIs as first-class graph entities with endpoint → service → sink flow tracing, inter-procedural taint, dependency advisories (OSV, opt-in), unused-dependency and replacement-impact analysis, PR intelligence over a real diff, and a cross-repository organisational graph — see the table below for what each does and does not know + +### What the intelligence layer actually knows + +Each row is reachable from a route and covered by tests. The **limits** column is the point: every analysis here reports what it could not determine instead of defaulting to a clean answer, because "we did not look" and "there is nothing there" are different claims. + +| Capability | Route | Limits it states about itself | +|---|---|---| +| **Ownership** — authors, per-file shares, bus factor, stale/orphaned areas, symbol-level attribution | `/api/repos/:id/ownership?op=summary\|file\|reviewers\|familiarity` | Shares are of COMMITS, not lines. Symbol attribution intersects historical hunks with CURRENT spans, so a symbol that moved is matched where it is now | +| **Reviewer recommendation** — ownership × recency × co-change | same route, `op=reviewers` | Excludes anyone inactive in the window; each recommendation carries the evidence it was derived from. No history → no recommendation, not a guess | +| **API entities + data flow** — endpoints as graph nodes, endpoint → service → database/fs/network/process flows | `/api/repos/:id/intel?op=endpoints\|flows\|api-impact\|unauth-paths` | `authenticated` is three-valued: `null` means the handler could not be resolved, and only `false` (resolved, no guard) reaches `unauth-paths` | +| **Inter-procedural taint** — untrusted values followed argument-index to parameter-index across calls | `/api/repos/:id/intel?op=taint` | Cannot see aliasing, collections, dynamic dispatch or unresolved cross-module calls. Confidence is derived from edge resolution and chain length. Defended paths are reported as `sanitized`, not dropped | +| **Dependency advisories** — OSV lookup | `/api/repos/:id/dependencies?op=advisories` | **Off by default** (`CG_ENABLE_ADVISORY_LOOKUP`): indexing runs on strangers' repos. Status is `checked` / `unavailable` / `disabled`; only `checked` licenses a conclusion. npm only | +| **Unused / replaceable dependencies** | `/api/repos/:id/dependencies?op=unused\|impact` | Candidates with a confidence and a caveat, never verdicts. Types-only packages, script-invoked CLIs, config-loaded plugins and peer deps are excluded or downgraded by name | +| **PR intelligence** — changed symbols, affected endpoints and DB models, blast radius, relevant tests, reviewers, risk | `/api/repos/:id/pr?base=&head=` | Joined against the LAST index, not `head`. Risk is a weighted sum whose every term is published with its evidence — it ranks, it does not predict | +| **Organisational graph** — cross-repo dependencies, shared libraries, repo cycles, contributors | `/api/org` | An edge is drawn only when one repo's manifest declares the name another depends on. Repos that cannot contribute are listed in `excluded` with a reason | ### Known issues / security status This project audits itself and publishes the results rather than hiding them. A comprehensive follow-up audit ([`docs/AUDIT_2026-07-12.md`](./docs/AUDIT_2026-07-12.md)) found **99 issues (5 critical, 24 high)** beyond what Phases 0–3 already fixed — including a confused-deputy token-relay path in the fix executor and two symlink-escape vectors. **Phase 7 has since closed all 5 criticals and 17 of 27 security findings** (symlink-escape fixes, credential redaction, job-ownership checks, OAuth open-redirect guard, session expiry, rate limiting, and more — each with regression tests), and an independent pen-test pass verified the controls live. The remaining items are testing-debt or deliberate product/infra tradeoffs, all **tracked in the open** ([`docs/PROGRESS_TRACKER.md`](./docs/PROGRESS_TRACKER.md)), not silently patched over. If you're evaluating this for anything beyond local/trusted-host use, read that audit first. diff --git a/apps/cli/package.json b/apps/cli/package.json index 36501b5..4fcec31 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -13,6 +13,7 @@ }, "dependencies": { "@codegraph/analysis": "*", + "@codegraph/analysis-model": "*", "@codegraph/config": "*", "@codegraph/core-domain": "*", "@codegraph/observability": "*", diff --git a/apps/cli/src/ci.ts b/apps/cli/src/ci.ts new file mode 100644 index 0000000..fe1afb3 --- /dev/null +++ b/apps/cli/src/ci.ts @@ -0,0 +1,249 @@ +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import path from "node:path"; +import { indexRepo, parseBaseline } from "@codegraph/analysis"; +import { + activeIssues, + applyBaseline, + buildBaseline, + confidenceTier, + findingKey, + gateFindings, + ruleIdOf, + tallyByRule, + toSarif, + type Baseline, + type ConfidenceTier, + type Issue, + type RuleTally, +} from "@codegraph/analysis-model"; + +/** + * `codegraph ci` / `codegraph baseline` — the gate a pull request runs (review C3). + * + * The product had a Health Score, a SARIF exporter and a baseline format, and no way to put + * any of them in front of a change before it merged. Everything here is assembly of parts + * that already exist; the only decisions it makes are the two a CI story has to make. + * + * DECISION 1 — the exit code is the verdict, and it is derived from `gateFindings`, not from + * the score. A score threshold fails a PR for debt the author did not write; a tier gate fails + * it for findings the author can see, check against `evidence`, and either fix or accept. + * + * DECISION 2 — accepted findings are REPORTED, never dropped. They stay in the SARIF (marked + * suppressed), they are counted in the summary, and they are out of the score. A baseline that + * makes findings vanish is an allowlist nobody reviews. + */ + +/** Mirrors `@codegraph/analysis`'s `BASELINE_FILE`, which its package index does not export. */ +export const BASELINE_FILE = ".codegraph-baseline.json"; + +/** How many gating findings the JSON summary carries. */ +const GATING_LIMIT = 50; + +export interface CiOptions { + readonly repo: string; + readonly failOn: ConfidenceTier; + /** Baseline file, absolute or relative to the repository root. */ + readonly baseline: string; + readonly sarif?: string; + readonly json?: string; +} + +export interface CiFinding { + rule: string; + file: string; + line: number; + title: string; + tier: ConfidenceTier; + severity: number; + confidence?: number; + evidence?: string; +} + +export interface CiSummary { + /** Health Score of the analysed tree — reported, deliberately not gated on. */ + score: number; + repo: string; + failOn: ConfidenceTier; + passed: boolean; + /** Every finding reported, accepted ones included. */ + total: number; + /** Findings that count: reported minus accepted. */ + active: number; + /** Accepted by a baseline entry or an inline `codegraph-ignore`. */ + accepted: number; + /** The subset of `accepted` that the baseline file is responsible for. */ + acceptedByBaseline: number; + /** The baseline actually read, or null when the repository has none. */ + baselineFile: string | null; + /** Active findings per confidence tier; accepted ones are in `accepted`, not here. */ + tiers: Record; + rules: RuleTally[]; + gatingCount: number; + /** The worst `GATING_LIMIT` gating findings — a PR comment shows five of these. */ + gating: CiFinding[]; + sarif: string | null; +} + +/** + * Read a baseline from an explicit PATH. + * + * The pipeline reads the repository root and hardcodes the filename, which cannot answer + * `--baseline ci/accepted.json`; this supplies the path. What counts as a VALID baseline is + * not re-decided here — `parseBaseline` is the one parser, so the gate and the pipeline can + * never disagree about which findings a project has accepted. + */ +function readBaselineAt(file: string): Baseline | null { + if (!existsSync(file)) return null; + try { + return parseBaseline(readFileSync(file, "utf8")); + } catch { + // Unreadable (permissions, a directory, a race with a writer) — same answer as malformed. + return null; + } +} + +/** + * Index once and answer every question the gate asks from that one result. + * + * `indexRepo` runs against the working tree in place — no copy, unlike `runFix`, because + * nothing here writes to the repository and CI has already checked out exactly the commit + * under test. Uncommitted changes are analysed, which is what a developer running this by + * hand before pushing wants. + */ +export async function runCi(opts: CiOptions): Promise { + const repo = path.resolve(opts.repo); + // `--baseline` names a repository-level file, so a relative path resolves against the repo. + const file = path.isAbsolute(opts.baseline) ? opts.baseline : path.join(repo, opts.baseline); + const baseline = readBaselineAt(file); + + const result = await indexRepo(repo); + + // `indexRepo` already applied `/.codegraph-baseline.json`, which is the default and the + // overwhelmingly common case. Re-applying it here is a no-op (the same keys, the same flag); + // applying a NON-default `--baseline` here is the only way it can be honoured at all, since + // the pipeline takes no parameter for it. + const issues = applyBaseline(result.issues, baseline); + + const accepted = issues.filter((i) => i.suppressed); + const acceptedKeys = new Set(baseline?.accepted ?? []); + const gating = gateFindings(issues, opts.failOn); + + const tiers: Record = { high: 0, medium: 0, low: 0 }; + for (const issue of activeIssues(issues)) tiers[confidenceTier(issue)]++; + + const summary: CiSummary = { + score: result.score, + repo, + failOn: opts.failOn, + passed: gating.length === 0, + total: issues.length, + active: issues.length - accepted.length, + accepted: accepted.length, + acceptedByBaseline: accepted.filter((i) => acceptedKeys.has(findingKey(i))).length, + baselineFile: baseline ? path.relative(repo, file) || path.basename(file) : null, + tiers, + // Tallied over ALL findings, accepted included: "this rule fires 40 times and you accepted + // 38 of them" is the sentence that tells a team the rule is miscalibrated, and dropping the + // accepted ones first hides it. + rules: tallyByRule(issues), + gatingCount: gating.length, + gating: [...gating].sort(worstFirst).slice(0, GATING_LIMIT).map(toFinding), + sarif: opts.sarif ?? null, + }; + + if (opts.sarif) { + // Every finding, accepted ones included and marked `suppressions: external` — code scanning + // then shows them as dismissed rather than as never having existed. + writeFileSync( + path.resolve(opts.sarif), + `${JSON.stringify(toSarif(issues, { endTimeUtc: new Date().toISOString() }), null, 2)}\n` + ); + } + if (opts.json) { + writeFileSync(path.resolve(opts.json), `${JSON.stringify(summary, null, 2)}\n`); + } + + return summary; +} + +/** Severity first, then confidence: the five a reviewer reads first must be the worst five. */ +function worstFirst(a: Issue, b: Issue): number { + return ( + b.severity - a.severity || + (b.confidence ?? 0) - (a.confidence ?? 0) || + a.file.localeCompare(b.file) || + a.line - b.line + ); +} + +function toFinding(issue: Issue): CiFinding { + return { + rule: ruleIdOf(issue), + file: issue.file, + line: issue.line, + title: issue.title, + tier: confidenceTier(issue), + severity: issue.severity, + ...(issue.confidence === undefined ? {} : { confidence: issue.confidence }), + ...(issue.evidence === undefined ? {} : { evidence: issue.evidence }), + }; +} + +export interface BaselineOutcome { + /** Absolute path written. */ + file: string; + /** + * Entries in the written baseline — NOT the number of findings accepted. + * + * `findingKey` is rule+file, so one entry can accept a dozen findings. Reporting the entry + * count as "findings accepted" understates the blast radius of the file being written, which + * is the one number the person adopting the gate is deciding on. + */ + entries: number; + /** Findings the written baseline accepts — what `ci` will then report as accepted. */ + covered: number; + /** Entries this run added on top of what the file already held. */ + added: number; + score: number; +} + +/** + * `codegraph baseline` — adopt the gate on an existing codebase without a thousand-line PR. + * + * MERGED with the file already on disk rather than overwriting it. `indexRepo` applies the + * existing baseline before this sees the findings, so everything previously accepted comes back + * SUPPRESSED and is therefore absent from `activeIssues` — writing that set alone would drop + * every prior entry and re-open every finding the team had already signed off. Running the + * command twice must not undo the first run. + * + * A stale entry (the finding was fixed) matches nothing and costs nothing; a dropped entry + * breaks the build. The asymmetry decides it. + */ +export async function runBaseline(opts: { + readonly repo: string; + readonly baseline: string; +}): Promise { + const repo = path.resolve(opts.repo); + const file = path.isAbsolute(opts.baseline) ? opts.baseline : path.join(repo, opts.baseline); + const prior = readBaselineAt(file); + + const result = await indexRepo(repo); + const fresh = buildBaseline(activeIssues(applyBaseline(result.issues, prior))); + const merged = [...new Set([...(prior?.accepted ?? []), ...fresh.accepted])].sort(); + + const written: Baseline = { ...fresh, accepted: merged }; + writeFileSync(file, `${JSON.stringify(written, null, 2)}\n`); + + // Counted over the raw findings rather than by re-applying: `applyBaseline` also preserves + // findings already suppressed by an inline `codegraph-ignore`, and those are not this file's + // doing. Same predicate as `runCi`'s `acceptedByBaseline`, so the two numbers agree. + const keys = new Set(merged); + + return { + file, + entries: merged.length, + covered: result.issues.filter((i) => keys.has(findingKey(i))).length, + added: merged.length - (prior?.accepted.length ?? 0), + score: result.score, + }; +} diff --git a/apps/cli/src/fix.ts b/apps/cli/src/fix.ts index f47335c..974625a 100644 --- a/apps/cli/src/fix.ts +++ b/apps/cli/src/fix.ts @@ -12,7 +12,7 @@ import { parseCheck, type FileChange, } from "@codegraph/remediate-engine"; -import { createSandbox, detectTestRunner, hasTypeConfig } from "@codegraph/sandbox"; +import { createSandbox, detectTestRunner, hasTypeConfig, typescriptCompiler } from "@codegraph/sandbox"; import { buildRecord, describeRecord, @@ -135,7 +135,7 @@ export async function runFix(opts: FixOptions): Promise { gates.push( await syntaxGate(candidate, sandbox, parseCheck, async (abs) => readFileSync(abs, "utf8")) ); - gates.push(await typesGate(sandbox, () => hasTypeConfig(work))); + gates.push(await typesGate(sandbox, () => hasTypeConfig(work), typescriptCompiler)); gates.push( await testsGate(sandbox, { // Both true, and that is the entire point of the CLI. On the developer's own machine diff --git a/apps/cli/src/main.ts b/apps/cli/src/main.ts index ab4a734..cf47c3f 100644 --- a/apps/cli/src/main.ts +++ b/apps/cli/src/main.ts @@ -1,40 +1,71 @@ +import { statSync } from "node:fs"; +import path from "node:path"; +import type { ConfidenceTier } from "@codegraph/analysis-model"; +import { runBaseline, runCi, BASELINE_FILE, type CiSummary } from "./ci"; import { runFix } from "./fix"; -import { color } from "./ui/ansi"; +import { color, padEnd, truncate } from "./ui/ansi"; const { bold, dim, green, red, yellow } = color; /** * `codegraph` — the CLI entry point (LLD §1, `terminal/` → `apps/cli`). * - * Argument parsing is hand-rolled. The surface is one command with five flags, and a parser + * Argument parsing is hand-rolled. The surface is three commands and nine flags, and a parser * dependency for that is more supply chain than the feature is worth in a project whose pitch * includes "no API key, one container". + * + * `--json` is the one flag whose arity depends on the command: a boolean for `fix` (dump the + * outcome to stdout), a path for `ci` (the file the PR comment is built from). Resolved by + * reading the command first, which means options must FOLLOW the command — as every line of + * the usage above shows. `codegraph --json out.json ci` fails loudly with + * `unknown command "out.json"` rather than doing something surprising. */ const USAGE = `${bold("codegraph")} — a codebase workbench ${bold("USAGE")} + codegraph ci [path] [options] + codegraph baseline [path] [options] codegraph fix [path] [options] -${bold("WHY THIS EXISTS")} +${bold("ci")} — fail a change on findings, not on a score + A score threshold fails a pull request for debt its author did not write. This gates on + UNACCEPTED findings at or above a confidence tier, each carrying one line of evidence you + can check without opening the file. Exit ${red("1")} when the gate fires, ${green("0")} when it does not. + + --fail-on high | medium | low (default high) + --baseline accepted findings (default ${BASELINE_FILE}) + --sarif write a SARIF 2.1.0 log — GitHub code scanning reads this + --json write the summary as JSON (what the PR comment is built from) + +${bold("baseline")} — adopt the gate on a codebase that already has findings + Accepts everything present today, so the gate starts green and fires on what you add next. + Accepted findings are still reported and still excluded from the Health Score — the file is + an audit trail, not an allowlist that hides things. + + --baseline file to write (default ${BASELINE_FILE}) + +${bold("fix")} — remediation verified where you already are Gate 3 of verification runs your repository's own test suite, which needs an isolated container. The hosted demo cannot provide one, so it reports ${yellow("verified: partial")}. Here, on your machine, with your toolchain — it reports ${green("verified: full")}. Your source is never modified. You get a diff. -${bold("OPTIONS")} --verify run your test suite as gate 3 (this is the point) --rule only fix findings of this rule --file only fix this file (repo-relative) --test-timeout seconds before gate 3 is SIGKILLed (default 300) --json machine-readable output + +${bold("OPTIONS")} -h, --help this ${bold("EXAMPLES")} + codegraph ci . --fail-on high --sarif codegraph.sarif + codegraph baseline . codegraph fix . --verify codegraph fix ~/src/app --rule legacy/leftover-debug-output --verify - codegraph fix . --verify --json | jq .record.level `; interface Parsed { @@ -43,11 +74,19 @@ interface Parsed { readonly verify: boolean; readonly rule?: string; readonly file?: string; + /** `fix`: dump the outcome to stdout. */ readonly json: boolean; + /** `ci`: write the summary here. */ + readonly jsonPath?: string; + readonly sarif?: string; + readonly failOn: ConfidenceTier; + readonly baseline: string; readonly testTimeout: number; readonly help: boolean; } +const TIERS: Record = { high: "high", medium: "medium", low: "low" }; + export function parseArgs(argv: readonly string[]): Parsed { let command: string | null = null; let target = "."; @@ -56,6 +95,10 @@ export function parseArgs(argv: readonly string[]): Parsed { let help = false; let rule: string | undefined; let file: string | undefined; + let jsonPath: string | undefined; + let sarif: string | undefined; + let failOn: ConfidenceTier = "high"; + let baseline = BASELINE_FILE; let testTimeout = 300; let sawPositional = false; @@ -83,7 +126,22 @@ export function parseArgs(argv: readonly string[]): Parsed { } else if (a === "--verify") { verify = true; } else if (a === "--json") { - json = true; + // Arity by command — see the note at the top of this file. + if (command === "ci") jsonPath = valueFor("--json", ++i); + else json = true; + } else if (a === "--sarif") { + sarif = valueFor("--sarif", ++i); + } else if (a === "--baseline") { + baseline = valueFor("--baseline", ++i); + } else if (a === "--fail-on") { + const raw = valueFor("--fail-on", ++i); + // Rejected rather than defaulted: `--fail-on hgih` quietly gating on `high` is fine, and + // `--fail-on critical` quietly gating on `high` when the author meant "stricter" is not. + const tier = TIERS[raw]; + if (tier === undefined) { + throw new Error(`--fail-on expects high, medium or low, got "${raw}"`); + } + failOn = tier; } else if (a === "--rule") { rule = valueFor("--rule", ++i); } else if (a === "--file") { @@ -114,10 +172,14 @@ export function parseArgs(argv: readonly string[]): Parsed { path: target, verify, json, + failOn, + baseline, testTimeout, help, ...(rule !== undefined ? { rule } : {}), ...(file !== undefined ? { file } : {}), + ...(jsonPath !== undefined ? { jsonPath } : {}), + ...(sarif !== undefined ? { sarif } : {}), }; } @@ -135,12 +197,50 @@ export async function main(argv: readonly string[]): Promise { return args.command === null && !args.help ? 2 : 0; } - if (args.command !== "fix") { + if (args.command !== "fix" && args.command !== "ci" && args.command !== "baseline") { process.stderr.write(`${red("error")}: unknown command "${args.command}". Try --help.\n`); return 2; } + // A gate that green-lights a typo is worse than no gate. `codegraph ci ./aps/web` indexed a + // directory that does not exist, found nothing, scored 100 and exited 0 — the build went + // green on a path that was never analysed. Exit 2, because a bad argument is a usage error + // and not a verdict on any code. + if (!statSync(path.resolve(args.path), { throwIfNoEntry: false })?.isDirectory()) { + process.stderr.write(`${red("error")}: "${args.path}" is not a directory.\n`); + return 2; + } + try { + if (args.command === "ci") { + const summary = await runCi({ + repo: args.path, + failOn: args.failOn, + baseline: args.baseline, + ...(args.sarif !== undefined ? { sarif: args.sarif } : {}), + ...(args.jsonPath !== undefined ? { json: args.jsonPath } : {}), + }); + process.stdout.write(renderCi(summary)); + // EXIT CODE IS THE VERDICT. Everything else this command prints is advisory; the number + // the CI runner reads is this one, and it comes from `gateFindings` alone. + return summary.passed ? 0 : 1; + } + + if (args.command === "baseline") { + const out = await runBaseline({ repo: args.path, baseline: args.baseline }); + // Entries AND findings. A baseline entry is rule+file, so "3 entries" routinely accepts + // twenty findings; printing only the entry count told the reader they were signing off on + // far less than they were. + process.stdout.write( + `${bold("Wrote")} ${out.file}\n` + + `${out.entries} entr${out.entries === 1 ? "y" : "ies"} accepting ${out.covered} finding(s)` + + `${out.added === out.entries ? "" : ` (${out.added} new)`}.\n` + + `${bold("Health")} ${out.score}/100 as analysed, before these were accepted.\n` + + `${dim("Still reported, still out of the Health Score. The gate now fires on what you add next.")}\n` + ); + return 0; + } + const out = await runFix({ repo: args.path, verify: args.verify, @@ -169,6 +269,52 @@ export async function main(argv: readonly string[]): Promise { } } +/** + * The summary a developer reads in a terminal and a reviewer reads in CI logs. + * + * Ordered by what changes a decision: the verdict's inputs first, then the rules costing the + * most, then the individual findings that are actually failing the build. The Health Score is + * printed and NOT the verdict — it is context for the number, not the number. + */ +function renderCi(s: CiSummary): string { + const tier = (t: string, n: number) => (n === 0 ? dim(`${t} ${n}`) : `${t} ${bold(n)}`); + const lines = [ + `${bold("Health")} ${s.score}/100 ${dim(s.repo)}`, + `${bold("Findings")} ${s.active} active · ${s.accepted} accepted${ + s.baselineFile ? dim(` (${s.baselineFile}: ${s.acceptedByBaseline})`) : "" + }`, + `${bold("Tiers")} ${tier("high", s.tiers.high)} · ${tier("medium", s.tiers.medium)} · ${tier("low", s.tiers.low)}`, + ]; + + if (s.rules.length > 0) { + lines.push("", bold("Top rules")); + for (const r of s.rules.slice(0, 5)) { + const acc = r.suppressed > 0 ? dim(` ${r.suppressed} accepted`) : ""; + lines.push(` ${padEnd(String(r.count), 4)}${padEnd(truncate(r.rule, 44), 46)}${dim(r.tier)}${acc}`); + } + } + + lines.push("", `${bold("Gate")} ${dim(`--fail-on ${s.failOn}`)}`); + if (s.passed) { + lines.push(` ${green("✓")} no unaccepted findings at or above ${s.failOn} confidence.`); + } else { + // Five, not all of them: a wall of findings is a wall nobody reads, and the full set is in + // the SARIF and the JSON summary for anyone who wants it. + for (const f of s.gating.slice(0, 5)) { + lines.push(` ${red("✗")} ${bold(`${f.file}:${f.line}`)} ${f.rule} ${dim(f.tier)}`); + lines.push(` ${dim(f.evidence ?? f.title)}`); + } + if (s.gatingCount > 5) lines.push(dim(` … and ${s.gatingCount - 5} more`)); + lines.push( + "", + `${red("FAIL")} — ${s.gatingCount} unaccepted finding(s) at or above ${s.failOn} confidence.`, + dim("Fix them, add `codegraph-ignore` with a reason, or run `codegraph baseline` to accept today's.") + ); + } + + return `${lines.join("\n")}\n`; +} + function render(out: Awaited>): string { const lines: string[] = []; if (out.editCount === 0) { diff --git a/apps/cli/tests/ci.test.ts b/apps/cli/tests/ci.test.ts new file mode 100644 index 0000000..098d7e7 --- /dev/null +++ b/apps/cli/tests/ci.test.ts @@ -0,0 +1,270 @@ +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { BASELINE_FILE, runBaseline } from "../src/ci"; +import { main, parseArgs } from "../src/main"; + +/** + * `codegraph ci` / `codegraph baseline` — the gate a pull request runs. + * + * Everything here goes through `main()` rather than `runCi()`, because the contract a CI runner + * depends on is the EXIT CODE and nothing else. A summary that says FAIL while `main` returns 0 + * is a green build on a rejected change, and only an end-to-end assertion catches that. + */ + +const trees: string[] = []; +afterEach(() => { + for (const t of trees.splice(0)) rmSync(t, { recursive: true, force: true }); + vi.restoreAllMocks(); +}); + +/** + * A repo with findings at three tiers, and two instances of ONE rule in ONE file. + * + * That duplication is load-bearing: `findingKey` is rule+file, so those two findings collapse + * to a single baseline entry. Any fixture with one finding per file would let a baseline that + * counts entries and one that counts findings agree, and the difference is what the adopter is + * actually deciding on. + */ +function repo(): string { + const root = mkdtempSync(path.join(tmpdir(), "cg-ci-test-")); + trees.push(root); + mkdirSync(path.join(root, "src")); + writeFileSync( + path.join(root, "src/db.js"), + [ + 'import fs from "node:fs";', + "", + "export function find(db, name) {", + // codegraph-ignore sql-concatenation — a fixture string, not a query this repo runs + ' return db.query("SELECT * FROM users WHERE name = " + name);', + "}", + "", + "export function audit(db, id) {", + // Same rule, same file -> one baseline entry, two findings. That is the point of it. + // codegraph-ignore sql-concatenation — a fixture string, not a query this repo runs + ' return db.query("SELECT * FROM audit WHERE id = " + id);', + "}", + "", + "export function load(p) {", + // low: security/detect-non-literal-fs-filename + ' return fs.readFileSync(p, "utf8");', + "}", + "", + ].join("\n") + ); + writeFileSync(path.join(root, "package.json"), JSON.stringify({ name: "fx", version: "1.0.0" })); + return root; +} + +/** Run the CLI exactly as `bin.mjs` does and keep what it printed. */ +async function run(...argv: string[]): Promise<{ code: number; out: string; err: string }> { + let out = ""; + let err = ""; + vi.spyOn(process.stdout, "write").mockImplementation((c) => ((out += String(c)), true)); + vi.spyOn(process.stderr, "write").mockImplementation((c) => ((err += String(c)), true)); + try { + return { code: await main(argv), out, err }; + } finally { + vi.restoreAllMocks(); + } +} + +describe("parseArgs — the ci surface", () => { + it("takes a path for --json after `ci` and a boolean after `fix`", () => { + // The one flag whose arity depends on the command. Getting this wrong means `ci --json f` + // swallows `f` as the positional path and analyses a directory that does not exist. + expect(parseArgs(["ci", "--json", "out.json"]).jsonPath).toBe("out.json"); + expect(parseArgs(["ci", "--json", "out.json"]).path).toBe("."); + expect(parseArgs(["fix", "--json"]).json).toBe(true); + expect(parseArgs(["fix", "--json"]).jsonPath).toBeUndefined(); + }); + + it("defaults the gate to high and the baseline to the conventional filename", () => { + const a = parseArgs(["ci"]); + expect(a.failOn).toBe("high"); + expect(a.baseline).toBe(BASELINE_FILE); + }); + + it("rejects an unknown tier rather than quietly gating on high", () => { + // `--fail-on critical` meant "stricter". Defaulting it to `high` would be a weaker gate + // than the author asked for, silently. + expect(() => parseArgs(["ci", "--fail-on", "critical"])).toThrow(/high, medium or low/); + expect(() => parseArgs(["ci", "--fail-on"])).toThrow(/expects a value/); + }); +}); + +describe("the exit code is the verdict", () => { + it("exits 1 on unaccepted high-confidence findings and names them", async () => { + const root = repo(); + const { code, out } = await run("ci", root); + + expect(code).toBe(1); + expect(out).toContain("FAIL"); + // Score, tiers and top rules are the summary's contract, not decoration. + expect(out).toMatch(/Health\s+\d{1,3}\/100/); + expect(out).toMatch(/Tiers\s+high 2 /); + expect(out).toContain("Top rules"); + expect(out).toContain("sql-concatenation"); + // Evidence, so a reviewer can falsify the finding without opening the file. + expect(out).toContain("SELECT * FROM users WHERE name = "); + }); + + it("gates strictly more at a lower tier", async () => { + const root = repo(); + const high = await run("ci", root); + const low = await run("ci", root, "--fail-on", "low"); + + expect(low.code).toBe(1); + const count = (s: string) => Number(/FAIL — (\d+) unaccepted/.exec(s)?.[1]); + expect(count(low.out)).toBeGreaterThan(count(high.out)); + expect(low.out).toContain("--fail-on low"); + }); + + it("exits 2 on a bad invocation — not 1, which a runner reads as a failed gate", async () => { + const { code, err } = await run("ci", "--fail-on", "critical"); + expect(code).toBe(2); + expect(err).toContain("error"); + }); + + it("refuses a path that is not a directory instead of passing on nothing", async () => { + // The real bug: `ci ./aps/web` indexed a directory that does not exist, found no findings, + // scored 100 and exited 0. A green build on a path that was never analysed. + const { code, err } = await run("ci", path.join(tmpdir(), "cg-definitely-not-here")); + expect(code).toBe(2); + expect(err).toContain("is not a directory"); + }); + + it("exits 0 once a baseline accepts today's findings, and reports what it accepted", async () => { + const root = repo(); + expect((await run("ci", root)).code).toBe(1); + + const wrote = await run("baseline", root); + expect(wrote.code).toBe(0); + expect(existsSync(path.join(root, BASELINE_FILE))).toBe(true); + + const { code, out } = await run("ci", root); + expect(code).toBe(0); + expect(out).toContain("no unaccepted findings"); + // Accepted findings are REPORTED, never dropped: the count and the file are both stated. + expect(out).toMatch(/Findings\s+0 active · \d+ accepted \(\.codegraph-baseline\.json: \d+\)/); + expect(out).toContain("accepted"); + }); + + it("counts findings accepted, not baseline entries", async () => { + // The bug this pins: `findingKey` is rule+file, so the two sql-concatenation findings in + // src/db.js are ONE entry. Reporting "1 finding accepted" understated the adoption by half. + const root = repo(); + const out = await runBaseline({ repo: root, baseline: BASELINE_FILE }); + + const written: { accepted: string[] } = JSON.parse( + readFileSync(path.join(root, BASELINE_FILE), "utf8") + ); + expect(written.accepted).toContain("sql-concatenation::src/db.js"); + expect(out.entries).toBe(written.accepted.length); + expect(out.covered).toBeGreaterThan(out.entries); + }); + + it("merges into an existing baseline instead of replacing it", async () => { + // Running it twice must not re-open what the first run accepted: `indexRepo` applies the + // file on disk before this sees the findings, so the fresh set alone would be empty. + const root = repo(); + const first = await runBaseline({ repo: root, baseline: BASELINE_FILE }); + const second = await runBaseline({ repo: root, baseline: BASELINE_FILE }); + + expect(second.entries).toBe(first.entries); + expect(second.added).toBe(0); + }); + + it("honours a non-default --baseline path", async () => { + // `indexRepo` only ever applies `/.codegraph-baseline.json`; a custom path is honoured + // by `runCi` alone, so it is the only thing keeping `--baseline` from being decorative. + const root = repo(); + mkdirSync(path.join(root, "ci")); + await run("baseline", root, "--baseline", "ci/accepted.json"); + + expect(existsSync(path.join(root, "ci/accepted.json"))).toBe(true); + // The default filename was never written, so a plain `ci` must still fail. + expect(existsSync(path.join(root, BASELINE_FILE))).toBe(false); + expect((await run("ci", root)).code).toBe(1); + expect((await run("ci", root, "--baseline", "ci/accepted.json")).code).toBe(0); + }); +}); + +describe("the SARIF log GitHub code scanning reads", () => { + interface Sarif { + $schema: string; + version: string; + runs: { + tool: { driver: { name: string; rules: { id: string; shortDescription: { text: string } }[] } }; + results: { + ruleId: string; + level: string; + locations: { physicalLocation: { artifactLocation: { uri: string }; region: { startLine: number } } }[]; + properties: { evidence?: string }; + suppressions?: { kind: string }[]; + }[]; + invocations: { executionSuccessful: boolean }[]; + }[]; + } + + const read = (f: string): Sarif => JSON.parse(readFileSync(f, "utf8")) as Sarif; + + it("is a well-formed 2.1.0 log whose every ruleId resolves", async () => { + const root = repo(); + const log = path.join(root, "out.sarif"); + await run("ci", root, "--fail-on", "low", "--sarif", log); + const sarif = read(log); + + expect(sarif.version).toBe("2.1.0"); + expect(sarif.$schema).toContain("sarif-2.1.0"); + expect(sarif.runs).toHaveLength(1); + const run0 = sarif.runs[0]!; + expect(run0.tool.driver.name).toBe("CodeGraph"); + expect(run0.results.length).toBeGreaterThan(0); + expect(run0.invocations[0]?.executionSuccessful).toBe(true); + + // A dangling ruleId is the failure GitHub rejects the upload on, and it is invisible in + // any test that only counts results. + const declared = new Set(run0.tool.driver.rules.map((r) => r.id)); + expect(declared.size).toBe(run0.tool.driver.rules.length); + for (const r of run0.results) expect(declared.has(r.ruleId)).toBe(true); + }); + + it("uses the stable rule id, not a slug of the title", async () => { + // Titles are prose and get reworded; a ruleId derived from one silently re-opens every + // alert a team had triaged. `Issue.rule` is the identity the baseline keys on too. + const root = repo(); + const log = path.join(root, "out.sarif"); + await run("ci", root, "--fail-on", "low", "--sarif", log); + const run0 = read(log).runs[0]!; + + const sql = run0.results.find((r) => r.ruleId === "sql-concatenation"); + expect(sql).toBeDefined(); + expect(sql!.locations[0]!.physicalLocation.artifactLocation.uri).toBe("src/db.js"); + expect(sql!.properties.evidence).toContain("SELECT"); + // The declared rule keeps the human title; the id stays machine-stable. + const rule = run0.tool.driver.rules.find((r) => r.id === "sql-concatenation"); + expect(rule?.shortDescription.text).not.toBe("sql-concatenation"); + }); + + it("marks accepted findings suppressed rather than dropping them", async () => { + const root = repo(); + const before = path.join(root, "before.sarif"); + const after = path.join(root, "after.sarif"); + + await run("ci", root, "--fail-on", "low", "--sarif", before); + expect(read(before).runs[0]!.results.some((r) => r.suppressions)).toBe(false); + + await run("baseline", root); + await run("ci", root, "--fail-on", "low", "--sarif", after); + const run0 = read(after).runs[0]!; + + // Same number of results as before the baseline — a baseline that made alerts vanish is an + // allowlist nobody reviews. They are marked, and `external` is what makes code scanning + // show them as dismissed. + expect(run0.results).toHaveLength(read(before).runs[0]!.results.length); + expect(run0.results.every((r) => r.suppressions?.[0]?.kind === "external")).toBe(true); + }); +}); diff --git a/apps/cli/tests/cli.test.ts b/apps/cli/tests/cli.test.ts index 2a1a6fb..fe7e205 100644 --- a/apps/cli/tests/cli.test.ts +++ b/apps/cli/tests/cli.test.ts @@ -4,7 +4,7 @@ import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { parseArgs } from "../src/main"; -import { runFix } from "../src/fix"; +import { buildDiff, runFix } from "../src/fix"; /** * `codegraph fix` — the command that makes `verified: full` reachable (SPIKES.md §2). @@ -28,6 +28,8 @@ function fixture(opts: { testExits?: number } = {}): string { path.join(root, "src/app.js"), [ "export function add(a, b) {", + // Reported, deliberately NOT auto-fixed: the debug-output codemod was withdrawn after + // it deleted a script's intended print(). It must survive every run below. ' console.log("debug", a, b);', " return a + b;", "}", @@ -38,9 +40,15 @@ function fixture(opts: { testExits?: number } = {}): string { " } catch (e) {}", "}", "", - "// TODO: remove before release", + "// TODO: remove before release — also reported, also not auto-fixed", 'export const VERSION = "1.0.0";', "", + "export function alsoRisky() {", + " try {", + ' return JSON.parse("[]");', + " } catch (e) {}", + "}", + "", ].join("\n") ); writeFileSync( @@ -144,12 +152,16 @@ describe("runFix", () => { verify: false, json: false, testTimeout: 60, - rule: "legacy/todo-fixme-marker", + rule: "legacy/empty-catch-block", }); - expect(out.editCount).toBe(1); - expect(out.diff).toMatch(/TODO/); - // The debug line is a different rule and must survive. - expect(out.diff).not.toMatch(/console\.log/); + // Two empty catches in the fixture, both annotated by the one surviving fixer. + expect(out.editCount).toBe(2); + expect(out.diff).toMatch(/intentionally ignored/); + // Other reported classes have no fixer and must survive untouched. They may still appear + // as CONTEXT lines in the diff, so the assertion is on the changed lines only. + const changedLines = out.diff.split("\n").filter((l) => /^[+-]/.test(l) && !/^[+-]{3}/.test(l)); + expect(changedLines.some((l) => l.includes("console.log"))).toBe(false); + expect(changedLines.some((l) => l.includes("TODO"))).toBe(false); }); it("refuses a rule no provider handles instead of running everything", async () => { @@ -190,8 +202,13 @@ describe("the emitted diff", () => { ).not.toThrow(); const after = readFileSync(path.join(root, "src/app.js"), "utf8"); - expect(after).not.toMatch(/console\.log/); - expect(after).not.toMatch(/TODO/); + // What the run DID: both empty catches documented, nothing deleted. + expect(after.match(/intentionally ignored/g)?.length).toBe(2); + // What it deliberately did NOT do. These classes are reported and left to a human since + // the line-deleting codemods were withdrawn; a run that silently removed them again is + // the regression this pins. + expect(after).toMatch(/console\.log/); + expect(after).toMatch(/TODO/); }); it("numbers the new side of each hunk correctly", async () => { @@ -215,9 +232,11 @@ describe("the emitted diff", () => { expect(h.newStart).toBe(h.oldStart - delta); delta += h.oldCount - h.newCount; } - // The fixture deletes lines, so the shift must actually be exercised — otherwise this test - // would pass on a diff where every delta is zero and prove nothing. - expect(delta).toBeGreaterThan(0); + // Every surviving fixer REPLACES lines, so `delta` is zero here by construction. The + // offset arithmetic that a deleting edit exercises is pinned directly against `buildDiff` + // in "shifts later hunks by the lines removed before them" below — via a real fixer it + // would need a line-deleting codemod, and those were withdrawn for good reason. + expect(delta).toBe(0); }); it("produces a multi-hunk diff for edits far apart in one file", async () => { @@ -226,3 +245,54 @@ describe("the emitted diff", () => { expect(out.diff.match(/^@@ /gm)?.length).toBeGreaterThanOrEqual(2); }); }); + +describe("buildDiff hunk offsets", () => { + /** + * The new-side offset arithmetic, pinned directly. + * + * It used to be exercised through `runFix` because a fixer DELETED lines, and deletion is + * what makes the second hunk's `+` start diverge from its `-` start. Those codemods were + * withdrawn (they deleted a script's intended output), so every surviving fixer replaces + * rather than removes and the property became unobservable end to end. It is not + * hypothetical: emitting the before-index on both sides produced a diff `git apply` rejected + * with "patch does not apply", twice, and neither failure was visible by reading the output. + */ + const file = Array.from({ length: 40 }, (_, i) => `line ${i + 1}`); + + it("shifts later hunks by the lines removed before them", () => { + const diff = buildDiff( + new Map([ + [ + "src/a.ts", + { + before: [...file, ""], + // Line 5 (index 4) deleted; line 30 (index 29) replaced, far enough away to force + // a second hunk. + edits: new Map([ + [4, null], + [29, "line 30 (annotated)"], + ]), + }, + ], + ]), + ); + const headers = [...diff.matchAll(/^@@ -(\d+),(\d+) \+(\d+),(\d+) @@$/gm)].map((m) => ({ + oldStart: Number(m[1]), + newStart: Number(m[3]), + })); + expect(headers).toHaveLength(2); + // First hunk: nothing removed before it, so both sides agree. + expect(headers[0]!.newStart).toBe(headers[0]!.oldStart); + // Second hunk: exactly one line was deleted earlier in the file. + expect(headers[1]!.newStart).toBe(headers[1]!.oldStart - 1); + }); + + it("never emits the phantom trailing line a newline-terminated file splits into", () => { + const diff = buildDiff( + new Map([["src/a.ts", { before: [...file, ""], edits: new Map([[39, null]]) }]]), + ); + const header = /^@@ -(\d+),(\d+) \+(\d+),(\d+) @@$/m.exec(diff)!; + // The hunk may not claim more lines than the file actually has. + expect(Number(header[1]) + Number(header[2]) - 1).toBeLessThanOrEqual(file.length); + }); +}); diff --git a/apps/web/DEPLOY.md b/apps/web/DEPLOY.md index a049a5d..41386ff 100644 --- a/apps/web/DEPLOY.md +++ b/apps/web/DEPLOY.md @@ -176,7 +176,8 @@ failures was reproducible locally in seconds, and none of them was reproducible | `NEXT_PUBLIC_APP_URL` | website build + GitHub OAuth callback | `https://app.codegraph.dev` | Marketing "Start Indexing" target; also the base URL used to build the OAuth `redirect_uri` (falls back to the request's own origin if unset) | | `PORT` | app runtime | `4000` | HTTP port | | `HOSTNAME` | app runtime | `0.0.0.0` | Bind address (Docker) | -| `CG_MAX_FILES` | app runtime | `4000` | Max files scanned per repo | +| `CG_MAX_FILES` | app runtime | `4000` | Max files scanned per repo. Reaching it truncates the walk, and the report then says so — the Health Score is labelled a sample rather than presented as the repository's score. `microsoft/TypeScript` holds 39,334 analysable files, so raise this if you want a whole-repository score for a codebase that size. | +| `CG_MAX_FILE_BYTES` | app runtime | `400000` | Per-file size ceiling; anything larger is counted in the coverage report as "over the size cap" and never read. Guards against minified bundles and vendored blobs. | | `CG_CLONE_TIMEOUT_MS` | app runtime | `90000` | git clone timeout | | `CG_DATA_DIR` | app runtime | `./data` | SQLite location | | `CG_ALLOW_LOCAL_ACCESS` | app runtime | unset (= off in production) | Opt in to local-folder indexing + server-side folder browsing on a public deployment. Only set `true` on a trusted, single-operator host. | diff --git a/apps/web/package.json b/apps/web/package.json index 3cb11bf..658f763 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -12,7 +12,6 @@ "cg": "tsx terminal/cli.ts" }, "dependencies": { - "@anthropic-ai/claude-agent-sdk": "^0.3.211", "@codegraph/analysis": "*", "@codegraph/analysis-model": "*", "@codegraph/config": "*", @@ -26,7 +25,6 @@ "@codegraph/sandbox": "*", "@codegraph/vcs": "*", "@codegraph/verify": "*", - "@modelcontextprotocol/sdk": "^1.29.0", "@monaco-editor/react": "^4.7.0", "@types/d3-hierarchy": "^3.1.7", "clsx": "^2.1.1", @@ -40,7 +38,6 @@ "tailwind-merge": "^3.6.0", "tree-sitter-wasms": "0.1.13", "web-tree-sitter": "0.22.6", - "zod": "^4.0.0", "@codegraph/score-engine": "*" }, "overrides": { diff --git a/apps/web/src/app/api/fleet/route.ts b/apps/web/src/app/api/fleet/route.ts index 993a73e..35acace 100644 --- a/apps/web/src/app/api/fleet/route.ts +++ b/apps/web/src/app/api/fleet/route.ts @@ -27,25 +27,33 @@ export const dynamic = "force-dynamic"; */ /** - * Repo name (lowercased) → repo id, keyed by both the full `owner/repo` name - * and its bare last segment, because dependencies are declared as `express` - * while repos are named `expressjs/express`. + * Package name -> the repository that publishes it. * - * Two passes, not one, so precedence is deterministic and does not depend on - * row order: an exact full-name match always beats another repo's bare segment. - * Between two repos with the same bare segment the first row wins, and the - * query's `ORDER BY created_at DESC, id ASC` makes "first" stable. + * WHAT THIS USED TO KEY ON, AND WHY IT FOUND NOTHING + * + * It indexed repositories by DISPLAY NAME (`CodeGraph`, `sindresorhus/slugify`) and then + * matched other repositories' declared DEPENDENCY names (`@codegraph/analysis`, `react`) + * against it. Those are different namespaces, and they coincide only by luck: this monorepo is + * displayed as `CodeGraph` and publishes `@codegraph/analysis`. Measured here: 12 repositories, + * 0 edges — while `/api/org`, a second implementation keyed on manifest names, found 14 across + * the same set. The page said "dependency edges between indexed repositories" and drew none. + * + * Manifest-declared names come first because they are the real answer. The display name is + * kept as a fallback, since a repository whose manifests were never parsed can still be the + * obvious target for `gorilla/mux`, and losing that would trade one silent gap for another. */ function indexByName(repos: readonly FleetRepo[]): Map { const byName = new Map(); + const claim = (key: string, id: string) => { + const k = key.trim().toLowerCase(); + // First claim wins, so a precise key is never overwritten by a looser one added later. + if (k && !byName.has(k)) byName.set(k, id); + }; + for (const r of repos) for (const pkg of r.packageNames) claim(pkg, r.id); + for (const r of repos) claim(r.name, r.id); for (const r of repos) { const name = r.name.toLowerCase(); - if (!byName.has(name)) byName.set(name, r.id); - } - for (const r of repos) { - const name = r.name.toLowerCase(); - const bare = name.slice(name.lastIndexOf("/") + 1); - if (bare && !byName.has(bare)) byName.set(bare, r.id); + claim(name.slice(name.lastIndexOf("/") + 1), r.id); } return byName; } @@ -65,6 +73,12 @@ export async function GET(req: NextRequest) { score: r.score, sourceType: r.sourceType, loc: r.loc, + // Movement since the previous index — what the fleet index below the graph ranks + // by. Computed in one query beside the rows, not per node. + drift: r.drift, + // The fleet index ranks these by score, so a score computed over a truncated walk has + // to say so where it is compared against whole-repository ones. + capHit: r.capHit, }); // Per-source, so one repo listing the same package twice (or listing both diff --git a/apps/web/src/app/api/github/repos/route.ts b/apps/web/src/app/api/github/repos/route.ts index 5d4ecc1..7fc32aa 100644 --- a/apps/web/src/app/api/github/repos/route.ts +++ b/apps/web/src/app/api/github/repos/route.ts @@ -1,6 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { getSession } from "@/lib/session"; import { fetchGithubRepos } from "@/lib/githubOAuth"; +import { logger } from "@codegraph/observability"; export const runtime = "nodejs"; export const dynamic = "force-dynamic"; @@ -15,6 +16,14 @@ export async function GET(req: NextRequest) { const { repos, hasMore } = await fetchGithubRepos(session.accessToken, page); return NextResponse.json({ repos, page, hasMore }); } catch (e) { - return NextResponse.json({ error: e instanceof Error ? e.message : "Failed to list GitHub repos" }, { status: 502 }); + // `fetchGithubRepos` throws its own controlled string, but a transport failure does not: + // undici surfaces `fetch failed` with a cause carrying host, port and TLS detail, and any + // future body-derived message would carry whatever GitHub said. Neither belongs on the + // wire, and the client can act on exactly one thing — retry or re-authenticate. + logger.warn("GitHub repo list failed", { error: e instanceof Error ? e.message : String(e) }); + return NextResponse.json( + { error: "Could not list your GitHub repositories. Sign in again, or retry in a moment." }, + { status: 502 }, + ); } } diff --git a/apps/web/src/app/api/index/route.ts b/apps/web/src/app/api/index/route.ts index 33d0ce0..597cb4c 100644 --- a/apps/web/src/app/api/index/route.ts +++ b/apps/web/src/app/api/index/route.ts @@ -14,6 +14,7 @@ import { ANONYMOUS_INDEXING_DISABLED_MESSAGE, ANONYMOUS_CONSENT_MESSAGE, } from "@/lib/authz"; +import { getVisitorId, mintVisitorId, privateTrialsAvailable, setVisitorCookie } from "@/lib/visitor"; import { rateLimit, clientIp } from "@/lib/rateLimit"; import { logger } from "@codegraph/observability"; @@ -60,15 +61,41 @@ export async function POST(req: NextRequest) { const localPath = (body.localPath || "").trim(); const session = getSession(req); - // Before any work: a signed-out index lands in the shared public bucket, so it needs - // either an account or an explicit acknowledgement. `requiresConsent` is what lets the - // console tell the two refusals apart — one is answerable by the user, the other is - // the operator's decision and only offers sign-in. + /** + * Who owns what this request creates, and whether an anonymous caller may create it at all. + * + * TWO CONTROLS, answering different questions, and both survive here. + * + * `anonymousIndexingAllowed()` is the OPERATOR's switch: a deployment can refuse signed-out + * indexing outright, and no acknowledgement from the caller overrides it. + * + * The visitor cookie is the TENANCY. Signed in → that account, private, as before. Signed + * out → the browser making the request, via a signed cookie, so a trial repository is + * private to whoever ran it instead of landing in a bucket every visitor can read, edit and + * delete. The shared bucket is still reachable and is now what `acknowledgePublic: true` + * MEANS — an explicit request to publish, rather than the only anonymous option. + * + * Consent is therefore demanded only when the result really will be world-readable: when + * the caller asks to publish, or when there is no `CG_SESSION_SECRET` to sign a visitor + * cookie with and the shared bucket is the only place left to put it. Asking for it on a + * run that lands somewhere private would be a warning about a thing that is not happening, + * which is how consent prompts get clicked through. `requiresConsent` still distinguishes + * the two refusals: one is answerable by the user, the other is the operator's decision and + * only offers sign-in. + * + * Without a session secret the old behaviour stands, deliberately: an unsigned owner id is + * one any visitor could claim by editing a cookie, which would be worse than the shared + * bucket precisely because it would look private. + */ + const wantsPublic = body.acknowledgePublic === true; + const existingVisitor = getVisitorId(req); + const trialsAvailable = privateTrialsAvailable(); + if (!session) { if (!anonymousIndexingAllowed()) { return NextResponse.json({ error: ANONYMOUS_INDEXING_DISABLED_MESSAGE }, { status: 401 }); } - if (body.acknowledgePublic !== true) { + if (!trialsAvailable && !wantsPublic) { return NextResponse.json( { error: ANONYMOUS_CONSENT_MESSAGE, requiresConsent: true }, { status: 401 } @@ -76,6 +103,15 @@ export async function POST(req: NextRequest) { } } + const visitorId = + session || wantsPublic || !trialsAvailable ? null : existingVisitor ?? mintVisitorId(); + const ownerId = session?.userId ?? visitorId; + + /** Issue the cookie on the way out, but only when this request minted a NEW identity. */ + const withVisitorCookie = (res: NextResponse): NextResponse => { + if (visitorId !== null && existingVisitor === null) setVisitorCookie(res, visitorId, req); + return res; + }; try { if (localPath) { if (!localAccessAllowed()) { @@ -89,7 +125,7 @@ export async function POST(req: NextRequest) { if (!withinLocalAccessRoot(path.resolve(localPath))) { return NextResponse.json({ error: LOCAL_ACCESS_ROOT_MESSAGE }, { status: 403 }); } - return enqueued(createIndexJob(localPath, "local", undefined, session?.userId ?? null)); + return withVisitorCookie(enqueued(createIndexJob(localPath, "local", undefined, ownerId))); } if (repoUrl) { if (!/^https?:\/\/[\w.-]+\/.+/.test(repoUrl) || !isPublicHttpUrl(repoUrl)) { @@ -98,7 +134,7 @@ export async function POST(req: NextRequest) { { status: 400 } ); } - return enqueued(createIndexJob(repoUrl, "git", session?.accessToken, session?.userId ?? null)); + return withVisitorCookie(enqueued(createIndexJob(repoUrl, "git", session?.accessToken, ownerId))); } return NextResponse.json({ error: "Provide repoUrl or localPath" }, { status: 400 }); } catch (e) { diff --git a/apps/web/src/app/api/jobs/[id]/events/route.ts b/apps/web/src/app/api/jobs/[id]/events/route.ts index c34a941..6cbe773 100644 --- a/apps/web/src/app/api/jobs/[id]/events/route.ts +++ b/apps/web/src/app/api/jobs/[id]/events/route.ts @@ -1,6 +1,8 @@ import { NextRequest, NextResponse } from "next/server"; import { logger } from "@codegraph/observability"; +import { config } from "@codegraph/config"; import { repoAccessDenied } from "@/lib/authz"; +import { clientIp } from "@/lib/rateLimit"; import { getJob } from "@/lib/store"; export const runtime = "nodejs"; @@ -30,6 +32,44 @@ const TICK_MS = 500; * tab closed mid-index does not always deliver an abort promptly. */ const MAX_STREAM_MS = 15 * 60_000; +/** + * Concurrently open streams, globally and per client. + * + * Each one holds a Node handle and a twice-a-second SQLite read for up to fifteen minutes, + * and nothing bounded how many a single client could open. Refusing past the ceiling is safe + * to do bluntly because the client already falls back to polling `/api/jobs/:id` when the + * stream is unavailable — SSE through a proxy was never guaranteed, so the fallback exists + * and is exercised. + * + * Module-level state, which is correct here and not a coincidence: the thing being counted is + * connections held open by THIS process, so a per-process counter is exactly the scope of the + * resource. Nothing to coordinate across instances. + */ +const openStreams = { total: 0, byIp: new Map() }; + +function acquireStream(ip: string): boolean { + if (openStreams.total >= config.maxEventStreams) return false; + const forIp = openStreams.byIp.get(ip) ?? 0; + if (forIp >= config.maxEventStreamsPerIp) return false; + openStreams.total++; + openStreams.byIp.set(ip, forIp + 1); + return true; +} + +function releaseStream(ip: string): void { + openStreams.total = Math.max(0, openStreams.total - 1); + const forIp = (openStreams.byIp.get(ip) ?? 1) - 1; + // Delete rather than keep a zero: the map is keyed by client IP and would otherwise grow + // once per distinct visitor for the life of the process. + if (forIp <= 0) openStreams.byIp.delete(ip); + else openStreams.byIp.set(ip, forIp); +} + +/** Test seam — a module-level counter outlives a test file otherwise. */ +export function resetEventStreamsForTests(): void { + openStreams.total = 0; + openStreams.byIp.clear(); +} export async function GET(req: NextRequest, { params }: { params: Promise<{ id: string }> }) { const { id } = await params; @@ -42,6 +82,17 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id: const denied = repoAccessDenied(req, initial.repoId); if (denied) return denied; + // Taken AFTER the access check so a refused stream cannot be used to probe job ids, and + // released by `finish()` on every exit path below. + const ip = clientIp(req); + if (!acquireStream(ip)) { + logger.warn("SSE stream refused: at capacity", { jobId: id, open: openStreams.total }); + return NextResponse.json( + { error: "Too many open progress streams. The client falls back to polling." }, + { status: 503, headers: { "Retry-After": "5" } }, + ); + } + const encoder = new TextEncoder(); const started = Date.now(); @@ -67,6 +118,9 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id: const finish = (): void => { if (closed) return; closed = true; + // Every exit runs through here — timeout, terminal status, vanished job, client + // disconnect — which is why the slot is released here and nowhere else. + releaseStream(ip); if (timer !== undefined) clearInterval(timer); try { controller.close(); diff --git a/apps/web/src/app/api/org/route.ts b/apps/web/src/app/api/org/route.ts new file mode 100644 index 0000000..1dbcda0 --- /dev/null +++ b/apps/web/src/app/api/org/route.ts @@ -0,0 +1,45 @@ +import { NextRequest, NextResponse } from "next/server"; +import { viewerId } from "@/lib/authz"; +import { getRepo, listRepos } from "@/lib/store"; +import { buildOrgGraph, type OrgRepoInput } from "@/lib/orggraph"; + +export const runtime = "nodejs"; +export const dynamic = "force-dynamic"; + +/** + * GET /api/org — the cross-repository knowledge graph. + * + * TENANCY IS THE WHOLE RISK HERE, and it is why this route resolves each repository through + * `getRepo(id, viewer)` rather than reading rows directly. Every other repo route is scoped to + * one id that `repoAccessDenied` has already checked; this one spans MANY, so a single + * unscoped query would return private repositories to anyone who asked — the exact + * cross-tenant leak this codebase has already had to close once (Phase 0.6). + * + * The scoping is enforced twice over, deliberately. `listRepos(viewer)` applies the + * `owner_id IS NULL OR owner_id = ?` predicate in SQL, and `getRepo(id, viewer)` applies it + * again per row. The second pass is not redundant: it means a future change to the listing + * query cannot silently widen what this route returns, because the detail fetch would still + * refuse. A repo that vanishes between the two calls simply contributes nothing. + */ +export async function GET(req: NextRequest) { + const viewer = viewerId(req); + + const inputs: OrgRepoInput[] = []; + for (const summary of listRepos(viewer)) { + // Only completed indexes carry the manifests and ownership this graph is built from. A + // queued or failed repo is not evidence of anything and is left out rather than reported + // as a node with no edges, which would read as "this repo depends on nothing". + if (summary.status !== "done") continue; + const repo = getRepo(summary.id, viewer); + if (!repo) continue; + inputs.push({ + id: repo.id, + name: repo.name, + dependencies: repo.dependencies, + packageNames: repo.packageNames ?? [], + ownership: repo.ownership, + }); + } + + return NextResponse.json(buildOrgGraph(inputs)); +} diff --git a/apps/web/src/app/api/repos/[id]/assistant/route.ts b/apps/web/src/app/api/repos/[id]/assistant/route.ts deleted file mode 100644 index 96d38f9..0000000 --- a/apps/web/src/app/api/repos/[id]/assistant/route.ts +++ /dev/null @@ -1,117 +0,0 @@ -import { NextRequest, NextResponse } from "next/server"; -import { getWorkspaceDir } from "@/lib/store"; -import { repoAccessDenied } from "@/lib/authz"; -import { getSession } from "@/lib/session"; -import { aiAssistantConfigured, resetAssistantSession, sendMessage } from "@/lib/agents/assistant"; -import { localLlmConfigured, resetLocalAssistantSession, sendLocalMessage } from "@/lib/agents/localAssistant"; -import { effectiveLocalLlmConfig, effectiveClaudeModel, ANONYMOUS_USER_ID } from "@/lib/settings"; -import type { AssistantProvider, AssistantProviders } from "@/lib/types"; - -export const runtime = "nodejs"; -export const dynamic = "force-dynamic"; - -const MAX_MESSAGE_LENGTH = 8000; - -function userIdFrom(req: NextRequest): number { - return getSession(req)?.userId ?? ANONYMOUS_USER_ID; -} - -function providers(userId: number): AssistantProviders { - const local = effectiveLocalLlmConfig(userId); - return { - claude: aiAssistantConfigured(userId), - local: !!local, - claudeModel: effectiveClaudeModel(userId), - localModel: local?.model, - }; -} - -// GET /api/repos/:id/assistant -> { providers } — lets the Editor UI decide -// whether to show the AI Assistant tab, and which backend(s) to offer. -export async function GET(req: NextRequest, { params }: { params: Promise<{ id: string }> }) { - const { id } = await params; - const denied = repoAccessDenied(req, id); - if (denied) return denied; - return NextResponse.json({ providers: providers(userIdFrom(req)) }); -} - -// POST /api/repos/:id/assistant { message: string, provider?: "claude" | "local" } -// -> text/event-stream of AssistantEvent frames for one conversation turn. -// `provider` defaults to Claude if configured, else the local model if that -// alone is configured. -export async function POST(req: NextRequest, { params }: { params: Promise<{ id: string }> }) { - const { id } = await params; - const denied = repoAccessDenied(req, id); - if (denied) return denied; - - const ws = getWorkspaceDir(id); - if (!ws) return NextResponse.json({ error: "Workspace not ready" }, { status: 404 }); - - let body: { message?: unknown; provider?: unknown }; - try { - body = await req.json(); - } catch { - return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 }); - } - const message = typeof body.message === "string" ? body.message.trim() : ""; - if (!message) return NextResponse.json({ error: "message is required" }, { status: 400 }); - if (message.length > MAX_MESSAGE_LENGTH) { - return NextResponse.json({ error: `message is too long (max ${MAX_MESSAGE_LENGTH} characters)` }, { status: 400 }); - } - - const userId = userIdFrom(req); - const available = providers(userId); - const requested = body.provider === "claude" || body.provider === "local" ? body.provider : undefined; - const provider: AssistantProvider | undefined = requested ?? (available.claude ? "claude" : available.local ? "local" : undefined); - if (!provider || !available[provider]) { - return NextResponse.json( - { - error: provider - ? `The "${provider}" AI Assistant provider is not configured on this deployment.` - : "No AI Assistant provider is configured on this deployment (set ANTHROPIC_API_KEY and/or CG_LOCAL_LLM_BASE_URL + CG_LOCAL_LLM_MODEL).", - }, - { status: 501 }, - ); - } - - const hasGit = ws.sourceType === "git"; - const events = provider === "claude" - ? sendMessage(id, ws.dir, hasGit, message, userId, req.signal) - : sendLocalMessage(id, ws.dir, hasGit, message, userId, req.signal); - const encoder = new TextEncoder(); - const stream = new ReadableStream({ - async start(controller) { - try { - for await (const event of events) { - controller.enqueue(encoder.encode(`data: ${JSON.stringify(event)}\n\n`)); - } - } catch (e) { - const msg = e instanceof Error ? e.message : String(e); - controller.enqueue(encoder.encode(`data: ${JSON.stringify({ kind: "error", message: msg })}\n\n`)); - } finally { - controller.close(); - } - }, - }); - - return new NextResponse(stream, { - headers: { - "Content-Type": "text/event-stream", - "Cache-Control": "no-cache, no-transform", - Connection: "keep-alive", - "X-Accel-Buffering": "no", - }, - }); -} - -// DELETE /api/repos/:id/assistant -> closes the repo's in-process assistant -// session(s) ("New chat"); safe to call even if none exist. -export async function DELETE(req: NextRequest, { params }: { params: Promise<{ id: string }> }) { - const { id } = await params; - const denied = repoAccessDenied(req, id); - if (denied) return denied; - const userId = userIdFrom(req); - resetAssistantSession(id, userId); - resetLocalAssistantSession(id, userId); - return NextResponse.json({ ok: true }); -} diff --git a/apps/web/src/app/api/repos/[id]/dependencies/route.ts b/apps/web/src/app/api/repos/[id]/dependencies/route.ts new file mode 100644 index 0000000..fea7421 --- /dev/null +++ b/apps/web/src/app/api/repos/[id]/dependencies/route.ts @@ -0,0 +1,76 @@ +import { NextRequest, NextResponse } from "next/server"; +import { repoAccessDenied, viewerId } from "@/lib/authz"; +import { getRepo } from "@/lib/store"; +import { replacementImpact } from "@codegraph/analysis"; +import { QueryEngine } from "@/lib/codeintel/query"; + +export const runtime = "nodejs"; +export const dynamic = "force-dynamic"; + +/** + * A package name from a query string. + * + * Never reaches a filesystem path or a subprocess — it is compared against strings already in + * the index — but it IS used to build a `RegExp` in `replacementImpact`, so an unbounded value + * is a ReDoS surface. npm's own naming rules are narrower than this and every other ecosystem's + * are too, so anything outside it cannot name a real package. + */ +const PACKAGE = /^[@a-zA-Z0-9._/-]{1,214}$/; + +// GET /api/repos/:id/dependencies?op=advisories | unused | impact&package=… +export async function GET(req: NextRequest, { params }: { params: Promise<{ id: string }> }) { + const { id } = await params; + const denied = repoAccessDenied(req, id); + if (denied) return denied; + const repo = getRepo(id, viewerId(req)); + if (!repo) return NextResponse.json({ error: "Repo not found" }, { status: 404 }); + + const { searchParams } = new URL(req.url); + const op = searchParams.get("op") || "advisories"; + + if (op === "advisories") { + /** + * ABSENT and `status: "disabled"` are DIFFERENT answers and both are returned as-is. + * + * Absent means the run predates advisory lookup. `disabled` means this deployment did not + * ask for one. `unavailable` means we tried and could not. Only `checked` licenses the + * reader to conclude anything about vulnerabilities, and flattening any of the other three + * into an empty list would turn "we did not look" into "there is nothing there" — the one + * failure this whole feature is shaped to prevent. + */ + if (!repo.advisories) { + return NextResponse.json({ error: "Not analysed — re-index this repo" }, { status: 409 }); + } + return NextResponse.json(repo.advisories); + } + + if (op === "unused") { + if (!repo.unusedDependencies) { + return NextResponse.json({ error: "Not analysed — re-index this repo" }, { status: 409 }); + } + // Candidates with a confidence and a caveat, never a verdict — the shape says so and the + // UI must render both, or a 0.15-confidence guess reads like a fact. + return NextResponse.json({ candidates: repo.unusedDependencies }); + } + + if (op === "impact") { + const pkg = searchParams.get("package"); + if (!pkg || !PACKAGE.test(pkg)) { + return NextResponse.json({ error: "Missing or malformed package" }, { status: 400 }); + } + /** + * Import sites are found by scanning source text, and the stored index does not keep file + * CONTENTS — only the graph, the viz nodes and the findings. So this answers from the + * symbol graph alone and says which half it could not compute, rather than silently + * returning an empty `importSites` that reads as "nothing imports this". + */ + const qe = new QueryEngine(repo.symbolGraph); + const impact = replacementImpact(pkg, [], repo.symbolGraph, qe); + return NextResponse.json({ + ...impact, + note: "import sites need file contents, which the stored index does not retain; blast radius is from the symbol graph", + }); + } + + return NextResponse.json({ error: `Unknown op: ${op}` }, { status: 400 }); +} diff --git a/apps/web/src/app/api/repos/[id]/git/route.ts b/apps/web/src/app/api/repos/[id]/git/route.ts index 62792ac..3193fb9 100644 --- a/apps/web/src/app/api/repos/[id]/git/route.ts +++ b/apps/web/src/app/api/repos/[id]/git/route.ts @@ -148,7 +148,19 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id: return NextResponse.json({ ok: true, output: out }); } if (op === "pull") { - const out = await pull(ws.dir); + // Same per-invocation credential as `push`, and the same host pin. The workspace's + // stored remote no longer carries a token (`cloneRepo` strips it), so a private repo + // is pulled with the caller's own PAT or not at all — the credential is never at rest + // on the data disk. + const repo = getRepo(id, viewerId(req)); + if (githubToken && repo?.sourceType === "git" && !isGithubHost(repo.url)) { + return NextResponse.json( + { error: "A GitHub PAT can only be used to pull from a github.com-hosted repo." }, + { status: 400 } + ); + } + const remote = githubToken && repo?.sourceType === "git" ? withToken(repo.url, githubToken) : undefined; + const out = await pull(ws.dir, remote); scheduleReindex(id); return NextResponse.json({ ok: true, output: out }); } diff --git a/apps/web/src/app/api/repos/[id]/intel/route.ts b/apps/web/src/app/api/repos/[id]/intel/route.ts index 0ee1c0a..2733435 100644 --- a/apps/web/src/app/api/repos/[id]/intel/route.ts +++ b/apps/web/src/app/api/repos/[id]/intel/route.ts @@ -1,13 +1,18 @@ import { NextRequest, NextResponse } from "next/server"; import { getRepo } from "@/lib/store"; import { repoAccessDenied, viewerId } from "@/lib/authz"; -import { QueryEngine } from "@/lib/codeintel/query"; +import { QueryEngine, blastRadius, isTestFile, rankUntestedHubs, untestedHubs } from "@/lib/codeintel/query"; import { buildContext } from "@/lib/codeintel/context"; +import { ask, endpointsAffectedBy, unauthenticatedSinkPaths } from "@codegraph/core-graph"; +import { askCorpus } from "@/lib/codeintel/askCorpus"; +import { logger } from "@codegraph/observability"; export const runtime = "nodejs"; export const dynamic = "force-dynamic"; -// GET /api/repos/:id/intel?op=search&q=... | callers | callees | impact | context | cycles | deadcode | hubs +// GET /api/repos/:id/intel?op=search&q=... | callers | callees | impact | blast | untested +// | context | cycles | deadcode | hubs | endpoints | flows | api-impact | unauth-paths | taint +// | ask export async function GET(req: NextRequest, { params }: { params: Promise<{ id: string }> }) { const { id } = await params; const denied = repoAccessDenied(req, id); @@ -33,6 +38,14 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id: return NextResponse.json({ symbol: qe.get(sym), results: qe.members(sym) }); case "impact": return NextResponse.json({ symbol: qe.get(sym), results: qe.impact(sym, 3) }); + case "blast": { + // Four hops rather than impact()'s three: the fourth is usually where a route + // handler or a test finally shows up, which is the answer people came for. + const depth = Math.min(6, Math.max(1, Number(url.searchParams.get("depth")) || 4)); + return NextResponse.json(blastRadius(qe, sym, depth)); + } + case "untested": + return NextResponse.json({ results: rankUntestedHubs(untestedHubs(qe)) }); case "cycles": return NextResponse.json({ cycles: qe.cycles() }); case "deadcode": @@ -41,6 +54,69 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id: return NextResponse.json({ results: qe.hubs(20) }); case "context": return NextResponse.json(buildContext(g, q)); + /** + * The API surface and taint report are computed at INDEX time and stored on the repo, so + * these ops read rather than recompute. `undefined` means the row predates the analysis; + * that is answered as a 409 rather than as an empty result, because "nothing found" and + * "never looked" are different answers and a caller cannot tell them apart from `[]`. + */ + case "endpoints": { + if (!repo.apiSurface) return NextResponse.json({ error: "Not analysed — re-index this repo" }, { status: 409 }); + return NextResponse.json({ endpoints: repo.apiSurface.endpoints, truncated: repo.apiSurface.truncated }); + } + case "flows": { + if (!repo.apiSurface) return NextResponse.json({ error: "Not analysed — re-index this repo" }, { status: 409 }); + return NextResponse.json({ flows: repo.apiSurface.flows, truncated: repo.apiSurface.truncated }); + } + case "api-impact": { + if (!repo.apiSurface) return NextResponse.json({ error: "Not analysed — re-index this repo" }, { status: 409 }); + if (!sym) return NextResponse.json({ error: "Missing symbol" }, { status: 400 }); + return NextResponse.json({ endpoints: endpointsAffectedBy(repo.apiSurface, g, sym) }); + } + case "unauth-paths": { + if (!repo.apiSurface) return NextResponse.json({ error: "Not analysed — re-index this repo" }, { status: 409 }); + // Only endpoints whose handler RESOLVED and carries no guard. An unresolved handler is + // `authenticated: null` and is excluded, so an analysis gap never becomes an accusation. + return NextResponse.json({ paths: unauthenticatedSinkPaths(repo.apiSurface) }); + } + case "taint": { + if (!repo.taint) return NextResponse.json({ error: "Not analysed — re-index this repo" }, { status: 409 }); + return NextResponse.json(repo.taint); + } + /** + * Deterministic natural-language querying. + * + * The compiler runs against the SAME graph, API surface and test predicate the other ops + * use, so an answer here cannot contradict the Impact page or the Agents tab. It never + * reaches the network and consults no model: the failure mode is a refusal, returned as a + * 200 with `ok: false` because "I cannot answer that" is a successful, meaningful response + * to a well-formed request, not an HTTP error. + */ + case "ask": { + if (!q.trim()) return NextResponse.json({ error: "Missing question" }, { status: 400 }); + // Bounded so a pathological question cannot drive the tokeniser or the edit-distance + // sweep across a large graph; the longest supported form is far below this. + if (q.length > 300) return NextResponse.json({ error: "Question too long" }, { status: 400 }); + const answer = ask(q, askCorpus(repo), isTestFile); + /** + * A refusal is the signal that grows the vocabulary. + * + * The synonym dictionary is hand-written, so it only ever contains the words its author + * thought of. Measured against forty-one questions phrased by someone else, four failed + * on missing vocabulary alone - including `blast radius`, which is the product's OWN + * term, printed on the Impact page. Guessing at the gaps is what produced them; this + * records the actual misses so the next dictionary entry is evidence-led. + * + * Only UNCLASSIFIED questions are logged, and only the question text. A classified + * question needs no help, and the text is what the user typed into a box they know is a + * query - the same expectation as any server access log. No repo id, no viewer id: this + * is for reading the vocabulary gap in aggregate, not for tracing a person. + */ + if (!answer.ok && answer.reason === "unclassified") { + logger.info("ask: unclassified question", { question: q }); + } + return NextResponse.json(answer); + } default: return NextResponse.json({ error: `Unknown op: ${op}` }, { status: 400 }); } diff --git a/apps/web/src/app/api/repos/[id]/ownership/route.ts b/apps/web/src/app/api/repos/[id]/ownership/route.ts new file mode 100644 index 0000000..0a90c19 --- /dev/null +++ b/apps/web/src/app/api/repos/[id]/ownership/route.ts @@ -0,0 +1,88 @@ +import { NextRequest, NextResponse } from "next/server"; +import { repoAccessDenied, requireWorkspace, viewerId } from "@/lib/authz"; +import { getRepo } from "@/lib/store"; +import { familiarity, gitCommitsForRoot, isGitRepo, recommendReviewers } from "@codegraph/vcs"; + +export const runtime = "nodejs"; +export const dynamic = "force-dynamic"; + +/** Matches `gitOwnership`'s own window, so "recently active" means one thing everywhere. */ +const WINDOW_DAYS = 180; + +/** Changed files a reviewer query will weigh. A larger list is a rename sweep. */ +const MAX_FILES = 50; + +/** + * GET /api/repos/:id/ownership?op=summary | file&path=… | reviewers&files=a,b | familiarity&author=… + * + * `summary` and `file` read the report computed at index time; `reviewers` and `familiarity` + * need the commit log, because both are questions about a set the caller supplies rather than + * about the repository as a whole, and precomputing every possible answer is not a thing. + */ +export async function GET(req: NextRequest, { params }: { params: Promise<{ id: string }> }) { + const { id } = await params; + const denied = repoAccessDenied(req, id); + if (denied) return denied; + const repo = getRepo(id, viewerId(req)); + if (!repo) return NextResponse.json({ error: "Repo not found" }, { status: 404 }); + + const { searchParams } = new URL(req.url); + const op = searchParams.get("op") || "summary"; + + if (op === "summary" || op === "file") { + // ABSENT is answered as 409, not as an empty report. A repository indexed before ownership + // analysis existed has no data; saying "no owners" would be a claim we cannot support. + if (!repo.ownership) { + return NextResponse.json({ error: "Not analysed — re-index this repo" }, { status: 409 }); + } + if (op === "summary") { + return NextResponse.json({ + authors: repo.ownership.authors, + windowDays: repo.ownership.windowDays, + commitsAnalysed: repo.ownership.commitsAnalysed, + truncated: repo.ownership.truncated, + // The stale/orphaned subset, which is the actionable half of the report. + stale: repo.ownership.files.filter((f) => f.orphaned).slice(0, 100), + }); + } + const path = searchParams.get("path"); + if (!path) return NextResponse.json({ error: "Missing path" }, { status: 400 }); + const entry = repo.ownership.files.find((f) => f.path === path); + if (!entry) return NextResponse.json({ error: "No ownership recorded for that path" }, { status: 404 }); + const symbols = repo.ownership.symbols.filter((s) => s.symbolId.startsWith(`${path}#`)); + return NextResponse.json({ file: entry, symbols }); + } + + // The remaining ops read git, so they need the workspace and the same ownership check again + // — `requireWorkspace` resolves and authorises in one step, which is what stops a handler + // obtaining a directory it was not allowed to open. + const { denied: wsDenied, ws } = requireWorkspace(req, id); + if (wsDenied) return wsDenied; + if (!(await isGitRepo(ws.dir))) { + return NextResponse.json({ error: "Not a git workspace" }, { status: 409 }); + } + const commits = gitCommitsForRoot(ws.dir, { since: `${WINDOW_DAYS}.days.ago` }); + + if (op === "reviewers") { + const files = (searchParams.get("files") ?? "") + .split(",") + .map((f) => f.trim()) + .filter(Boolean) + .slice(0, MAX_FILES); + if (files.length === 0) return NextResponse.json({ error: "Missing files" }, { status: 400 }); + return NextResponse.json({ + reviewers: recommendReviewers(commits, files, { windowDays: WINDOW_DAYS }), + // Stated so an empty list is readable: no history means no recommendation, which is a + // different answer from "nobody is a good reviewer". + commitsAnalysed: commits.length, + }); + } + + if (op === "familiarity") { + const author = searchParams.get("author"); + if (!author) return NextResponse.json({ error: "Missing author" }, { status: 400 }); + return NextResponse.json(familiarity(commits, author)); + } + + return NextResponse.json({ error: `Unknown op: ${op}` }, { status: 400 }); +} diff --git a/apps/web/src/app/api/repos/[id]/pr/route.ts b/apps/web/src/app/api/repos/[id]/pr/route.ts new file mode 100644 index 0000000..3609af4 --- /dev/null +++ b/apps/web/src/app/api/repos/[id]/pr/route.ts @@ -0,0 +1,100 @@ +import { NextRequest, NextResponse } from "next/server"; +import { requireWorkspace } from "@/lib/authz"; +import { getRepo } from "@/lib/store"; +import { viewerId } from "@/lib/authz"; +import { diffRange, gitCommitsForRoot, isGitRepo } from "@codegraph/vcs"; +import { analysePr } from "@/lib/printel/analyse"; +import { parseUnifiedDiff } from "@/lib/printel/diff"; +import { logger } from "@codegraph/observability"; + +export const runtime = "nodejs"; +export const dynamic = "force-dynamic"; + +/** + * History window the reviewer recommendation is drawn from. Six months, matching `gitSignals` + * and `gitOwnership`, so "recently active" means the same thing on every surface. + */ +const WINDOW_DAYS = 180; + +/** + * A git ref supplied by a caller. + * + * Deliberately NARROWER than git's own ref grammar. `git diff` takes options and refs in the + * same argv position, so a value beginning with `-` is an OPTION — `--output=/path` writes a + * file, `--upload-pack=cmd` runs one on a fetch. `packages/vcs`'s `assertRefArg` rejects the + * leading dash at the choke point, and this rejects it earlier and rejects more: a ref here can + * only be a branch, tag or hash-shaped name, because nothing this route legitimately serves + * needs `HEAD@{2}` or a pathspec. Whitelist, not blacklist, for the reason the timeline route + * validates its hash the same way — sanitising downstream is where these get missed. + */ +const REF = /^[A-Za-z0-9][A-Za-z0-9._/-]{0,199}$/; +function isRef(value: string | null): value is string { + // `..` would turn one ref into a range and `.lock`/trailing-dot are invalid to git anyway. + return value !== null && REF.test(value) && !value.includes(".."); +} + +// GET /api/repos/:id/pr?base=&head= +export async function GET(req: NextRequest, { params }: { params: Promise<{ id: string }> }) { + const { id } = await params; + const { denied, ws } = requireWorkspace(req, id); + if (denied) return denied; + if (!(await isGitRepo(ws.dir))) { + return NextResponse.json({ error: "Not a git workspace" }, { status: 409 }); + } + + const { searchParams } = new URL(req.url); + const base = searchParams.get("base"); + const head = searchParams.get("head") ?? "HEAD"; + if (!isRef(base) || !isRef(head)) { + return NextResponse.json( + { error: "Missing or malformed base/head ref" }, + { status: 400 }, + ); + } + + // The graph and API surface come from the stored index, not from a fresh one: analysing a + // diff must not cost a full re-index per request. The consequence — symbol spans are the + // CURRENT tree's, not `head`'s — is documented on `analysePr` and surfaced to the caller + // below rather than hidden. + const repo = getRepo(id, viewerId(req)); + if (!repo) return NextResponse.json({ error: "Repo not found" }, { status: 404 }); + if (repo.status !== "done") { + return NextResponse.json({ error: "Repo not indexed yet" }, { status: 409 }); + } + + try { + const raw = await diffRange(ws.dir, base, head); + const changed = parseUnifiedDiff(raw); + const commits = gitCommitsForRoot(ws.dir, { since: `${WINDOW_DAYS}.days.ago` }); + + const analysis = analysePr({ + base, + head, + changed, + graph: repo.symbolGraph, + apiSurface: repo.apiSurface, + commits, + windowDays: WINDOW_DAYS, + // The indexed file list, for test discovery. `viz` carries every file the scan kept, + // which is the same set the symbol graph was built from. + files: repo.viz.nodes.filter((n) => n.kind === "file").map((n) => ({ rel: n.id })), + }); + + return NextResponse.json({ + ...analysis, + // Named, not implied: the graph this was joined against is the last index, so a PR that + // moves code is matched against where that code is now. + analysedAgainst: { indexedAt: repo.finishedAt, note: "symbol spans are from the latest index, not from `head`" }, + }); + } catch (e) { + // Ref resolution failures are the common case (a branch that does not exist locally), + // but git says so as `Command failed: git -C /app/data/workspaces/ diff …`, which + // hands every repo viewer the absolute workspace path and the internal repo UUID for + // the sake of one word. The actionable half is restated here without them (F023). + logger.warn("pr diff failed", { repoId: id, base, head, error: e instanceof Error ? e.message : String(e) }); + return NextResponse.json( + { error: "Could not diff those refs. Check that both the base and the head exist in this workspace." }, + { status: 400 }, + ); + } +} diff --git a/apps/web/src/app/api/repos/[id]/reindex/route.ts b/apps/web/src/app/api/repos/[id]/reindex/route.ts index 3e0ef98..8f124ed 100644 --- a/apps/web/src/app/api/repos/[id]/reindex/route.ts +++ b/apps/web/src/app/api/repos/[id]/reindex/route.ts @@ -29,7 +29,11 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id: // Same gate as every other workspace-touching route: the check and the workspace // resolution are one step so a handler cannot obtain a directory without having // passed the tenant check. - const { denied } = requireWorkspace(req, id); + // + // `files: false` because re-indexing reads the repository out of git, not off disk. Letting + // this materialise the working tree would reintroduce the 614 MB checkout at exactly the + // moment the design exists to avoid it — and re-index is the one route guaranteed to run. + const { denied } = requireWorkspace(req, id, { files: false }); if (denied) return denied; const result = reindexRepo(id); diff --git a/apps/web/src/app/api/repos/[id]/timeline/route.ts b/apps/web/src/app/api/repos/[id]/timeline/route.ts index 154397a..9c49814 100644 --- a/apps/web/src/app/api/repos/[id]/timeline/route.ts +++ b/apps/web/src/app/api/repos/[id]/timeline/route.ts @@ -1,7 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { requireWorkspace } from "@/lib/authz"; import { TimelineEngine, Strategies } from "@/lib/gitops/timelineApi"; -import { isCommitHash, loadSnapshotCache } from "@/lib/gitops/timelineStore"; +import { loadSnapshotCache } from "@/lib/gitops/timelineStore"; import { isGitRepo } from "@codegraph/vcs"; import { logger } from "@codegraph/observability"; @@ -22,7 +22,18 @@ function badHash(): NextResponse { return NextResponse.json({ error: "Invalid commit hash" }, { status: 400 }); } -// GET /api/repos/:id/timeline?op=metadata|trends|snapshot|compare +/** + * A commit hash from the client becomes a FILENAME in the snapshot cache + * (`data/timeline//.json`) and an argument to `git archive`. Anything that is + * not a hash has no business doing either, so it is rejected before it reaches the engine + * rather than sanitised somewhere downstream. + */ +const HASH = /^[0-9a-f]{7,40}$/; +function isCommitHash(value: string | null): value is string { + return value !== null && HASH.test(value); +} + +// GET /api/repos/:id/timeline?op=metadata|trends|points|snapshot|compare|delta export async function GET(req: NextRequest, { params }: { params: Promise<{ id: string }> }) { const { id } = await params; const { denied, ws } = requireWorkspace(req, id); @@ -46,6 +57,12 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id: return NextResponse.json({ trends }); } + if (op === "points") { + // Cache-only: the structural series plot whatever is already indexed. No build. + const points = await engine.getTrendPoints(); + return NextResponse.json({ points }); + } + if (op === "snapshot") { const hash = searchParams.get("hash"); if (!hash) return NextResponse.json({ error: "Missing hash" }, { status: 400 }); @@ -57,21 +74,29 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id: return NextResponse.json({ snapshot }); } - if (op === "compare") { + if (op === "compare" || op === "delta") { const base = searchParams.get("base"); const head = searchParams.get("head"); if (!base || !head) return NextResponse.json({ error: "Missing base or head hash" }, { status: 400 }); if (!isCommitHash(base) || !isCommitHash(head)) return badHash(); - await engine.ensureSnapshot(base); - await engine.ensureSnapshot(head); - const controller = await engine.getController(); - const evolution = await controller.compare(base, head); - - if (!evolution) { + // `delta` is the one-click "what changed since last index" path: both sides are + // already cached by definition, so it never spends minutes building one. + if (op === "compare") { + await engine.ensureSnapshot(base); + await engine.ensureSnapshot(head); + } + + const delta = await engine.getSnapshotDelta(base, head); + if (!delta) { return NextResponse.json({ error: "One or both snapshots are not cached. Call snapshot first." }, { status: 404 }); } - return NextResponse.json({ evolution }); + + // Both sides are cached (the delta above proves it), so `compare` reads the same two + // files rather than building anything — the evolution narrative comes free either way. + const controller = await engine.getController(); + const evolution = await controller.compare(base, head); + return NextResponse.json({ evolution, delta }); } return NextResponse.json({ error: "Unknown op" }, { status: 400 }); diff --git a/apps/web/src/app/api/repos/[id]/trash/route.ts b/apps/web/src/app/api/repos/[id]/trash/route.ts index 284b2e7..f7d73c5 100644 --- a/apps/web/src/app/api/repos/[id]/trash/route.ts +++ b/apps/web/src/app/api/repos/[id]/trash/route.ts @@ -1,6 +1,5 @@ import { NextRequest, NextResponse } from "next/server"; -import { getWorkspaceDir } from "@/lib/store"; -import { repoAccessDenied } from "@/lib/authz"; +import { repoAccessDenied, requireWorkspace } from "@/lib/authz"; import { listTrash, restoreFromTrash, purgeTrashEntry, emptyTrash } from "@/lib/trash"; import { WorkspacePathError } from "@codegraph/fsx"; import { logger } from "@codegraph/observability"; @@ -45,8 +44,14 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id: const { op, trashId } = body as { op: string; trashId?: string }; try { if (op === "restore") { - const ws = getWorkspaceDir(id); - if (!ws) return NextResponse.json({ error: "Workspace not ready" }, { status: 404 }); + /* + * Through `requireWorkspace`, not `getWorkspaceDir`, so restoring a file lands in a tree + * that actually exists: repositories are cloned `--no-checkout` and materialised on + * first file access. This was the one route reaching past the shared guard, which also + * meant it was the one route not getting the guard's access semantics. + */ + const { denied: noWs, ws } = requireWorkspace(req, id); + if (noWs) return noWs; if (!trashId) return NextResponse.json({ error: "Missing trashId" }, { status: 400 }); return NextResponse.json({ ok: true, entry: restoreFromTrash(id, ws.dir, trashId) }); } diff --git a/apps/web/src/app/api/settings/assistant/route.ts b/apps/web/src/app/api/settings/assistant/route.ts deleted file mode 100644 index 9063194..0000000 --- a/apps/web/src/app/api/settings/assistant/route.ts +++ /dev/null @@ -1,134 +0,0 @@ -import { randomUUID } from "node:crypto"; -import { NextRequest, NextResponse } from "next/server"; -import { getSession } from "@/lib/session"; -import { githubOAuthConfigured } from "@/lib/githubOAuth"; -import { deleteLocalProvider, saveLocalProvider, setAssistantSettings, applyLocalProvider, viewAssistantSettings, ANONYMOUS_USER_ID } from "@/lib/settings"; -import { verifyAnthropicApiKey } from "@/lib/anthropicKeyCheck"; -import { rateLimit, clientIp } from "@/lib/rateLimit"; - -export const runtime = "nodejs"; -export const dynamic = "force-dynamic"; - -function unauthorized(req: NextRequest): NextResponse | null { - if (githubOAuthConfigured() && !getSession(req)?.userId) { - return NextResponse.json({ error: "Unauthorized. Please sign in." }, { status: 401 }); - } - return null; -} - -// Every value here is scoped to the calling account: `userId` derives from -// the signed-in session (falling back to the shared ANONYMOUS_USER_ID -// bucket only when GitHub sign-in isn't configured at all, or the -// deployment is genuinely unauthenticated). A saved key/model/profile is -// therefore visible and editable only from the GitHub account that saved -// it -- never a different signed-in user, never overwritten by one. -function userIdFrom(req: NextRequest): number { - return getSession(req)?.userId ?? ANONYMOUS_USER_ID; -} - -export async function GET(req: NextRequest) { - const denied = unauthorized(req); - if (denied) return denied; - return NextResponse.json(viewAssistantSettings(userIdFrom(req))); -} - -export async function POST(req: NextRequest) { - const denied = unauthorized(req); - if (denied) return denied; - // The key-verification call below reaches out to Anthropic on the caller's behalf, so an - // unthrottled POST loop turns this route into an outbound request amplifier (and, on a - // deployment where sign-in is not configured, an anonymous one). Same limiter, same - // shape as /api/browse and /api/index. - const limited = rateLimit(`settings:${clientIp(req)}`, { capacity: 20, windowMs: 60_000 }); - if (!limited.ok) { - return NextResponse.json( - { error: "Too many settings updates. Try again shortly." }, - { status: 429, headers: { "Retry-After": String(limited.retryAfter) } } - ); - } - const userId = userIdFrom(req); - let body: Record; - try { - body = await req.json(); - } catch { - return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 }); - } - - const patch: Parameters[0] = {}; - - if (typeof body.anthropicApiKey === "string" || body.anthropicApiKey === null) { - patch.anthropicApiKey = body.anthropicApiKey; - } - // Verify a newly pasted Anthropic key against Anthropic's own API before - // ever persisting it -- previously any string was accepted silently and - // only failed deep inside a real chat turn ("Invalid API key"), with no - // way to tell a CodeGraph bug from a bad paste. A confirmed-invalid key - // (401/403) is rejected here with Anthropic's own error text; a network - // failure verifying it is NOT evidence the key is bad, so it still saves. - if (typeof patch.anthropicApiKey === "string" && patch.anthropicApiKey.trim()) { - // Bounded. `fetch` has no default timeout, so an Anthropic endpoint that accepts the - // connection and then stalls (a captive portal, a blocked egress path that blackholes - // rather than resets) left this await pending forever — and with it the user's - // settings save, holding a request handler open with no way to recover. The helper - // already takes a signal; nothing was passing one. An abort lands in its catch and is - // reported as `network`, which by design does not block the save. - const check = await verifyAnthropicApiKey(patch.anthropicApiKey.trim(), AbortSignal.timeout(8_000)); - if (!check.ok && check.reason === "invalid") { - return NextResponse.json({ error: `Anthropic rejected this API key: ${check.message}` }, { status: 400 }); - } - } - if (typeof body.claudeModel === "string" || body.claudeModel === null) { - patch.claudeModel = body.claudeModel; - } - if (typeof body.useClaudeSubscription === "boolean") { - patch.useClaudeSubscription = body.useClaudeSubscription ? "true" : null; - } - if (typeof body.localBaseUrl === "string" || body.localBaseUrl === null) { - patch.localBaseUrl = body.localBaseUrl; - } - if (typeof body.localModel === "string" || body.localModel === null) { - patch.localModel = body.localModel; - } - if (typeof body.localApiKey === "string" || body.localApiKey === null) { - patch.localApiKey = body.localApiKey; - } - if (Array.isArray(body.localModelList)) { - const cleaned = body.localModelList.filter((m): m is string => typeof m === "string" && m.trim().length > 0); - patch.localModelList = cleaned.length > 0 ? JSON.stringify(cleaned) : null; - } else if (body.localModelList === null) { - patch.localModelList = null; - } - - // Saved local-model provider profiles -- checked before the plain field - // patch so a single POST can either mutate the active config directly, or - // manage/apply a saved preset, without callers needing two round trips. - if (body.saveProvider && typeof body.saveProvider === "object") { - const p = body.saveProvider as Record; - const name = typeof p.name === "string" ? p.name.trim() : ""; - const baseUrl = typeof p.baseUrl === "string" ? p.baseUrl.trim() : ""; - if (!name || !baseUrl) { - return NextResponse.json({ error: "Profile name and base URL are required" }, { status: 400 }); - } - const models = Array.isArray(p.models) ? p.models.filter((m): m is string => typeof m === "string" && m.trim().length > 0) : []; - saveLocalProvider({ - id: typeof p.id === "string" && p.id ? p.id : randomUUID(), - name, - baseUrl, - apiKey: typeof p.apiKey === "string" && p.apiKey ? p.apiKey : null, - models, - }, userId); - } - if (typeof body.deleteProviderId === "string") { - deleteLocalProvider(body.deleteProviderId, userId); - } - if (typeof body.useProviderId === "string") { - if (!applyLocalProvider(body.useProviderId, userId)) { - return NextResponse.json({ error: "Provider profile not found" }, { status: 404 }); - } - } - - setAssistantSettings(patch, userId); - - // Return updated view - return NextResponse.json(viewAssistantSettings(userId)); -} diff --git a/apps/web/src/app/api/settings/models/route.ts b/apps/web/src/app/api/settings/models/route.ts deleted file mode 100644 index 4a00359..0000000 --- a/apps/web/src/app/api/settings/models/route.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { NextRequest, NextResponse } from "next/server"; -import { getSession } from "@/lib/session"; -import { githubOAuthConfigured } from "@/lib/githubOAuth"; -import { effectiveLocalLlmConfig, effectiveLocalModelList, ANONYMOUS_USER_ID } from "@/lib/settings"; - -export const runtime = "nodejs"; -export const dynamic = "force-dynamic"; - -// By default returns the user's curated model list (added/removed from -// /settings) so the chat panel's dropdown stays short and intentional -// instead of dumping a provider's full, often huge, raw catalog. Pass -// ?discover=true to bypass the curation and fetch the provider's live -// /v1/models list instead -- used by the "Discover from server" picker on -// the Settings page when the user wants to browse what's actually available. -export async function GET(req: NextRequest) { - // Same account-scoping as /api/settings/assistant: this reads the - // caller's own saved local-model config, so it needs the same gate. - if (githubOAuthConfigured() && !getSession(req)?.userId) { - return NextResponse.json({ error: "Unauthorized. Please sign in." }, { status: 401 }); - } - const userId = getSession(req)?.userId ?? ANONYMOUS_USER_ID; - const curated = effectiveLocalModelList(userId); - const discover = req.nextUrl.searchParams.get("discover") === "true"; - if (curated.length > 0 && !discover) { - return NextResponse.json({ models: curated, curated: true }); - } - - const config = effectiveLocalLlmConfig(userId); - if (!config) return NextResponse.json({ models: curated, curated: curated.length > 0 }); - - try { - const baseUrl = config.baseUrl.replace(/\/+$/, ""); - const res = await fetch(`${baseUrl}/models`, { - headers: config.apiKey && config.apiKey !== "local" ? { Authorization: `Bearer ${config.apiKey}` } : {} - }); - if (!res.ok) return NextResponse.json({ models: curated, curated: curated.length > 0 }); - - const data = await res.json() as { data?: Array<{ id: string }> }; - const models = data.data?.map(m => m.id) || []; - return NextResponse.json({ models, curated: false }); - } catch { - return NextResponse.json({ models: curated, curated: curated.length > 0 }); - } -} diff --git a/apps/web/src/app/dashboard/page.tsx b/apps/web/src/app/dashboard/page.tsx index 2ec8e50..9ebf753 100644 --- a/apps/web/src/app/dashboard/page.tsx +++ b/apps/web/src/app/dashboard/page.tsx @@ -2,8 +2,9 @@ import { useEffect, useState } from "react"; import Link from "next/link"; -import { ArrowRight, FolderGit2, Loader2, Network, Trash2 } from "lucide-react"; -import { fetchRepos, deleteRepo } from "@/lib/api"; +import { ArrowRight, FolderGit2, Loader2, Network, RotateCw, Trash2 } from "lucide-react"; +import { fetchRepos, deleteRepo, startIndex } from "@/lib/api"; +import { plural } from "@/lib/plural"; import type { RepoSummary } from "@/lib/types"; import { CountUp, Reveal, Stagger, StaggerItem } from "@/components/motion/primitives"; import { ScoreDial } from "@/components/ScoreDial"; @@ -87,9 +88,50 @@ const BTN_GHOST = type Order = "risk" | "recent"; +/** + * Which repositories compete for attention, and which are merely dead entries. + * + * Default order is RISK, not recency: the question this page exists to answer is "where do I + * look first", and insertion order answers a different one. In-flight rows sort to the top of + * the risk view because an index still running is the most urgent thing on the page and has no + * score to rank it by. + * + * FAILED ROWS ARE NOT RANKED AT ALL, and that is a correction. They used to sort above + * everything for the same "no score" reason. Measured on a real dashboard: three dead entries + * — a typo'd URL from the previous day among them — sat above every repository the reader + * actually works on, 4 of 19 rows outranking the other 15. A run that never finished is + * urgent; a run that finished by failing yesterday is a dead entry needing dismissal or + * another attempt. They get their own group, where they can be acted on without competing. + * + * Pure and exported because the partition is the part that can be wrong in a way users see, + * and a rendered component is a bad place to prove it is not. + */ +export function triage( + repos: readonly RepoSummary[] | null, + order: Order, +): { live: RepoSummary[]; failed: RepoSummary[] } { + const recent = (a: RepoSummary, b: RepoSummary) => + (b.finishedAt ?? b.createdAt) - (a.finishedAt ?? a.createdAt); + // `filter` allocates, so neither branch can reach the caller's array — no defensive copy. + const all = repos ?? []; + return { + live: all + .filter((r) => r.status !== "error") + .sort((a, b) => + order === "recent" + ? recent(a, b) + : (a.status !== "done" ? -1 : (a.score ?? 101)) - (b.status !== "done" ? -1 : (b.score ?? 101)), + ), + // Always newest-first: there is no risk to rank, and the one you just tried is the one + // you are still thinking about. + failed: all.filter((r) => r.status === "error").sort(recent), + }; +} + export default function DashboardPage() { const [repos, setRepos] = useState(null); const [deletingId, setDeletingId] = useState(null); + const [retryingId, setRetryingId] = useState(null); const [order, setOrder] = useState("risk"); async function handleDelete(id: string, name: string) { @@ -105,6 +147,36 @@ export default function DashboardPage() { } } + /** + * Another attempt at a repository whose index failed. + * + * Re-submits the URL rather than calling `/api/repos/:id/reindex`. Verified, not assumed: + * re-index answers 404 "Workspace not ready" for a failed row, and correctly so — the clone + * never landed, so there is nothing on disk to read again. + * + * THE OLD ROW IS THEN RETIRED, because `/api/index` does NOT reuse it. Measured: retrying + * left two rows for the same URL, both errored, so the dashboard grew a duplicate every time + * someone tried again — turning a fix for clutter into a source of it. Deleting only when + * the id actually changed keeps this correct if that ever starts reusing: superseding a row + * is the intent, and deleting the id the new job is running under would kill the retry. + * + * Worth having because the common cause is transient or trivially correctable — a rate + * limit, a private repo before signing in, a typo already visible in the row — and the URL + * is right there. Without it the only path forward is retyping it on another page. + */ + async function handleRetry(repo: RepoSummary) { + setRetryingId(repo.id); + try { + const { repoId } = await startIndex({ repoUrl: repo.url }); + if (repoId !== repo.id) await deleteRepo(repo.id); + setRepos(await fetchRepos()); + } catch (err) { + window.alert(err instanceof Error ? err.message : "Failed to start indexing"); + } finally { + setRetryingId(null); + } + } + useEffect(() => { let active = true; const load = async () => { @@ -134,20 +206,7 @@ export default function DashboardPage() { ? scored.reduce((lo, r) => ((r.score ?? 100) < (lo.score ?? 100) ? r : lo)) : null; - /** - * Default order is RISK, not recency. - * - * The question this page exists to answer is "where do I look first", and - * insertion order answers a different one. In-flight and failed rows sort to the - * top of the risk view because an index that never finished is the most urgent - * thing on the page and has no score to rank it by. - */ - const rows = [...(repos ?? [])].sort((a, b) => { - if (order === "recent") return (b.finishedAt ?? b.createdAt) - (a.finishedAt ?? a.createdAt); - const rank = (r: RepoSummary) => - r.status === "error" ? -2 : r.status !== "done" ? -1 : (r.score ?? 101); - return rank(a) - rank(b); - }); + const { live, failed } = triage(repos, order); const meanBand = band(mean); const meanBandColor = meanBand.color; @@ -331,7 +390,7 @@ export default function DashboardPage() { - {rows.map((r) => { + {live.map((r) => { const processing = r.status !== "done" && r.status !== "error"; const clickable = r.status === "done"; const b = band(r.score); @@ -382,6 +441,28 @@ export default function DashboardPage() { lowest )} + {/* A ranked table that puts a sampled score beside whole-repository + ones without saying so is the misleading case ADR-008 names. The + word carries the meaning; the colour only reinforces it. */} + {r.capHit && ( + + sample + {/* The row is `pointer-events-none`, so a `title` tooltip would + never open — the explanation goes where it can still be read. */} + + {" "} + — the walk stopped at the CG_MAX_FILES cap, so this score was + computed over part of the repository + + + )} {r.sourceType} @@ -446,6 +527,52 @@ export default function DashboardPage() { })} + + {/* + * Below the list, not inside it. These have no score to rank and no page to open; + * what they need is another attempt or removal, which is all this group offers. + */} + {failed.length > 0 && ( +
+

+ {plural(failed.length, "repository", "repositories")} failed to index · not scored, not counted +

+
+ {failed.map((r) => ( +
+
+

{r.name}

+ {/* The URL is the diagnosis for the most common cause: a typo you can + see the moment it is put in front of you. */} +

{r.url}

+
+ {ago(r.finishedAt ?? r.createdAt)} +
+ + +
+
+ ))} +
+
+ )} )} diff --git a/apps/web/src/app/fleet/page.tsx b/apps/web/src/app/fleet/page.tsx index 46a61c2..a0e54a3 100644 --- a/apps/web/src/app/fleet/page.tsx +++ b/apps/web/src/app/fleet/page.tsx @@ -1,6 +1,7 @@ "use client"; import { useEffect, useState } from "react"; +import { rankByDrift } from "@/lib/fleet-drift"; import Link from "next/link"; import { ArrowLeft, ArrowRight, Loader2, Network } from "lucide-react"; import type { FleetGraph } from "@/lib/types"; @@ -33,9 +34,15 @@ function scoreColor(s: number | null): string { return "text-[var(--coral-text)]"; } -/** One grid track definition shared by the header row and every data row. */ +/** + * One grid track definition shared by the header row and every data row. + * + * At (); for (const e of graph.edges) outDegree.set(e.source, (outDegree.get(e.source) ?? 0) + 1); + // The order the table is drawn in: biggest movers since each repo's previous index. + // Same comparator the dashboard's ranked table uses, so the two pages cannot disagree + // about which repository is first. + const ranked = rankByDrift(graph.nodes); + + // The mean is a secondary reading, not the headline: it says how the estate is doing + // overall and nothing at all about where to look next, which is what the ranking above + // is for. + const scored = graph.nodes.filter((n) => n.score !== null); + const mean = scored.length + ? Math.round(scored.reduce((acc, n) => acc + (n.score ?? 0), 0) / scored.length) + : null; + return (
- {n.name} + + {n.name} + {/* Same marker the dashboard row carries, for the same reason: this list is + ranked by score, and a truncated walk's score is not comparable with a + whole repository's without saying so. */} + {n.capHit && ( + + sample + + {" "} + — the walk stopped at the CG_MAX_FILES cap, so this score was computed + over part of the repository + + + )} + {n.url} {n.sourceType === "git" ? "git" : "local"} diff --git a/apps/web/src/app/page.tsx b/apps/web/src/app/page.tsx index 23f87f6..f28091f 100644 --- a/apps/web/src/app/page.tsx +++ b/apps/web/src/app/page.tsx @@ -14,11 +14,38 @@ import { CountUp, Entrance, Magnetic, Reveal, Stagger, StaggerItem } from "@/com * do, and a landing page is a README with better typography. */ +/** + * The figures the hero publishes. + * + * WHY THESE FOUR, AND WHY TWO OTHERS ARE GONE + * + * The page's own footer promises that every number on it was measured on a real repository and + * that the commands producing them are in the repo. That promise was false in four places. + * `957 tests / 75 files` was stale against 1,844 across 107; `2.1s to index 327 files` was + * taken on a 303-file tree before six of the eleven pipeline stages existed, and the largest + * one today (`taint`, 3.0s of 6.0s) was not among them. + * + * Two claims were REMOVED rather than corrected, because nothing in the repository + * substantiates them and a number nobody can re-derive is the thing the footer promises not to + * publish: + * + * - a claimed detector accuracy percentage on held-out repositories - no corpus, protocol or + * command backing it exists anywhere here. + * - a claimed peak-memory figure of 313.9 MiB - `npm run selfindex` under `/usr/bin/time -l` + * on this machine. The container may well be leaner; until something measures IT, the + * honest move is silence. The "runs on 512 MB" badge is a deployment target, not a claim + * about a measurement, and is left alone. + * + * What remains is asserted by `apps/web/tests/landing-claims.test.ts` against the working tree, + * except the wall clock, which is published with a date the way the README publishes its case + * count - a timing measures the machine as much as the code. `npm run selfindex` reprints all + * of it. + */ const PROOF = [ - { value: 957, suffix: "", label: "tests, all green", note: "75 files, every gate in CI" }, - { value: 87, suffix: "%", label: "detection precision", note: "on held-out repos, never tuned against" }, - { value: 2.1, decimals: 1, suffix: "s", label: "to index 327 files", note: "cold, single container" }, - { value: 313.9, decimals: 1, suffix: " MiB", label: "peak memory", note: "under a hard 512 MiB cap" }, + { value: 2206, suffix: "", label: "tests, all green", note: "120 files, every gate in CI" }, + { value: 379, suffix: "", label: "TypeScript files", note: "498 of 519 scanned, 90,663 LOC analysed" }, + { value: 6.0, decimals: 1, suffix: "s", label: "to index this repo, cold", note: "11 stages, measured 2026-08-09" }, + { value: 11, suffix: "", label: "instrumented stages", note: "each one times itself, every run" }, ]; const LENSES = [ @@ -234,7 +261,7 @@ export default function LandingPage() { index gets slower you get a package name, not a shrug.

- Below: an actual run over this repository's 327 TypeScript files. + Below: an actual run over this repository's 379 TypeScript files.

diff --git a/apps/web/src/app/repos/[id]/agents/page.tsx b/apps/web/src/app/repos/[id]/agents/page.tsx index 20b6672..854e155 100644 --- a/apps/web/src/app/repos/[id]/agents/page.tsx +++ b/apps/web/src/app/repos/[id]/agents/page.tsx @@ -12,7 +12,7 @@ export default function AgentsPage() { title="Agent swarm" blurb="Deterministic specialists argue findings out among themselves, a critic challenges them, a judge ranks what survives. Any finding can then be turned into a fix proved against your own test suite." /> - + ); } diff --git a/apps/web/src/app/repos/[id]/architecture/page.tsx b/apps/web/src/app/repos/[id]/architecture/page.tsx index ac1a9d0..d48d437 100644 --- a/apps/web/src/app/repos/[id]/architecture/page.tsx +++ b/apps/web/src/app/repos/[id]/architecture/page.tsx @@ -21,6 +21,7 @@ export default function ArchitecturePage() { diff --git a/apps/web/src/app/repos/[id]/ask/page.tsx b/apps/web/src/app/repos/[id]/ask/page.tsx new file mode 100644 index 0000000..1e903f3 --- /dev/null +++ b/apps/web/src/app/repos/[id]/ask/page.tsx @@ -0,0 +1,327 @@ +"use client"; + +import { useCallback, useEffect, useState } from "react"; +import { usePathname, useRouter, useSearchParams } from "next/navigation"; +import { ArrowRight, Loader2, Search } from "lucide-react"; +import Link from "next/link"; +import type { AskIntent, AskPlan, AskResult } from "@codegraph/core-graph"; +import { intelAsk } from "@/lib/api"; +import { directionApplies, intentsFor, phraseFor, transitiveApplies, type PlanDraft } from "@/lib/askPhrase"; +import { editorHref } from "@/lib/findings"; +import { plural } from "@/lib/plural"; +import { SectionHead, useRepo } from "../repo-context"; + +/** + * Ask the graph a question, in English, with no model involved. + * + * WHY THE PLAN IS ON SCREEN AND NOT BEHIND A TOGGLE + * + * Every other answer in this product shows its working: the Health Score lists the dimensions + * that moved it, a finding carries the line that triggered it. A natural-language box is the + * one surface where a reader has no way to tell an analysis from a guess, so the compiled + * plan - the intent, the entity it bound, the direction it walked, the operations it ran - is + * rendered next to the answer rather than hidden. It is the same promise the rest of the + * report makes, kept in the place it is easiest to break. + * + * A question the compiler cannot classify is answered as a refusal listing the forms that do + * work. That is deliberate and it is the feature: the alternative is a confident wrong answer + * about who calls what, which a reader cannot detect. + */ + +/** Enough to teach the grammar without turning the page into documentation. */ +const SUGGESTIONS = [ + "what breaks if I change ", + "what depends on ", + "which endpoints are unauthenticated", + "what is dead code", + "what are the circular dependencies", + "what is untested", +]; + +export default function AskPage() { + const repo = useRepo(); + const router = useRouter(); + const pathname = usePathname(); + const params = useSearchParams(); + /* The question lives in the URL so an answer can be shared and re-derived, exactly like + `?symbol=` on the impact page. Same question, same repository, same answer. */ + const asked = params.get("q") ?? ""; + + const [draft, setDraft] = useState(asked); + const [result, setResult] = useState(null); + /* Derived, for the same reason as `syncedTo` below: a question with no answer yet IS the + loading state, so storing it separately only creates a second thing to keep in step. */ + const [answeredFor, setAnsweredFor] = useState(null); + const loading = asked.trim() !== "" && answeredFor !== asked; + const [error, setError] = useState(null); + + /* + * Adjusted DURING RENDER, not in an effect. + * + * Two effects used to do this — one copying `?q=` into the input, one clearing the answer + * when the question emptied — and both are the cascading-render shape React warns about: + * the component paints the previous question's answer, then re-renders to correct itself. + * Comparing against the last question this render tree synced to is the documented + * alternative, and it fixes a visible artefact rather than only a lint error: the stale + * answer no longer flashes under the new question. + */ + const [syncedTo, setSyncedTo] = useState(asked); + if (syncedTo !== asked) { + setSyncedTo(asked); + setDraft(asked); + setResult(null); + setError(null); + } + + useEffect(() => { + if (!asked.trim()) return; + let active = true; + // No `setLoading(true)` here: `answeredFor` still names the previous question, which is + // what `loading` reads. Nothing to set, so nothing cascades. + intelAsk(repo.id, asked) + .then((r) => { if (active) setResult(r); }) + .catch((e: unknown) => { if (active) setError(e instanceof Error ? e.message : "Query failed"); }) + .finally(() => { if (active) setAnsweredFor(asked); }); + return () => { active = false; }; + }, [repo.id, asked]); + + const submit = useCallback( + (q: string) => { + const next = new URLSearchParams(params.toString()); + if (q.trim()) next.set("q", q.trim()); + else next.delete("q"); + router.replace(`${pathname}?${next.toString()}`, { scroll: false }); + }, + [params, pathname, router], + ); + + return ( +
+ + +
{ e.preventDefault(); submit(draft); }} + className="flex items-center gap-sm rounded-lg border border-[var(--line)] bg-[var(--surface-1)] px-md py-sm focus-within:border-[var(--accent-fill)]" + > + + setDraft(e.target.value)} + maxLength={300} + placeholder="what breaks if I change indexRepo" + aria-label="Ask a question about this repository" + className="w-full bg-transparent text-body text-[var(--text-primary)] outline-none placeholder:text-[var(--text-muted)]" + /> + {loading ? : null} + + + + {!asked.trim() && ( +
+ {SUGGESTIONS.map((s) => ( + + ))} +
+ )} + + {error && ( +

{error}

+ )} + + {result && !result.ok && ( +
+

{result.message}

+ {result.didYouMean.length > 0 && ( +
+ Did you mean +
+ {result.didYouMean.map((s) => ( + + ))} +
+
+ )} +
+ Forms this graph can answer +
    + {result.examples.map((x) => ( +
  • + +
    • + ))} +
+
+
+ )} + + {result?.ok && ( +
+
+

{result.headline}

+ {/* The receipt. See the module comment: this is why the surface is trustworthy. */} +
+ {result.plan.intent} + {result.plan.entity && ( + + {result.plan.entity.kind}: {result.plan.entity.label} + + )} + {result.plan.direction !== "none" && ( + {result.plan.direction} + )} + {result.plan.transitive && transitive · depth {result.plan.depth}} + {result.plan.operations.join(" · ")} +
+ + {/* + * The receipt above says what ran. These say what to run INSTEAD. + * + * Measured classifier recall is high but not perfect, and the failure a reader + * cannot recover from is a plan that is nearly right - correct entity, wrong + * direction. Rephrasing blindly is a guessing game; changing the chip is not. + * Every control rewrites the QUESTION through `phraseFor`, so the compiler still + * does the classifying and there is exactly one code path. A synthesised phrase + * that would not round-trip is not offered at all: `phraseFor` returns null and + * the control disappears rather than producing a question the compiler misreads. + */} + +
+ + {result.chains?.length ? ( +
+ Resolved flow + {result.chains.map((c, i) => ( +
+
+ {c.steps.map((s, j) => ( + + {j > 0 && } + {s} + + ))} +
+ {c.sink && reaches {c.sink}} +
+ ))} +
+ ) : null} + + {result.rows.length > 0 && ( +
+ {result.rows.map((r) => ( +
+
+
{r.label}
+
{r.detail}
+
+ {/* Every row is a place in the code, so every row opens there. */} + + {r.file}:{r.line} + +
+ ))} +
+ )} + + {result.truncated && ( +

+ A cap was reached: this answer is a prefix of the truth, not the whole of it. +

+ )} +
+ )} +
+ ); +} + +/** One control chip: a label and whatever sets it. */ +function Control({ label, children }: { label: string; children: React.ReactNode }) { + return ( + + ); +} + +const SELECT = "bg-transparent text-micro font-mono text-[var(--text-primary)] outline-none"; + +/** + * The plan, as controls. + * + * Each change is expressed by rewriting the QUESTION, never by posting a plan: `phraseFor` + * produces a canonical sentence the compiler is guaranteed to classify back to this plan, and + * `ask-phrase.test.ts` asserts that round-trip for every combination offered here. So the + * controls cannot drift away from the grammar - if a phrasing stopped compiling, that test + * fails rather than this panel silently generating questions the compiler misreads. + */ +function PlanControls({ plan, onChange }: { plan: AskPlan; onChange: (q: string) => void }) { + const draft: PlanDraft = { intent: plan.intent, entity: plan.entity, direction: plan.direction, transitive: plan.transitive }; + const emit = (next: Partial) => { + const phrase = phraseFor({ ...draft, ...next }); + if (phrase) onChange(phrase); + }; + + const intents = intentsFor(plan.entity?.kind ?? null); + // Nothing to switch between, and no modifier applies: a panel with one frozen control is + // furniture. Drawn only when it can actually change the answer. + const canSwitchIntent = intents.length > 1; + if (!canSwitchIntent && !directionApplies(draft) && !transitiveApplies(draft)) return null; + + return ( +
+ Not what you meant? + {canSwitchIntent && ( + + + + )} + {directionApplies(draft) && ( + + + + )} + {transitiveApplies(draft) && ( + + + + )} +
+ ); +} diff --git a/apps/web/src/app/repos/[id]/circle-pack/page.tsx b/apps/web/src/app/repos/[id]/circle-pack/page.tsx index 6067c92..44d7e1a 100644 --- a/apps/web/src/app/repos/[id]/circle-pack/page.tsx +++ b/apps/web/src/app/repos/[id]/circle-pack/page.tsx @@ -15,7 +15,7 @@ export default function CirclePackPage() { /> {repo.tree && repo.tree.children && repo.tree.children.length > 0 ? ( - {(onSelect) => } + {(onSelect) => } ) : ( diff --git a/apps/web/src/app/repos/[id]/impact/page.tsx b/apps/web/src/app/repos/[id]/impact/page.tsx new file mode 100644 index 0000000..a46eb08 --- /dev/null +++ b/apps/web/src/app/repos/[id]/impact/page.tsx @@ -0,0 +1,394 @@ +"use client"; + +import { useCallback, useEffect, useMemo, useState } from "react"; +import { usePathname, useRouter, useSearchParams } from "next/navigation"; +import { AlertTriangle, FlaskConical, Loader2, Radius, Search } from "lucide-react"; +import { intelBlast, intelSearch, intelUntestedHubs } from "@/lib/api"; +import type { BlastCaller, BlastReport } from "@/lib/codeintel/query"; +import type { CodeSymbol } from "@/lib/types"; +import { Empty, SectionHead, useRepo } from "../repo-context"; + +const NO_SYMBOLS: CodeSymbol[] = []; + +/** Callers of one file, so the answer reads as "these files break", not "these 40 functions". */ +type FileGroup = { + file: string; + callers: BlastCaller[]; + /** Nearest hop in the group — the sort key, because closest breaks first. */ + nearest: number; + tested: number; +}; + +/** + * Group by file, nearest-first. + * + * A blast radius arrives as a flat list of symbols, and a flat list of forty function + * names is a wall. What a reader is deciding is which FILES they now have to open, and + * how far away each one is, so the file is the row and the hop distance of its closest + * caller is the order. + */ +function groupByFile(callers: readonly BlastCaller[]): FileGroup[] { + const byFile = new Map(); + for (const c of callers) { + const bucket = byFile.get(c.file); + if (bucket) bucket.push(c); + else byFile.set(c.file, [c]); + } + return [...byFile.entries()] + .map(([file, group]) => ({ + file, + callers: [...group].sort((a, b) => a.hops - b.hops || a.line - b.line), + nearest: Math.min(...group.map((c) => c.hops)), + tested: group.filter((c) => c.tested).length, + })) + .sort((a, b) => a.nearest - b.nearest || b.callers.length - a.callers.length || a.file.localeCompare(b.file)); +} + +/** The result of one blast-radius request, tagged with the symbol that asked for it. */ +interface Loaded { + readonly for: string; + readonly report: BlastReport | null; + readonly error: string | null; +} + +export default function ImpactPage() { + const repo = useRepo(); + const router = useRouter(); + const pathname = usePathname(); + const params = useSearchParams(); + const selectedId = params.get("symbol") ?? ""; + + /** + * The selection lives in `?symbol=`, not in state. + * + * A blast radius is the thing you paste into a pull request or a Slack thread — + * "here is what this touches" is worth nothing if the recipient has to re-run the + * search to see it. `replace` rather than `push` for the same reason `useGraphUrl` + * does: picking through a candidate list is not twenty history entries. + */ + const select = useCallback( + (id: string) => { + const next = new URLSearchParams(params.toString()); + if (id) next.set("symbol", id); + else next.delete("symbol"); + const qs = next.toString(); + router.replace(qs ? `${pathname}?${qs}` : pathname, { scroll: false }); + }, + [params, pathname, router] + ); + + const [q, setQ] = useState(""); + /** Symbols together with the exact query they answer. */ + const [found, setFound] = useState<{ readonly for: string; readonly symbols: CodeSymbol[] } | null>(null); + + /** A blast radius together with the symbol id it was computed for. */ + const [loaded, setLoaded] = useState(null); + + /** + * DERIVED, not stored. An empty query has no results and an unselected symbol has no blast + * radius — those are facts about the current inputs, so reading them from the inputs cannot + * go stale. The version that cleared them with `setState` inside the effect rendered the + * PREVIOUS symbol's radius for one frame after a deselect, and cost a cascading render on + * every keystroke that emptied the box (`react-hooks/set-state-in-effect`). + */ + const query = q.trim(); + const results = query && found?.for === query ? found.symbols : NO_SYMBOLS; + // In flight whenever the answer on hand is not the answer to what is typed now. A stored + // boolean said "done" while the previous query's rows were still on screen. + const searching = query !== "" && found?.for !== query; + const current = selectedId && loaded?.for === selectedId ? loaded : null; + const blast = current?.report ?? null; + const blastError = current?.error ?? null; + const blastLoading = selectedId !== null && current === null; + + const [hubs, setHubs] = useState(NO_SYMBOLS); + const [hubsLoading, setHubsLoading] = useState(true); + + // Debounced, mirroring CodeIntelPanel: the same 250ms and the same cancel-on-clear, + // because a search box that behaves differently on two pages is two search boxes. + useEffect(() => { + if (!query) return; + let cancelled = false; + const t = setTimeout(async () => { + // Failure records the query too, so the box settles on "no matches" instead of + // spinning forever against a request that will never arrive. + const symbols = await intelSearch(repo.id, query).catch(() => NO_SYMBOLS); + if (!cancelled) setFound({ for: query, symbols }); + }, 250); + return () => { + cancelled = true; + clearTimeout(t); + }; + }, [query, repo.id]); + + useEffect(() => { + if (!selectedId) return; + let cancelled = false; + // One state write per outcome, and the write CARRIES the symbol it describes. Loading is + // then "the record on hand is not for this symbol", which cannot get out of step with the + // selection the way a separate boolean did. + void intelBlast(repo.id, selectedId) + .then((r): Loaded => ({ + for: selectedId, + report: r, + // A link can outlive a re-index, and an empty panel would look like "nothing calls + // this" rather than "this symbol is gone". + error: r.symbol ? null : `No symbol ${selectedId} in the current index.`, + })) + .catch( + (e: unknown): Loaded => ({ + for: selectedId, + report: null, + error: e instanceof Error ? e.message : "Could not compute the blast radius.", + }), + ) + .then((next) => { + if (!cancelled) setLoaded(next); + }); + return () => { + cancelled = true; + }; + }, [repo.id, selectedId]); + + useEffect(() => { + let cancelled = false; + intelUntestedHubs(repo.id) + .then((r) => { + if (!cancelled) setHubs(r); + }) + .catch(() => { + if (!cancelled) setHubs(NO_SYMBOLS); + }) + .finally(() => { + if (!cancelled) setHubsLoading(false); + }); + return () => { + cancelled = true; + }; + }, [repo.id]); + + const groups = useMemo(() => groupByFile(blast?.callers ?? []), [blast]); + + if (!repo.symbolGraph || repo.symbolGraph.symbols.length === 0) { + return ( + <> + + + + ); + } + + return ( + <> + + +
+
+ {/* min-w-0: an unbroken path in the results would otherwise widen the track. */} +
+
+ + setQ(e.target.value)} + placeholder="Search a symbol or a file to change…" + className="w-full rounded-md border border-[var(--line)] bg-[var(--surface-2)] py-sm pl-xl pr-md text-meta text-[var(--text-primary)] placeholder-[var(--text-muted)] focus:border-[var(--violet-500)]/50 focus:outline-none" + /> + {searching && ( + + )} +
+
+ {results.map((s) => ( + + ))} + {!searching && query && results.length === 0 && ( +

No matches.

+ )} + {!q.trim() && ( +

+ Name the thing you are about to change. +

+ )} +
+
+ +
+ {!selectedId && ( +

+ Pick a symbol. Its transitive callers appear here, grouped by file. +

+ )} + {selectedId && blastLoading && !blast && ( +

+ Walking the call graph… +

+ )} + {blastError && ( +

+ + {blastError} +

+ )} + {blast?.symbol && ( + <> +
+ +

{blast.symbol.name}

+ {blast.symbol.kind} +
+

+ {blast.symbol.file}:{blast.symbol.line} +

+ +
+ + {blast.callers.length} callers + + + {groups.length} files + + 0 ? "var(--accent-text)" : "var(--coral-text)" }} + > + {blast.testedCount} in test files + +
+ + {/* The hop histogram: one hop is "callers", three hops is "the part you + were not going to check". Counting them is the cheap version of that. */} +
+ {blast.perHop.map((n, i) => ( + + hop {i + 1}: {n} + + ))} +
+ + {blast.testedCount === 0 && blast.callers.length > 0 && ( +

+ + Nothing in this radius is a test file — every one of these callers changes + unobserved. +

+ )} + + {blast.callers.length === 0 && ( +

+ No resolved callers within {blast.depth} hops. Either it is an entry point, or + the call is made in a way the extractor cannot resolve. +

+ )} + +
+ {groups.map((g) => ( +
+
+ + {g.file} + + + {g.callers.length} · hop {g.nearest} + + {g.tested > 0 ? ( + + ) : null} +
+
    + {g.callers.map((c) => ( +
  • + + {c.hops} + + + {c.name} + :{c.line} + + + {c.tested ? "covered" : "not covered"} + +
    • + ))} +
+
+ ))} +
+ + )} +
+
+
+ +
+

Untested hubs

+

+ Symbols with the most callers that no test file reaches, most-depended-upon first. + This is the same set the agent swarm raises as “untested core logic” — one + definition, so the two cannot disagree. Pick one to see what it would take down. +

+
+ {hubsLoading && ( +

+ Ranking hubs… +

+ )} + {!hubsLoading && hubs.length === 0 && ( +

+ Every hub in this graph has a test caller. That is the good outcome. +

+ )} +
    + {hubs.map((h) => ( +
  • + +
    • + ))} +
+
+
+ + ); +} diff --git a/apps/web/src/app/repos/[id]/layout.tsx b/apps/web/src/app/repos/[id]/layout.tsx index 7416d2e..afd2dd5 100644 --- a/apps/web/src/app/repos/[id]/layout.tsx +++ b/apps/web/src/app/repos/[id]/layout.tsx @@ -409,8 +409,11 @@ export default function RepoLayout({
+ {/* Points at `ask`, not `network`. It used to open the force-directed graph, + which is already one click away in the sidebar and answers no question: + the label promised a query surface and delivered a picture. */} Query the graph diff --git a/apps/web/src/app/repos/[id]/network/page.tsx b/apps/web/src/app/repos/[id]/network/page.tsx index 1fca3b8..9c0b281 100644 --- a/apps/web/src/app/repos/[id]/network/page.tsx +++ b/apps/web/src/app/repos/[id]/network/page.tsx @@ -10,7 +10,7 @@ export default function NetworkPage() { <> {repo.viz && repo.viz.nodes.length > 0 ? ( - {(onSelect) => } + {(onSelect) => } ) : ( diff --git a/apps/web/src/app/repos/[id]/ownership/page.tsx b/apps/web/src/app/repos/[id]/ownership/page.tsx new file mode 100644 index 0000000..eccb26b --- /dev/null +++ b/apps/web/src/app/repos/[id]/ownership/page.tsx @@ -0,0 +1,256 @@ +"use client"; + +import { useEffect, useMemo, useState } from "react"; +import { Loader2, UserRound, Users } from "lucide-react"; +import { ownershipReviewers, ownershipSummary, type OwnershipSummary, type ReviewerSuggestion } from "@/lib/api"; +import { Empty, SectionHead, useRepo } from "../repo-context"; + +/** + * Ownership: who knows this code, and where nobody does any more. + * + * EVERY NUMBER HERE IS A SHARE OF COMMITS, not of lines, and the page says so rather than + * leaving a percentage to be read as something stronger. Git attributes a commit to a file; + * per-line authorship for a whole tree means a blame per file, which the index does not run. + * "Owns 62%" therefore means "made 62% of the commits that touched this", which is the standard + * proxy and is not the same claim. + * + * The report is computed at index time, so a repository indexed before it existed has none — + * the route answers 409 and this renders that as "re-index", never as "no owners". + */ + +const DAY_MS = 86_400_000; + +/** Files listed under stale areas. The report already caps at 100; this is what fits a page. */ +const SHOWN_STALE = 25; + +function relativeDay(epochSeconds: number): string { + const days = Math.round((Date.now() - epochSeconds * 1000) / DAY_MS); + if (days <= 0) return "today"; + if (days === 1) return "yesterday"; + if (days < 60) return `${days} days ago`; + return `${Math.round(days / 30)} months ago`; +} + +function Bar({ share }: { share: number }) { + const pct = Math.round(share * 100); + return ( + + + + + {/* The number is the accessible carrier; the bar is decoration. */} + {pct}% + + ); +} + +export default function OwnershipPage() { + const repo = useRepo(); + const [data, setData] = useState(null); + const [error, setError] = useState(null); + /* + * DERIVED, not set inside the effect. `setLoading(true)` in an effect body is a cascading + * render, and it also encoded the state twice: "loading" is exactly "the data I hold is not + * for the repo I am rendering", which this compares directly and cannot get out of step. + */ + const [loadedFor, setLoadedFor] = useState(null); + const loading = loadedFor !== repo.id; + + const [files, setFiles] = useState(""); + const [reviewers, setReviewers] = useState(null); + const [reviewerError, setReviewerError] = useState(null); + const [reviewersLoading, setReviewersLoading] = useState(false); + + useEffect(() => { + let cancelled = false; + ownershipSummary(repo.id) + .then((d) => { + if (!cancelled) { + setData(d); + setError(null); + } + }) + .catch((e: unknown) => { + if (!cancelled) setError(e instanceof Error ? e.message : "Failed to load ownership"); + }) + .finally(() => { + // Marks the data as belonging to THIS repo, which is what ends the loading state. + if (!cancelled) setLoadedFor(repo.id); + }); + return () => { + cancelled = true; + }; + }, [repo.id]); + + const parsedFiles = useMemo( + () => files.split(/[\n,]/).map((f) => f.trim()).filter(Boolean), + [files], + ); + + async function askReviewers(): Promise { + if (parsedFiles.length === 0) return; + setReviewersLoading(true); + setReviewerError(null); + try { + const res = await ownershipReviewers(repo.id, parsedFiles); + setReviewers(res.reviewers); + } catch (e: unknown) { + setReviewers(null); + setReviewerError(e instanceof Error ? e.message : "Failed to load reviewers"); + } finally { + setReviewersLoading(false); + } + } + + return ( +
+ + + {loading ? ( +

+ Reading history… +

+ ) : error ? ( + // The route's own message. A 409 says "re-index"; anything else is a real failure. Both + // are shown verbatim rather than flattened into an empty state that would read as + // "this repository has no owners". + + ) : data === null ? ( + + ) : ( + <> +

+ {data.commitsAnalysed.toLocaleString()} commits + over the last {data.windowDays} days, from{" "} + {data.authors.length} author + {data.authors.length === 1 ? "" : "s"}. Shares are of commits touching a file, not + of lines. + {data.truncated ? " A bound was hit, so this is a partial view." : ""} +

+ +
+

+ Contributors +

+ {data.authors.length === 0 ? ( + + ) : ( +
    + {data.authors.map((a) => ( +
  • + {a.name} + + {a.commits.toLocaleString()} commit{a.commits === 1 ? "" : "s"} + + + {a.filesTouched.toLocaleString()} file{a.filesTouched === 1 ? "" : "s"} + + + last commit {relativeDay(a.lastAt)} + +
    • + ))} +
+ )} +
+ +
+

Orphaned areas

+

+ Files whose every owner has stopped committing in the recent window. Old is not the + same as abandoned — a file nobody has needed to touch is merely stale, and is not + listed here. +

+ {data.stale.length === 0 ? ( + + ) : ( +
    + {data.stale.slice(0, SHOWN_STALE).map((f) => ( +
  • + {f.path} + + {f.staleDays === null ? "age unknown" : `${f.staleDays} days untouched`} + + + bus factor {f.busFactor} + + + {f.owners.slice(0, 2).map((o) => ( + + {o.author} + + + ))} + +
    • + ))} +
+ )} +
+ +
+

+ Who should review this? +

+

+ Ranked over real history: ownership of the files you name, how recently each person + touched that area, and what else changes alongside it. Anyone inactive in the window + is excluded — routing a review to someone who left is worse than routing it nowhere. +

+