diff --git a/AGENTS.md b/AGENTS.md index d11ff9d..43ddade 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -434,11 +434,9 @@ one-time setup. A file-map or table row gets a clause, not a paragraph. Don't funnel many routes into one dispatcher that re-parses `ctx.url.pathname`: it duplicates the URL shape, ignores the router's params, and has to re-handle HEAD. Factor shared per-request setup into a small `withX` wrapper — see `examples/plugins/admin/`. -- **`handleRequest` (`src/http/app.ts`) is a known complexity hotspot** — ~160 lines tracking - canonical host, static, locale, session + re-mint, CSRF, chrome, hooks, plugin routing, builtin - routing, 405/404 and error mapping. The pure parts are already extracted and separately tested; - what remains is orchestration. Planned split along those seams; don't grow it further without - taking one out. +- **A function stays within 50 lines and complexity 20** (ESLint's defaults, measured by + `src/function-limits.test.ts`). Split one that outgrows them; never add it to the test's + `grandfathered` list or raise a ceiling there. - Reviews are maintainer-triggered (e.g. via the larv-review skill) — never auto-run reviewer agents. - **A user-visible string belongs in a catalog, not in the code or a view.** Core strings go in `src/i18n/locales/en-US.ts` (then every other locale, or the boot fails); a plugin's go in its own diff --git a/README.md b/README.md index 8e6a230..0f1871b 100644 --- a/README.md +++ b/README.md @@ -1813,7 +1813,7 @@ unnecessary here — the signature itself is already invalid. ``` src/ The app — strict tsc, no build step. *.test.ts sit beside their module. server.ts Entry point; config.ts env loader; logger.ts structured log + trace span + tracedFetch - *.test.ts Topology guards with no source counterpart (compose/kratos/keto/hydra/postgres/ci-gate) + *.test.ts Guards with no source counterpart (compose/kratos/keto/hydra/postgres/ci-gate/function-limits) http/ Request pipeline: app.ts (createApp), builtin-routes, context, body, cookie, static, safe-url (safeUrl + localPath), security-headers auth/ Identity + the session-JWT hot path: jwt · jwt-middleware · jwks · gen-jwks (CLI) · diff --git a/docs/reading-cost.md b/docs/reading-cost.md index 47125bd..b454c82 100644 --- a/docs/reading-cost.md +++ b/docs/reading-cost.md @@ -89,3 +89,19 @@ Read most (opens and searches): - `e2e-tests/mock-oidc.ts` 2 - `src/ci-gate.test.ts` 2 - `.dockerignore` 1 + +## Reading cost, chunk 25b7840ff611, finished 2026-10-31T07:46:18Z + +73781 new input tokens before the first commit. + +Read most (opens and searches): + +- `todo.md` 5 +- `README.md` 2 +- `src/http/app.ts` 2 +- `AGENTS.md` 1 +- `src/http/app.test.ts` 1 +- `src/ui/chrome.test.ts` 1 +- `src/ui/chrome.ts` 1 +- `views/index.ejs` 1 +- `views/partials/shell.ejs` 1 diff --git a/package-lock.json b/package-lock.json index a0671e0..4ecde74 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,6 +14,7 @@ "devDependencies": { "@types/ejs": "3.1.5", "@types/node": "24.19.2", + "acorn": "8.19.0", "typescript": "7.0.2" }, "engines": { @@ -386,6 +387,19 @@ "node": ">=16.20.0" } }, + "node_modules/acorn": { + "version": "8.19.0", + "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.19.0.tgz", + "integrity": "sha512-oJlA3XiRm7Cyk6qFD2Jn8ak9B7jSy0qy00ADO3+8dpT0LSjFihQYv4C02LFCSYJV3Q37xYwwRy5m+IpIiUzqWw==", + "dev": true, + "license": "MIT", + "bin": { + "acorn": "bin/acorn" + }, + "engines": { + "node": ">=0.4.0" + } + }, "node_modules/ejs": { "version": "6.0.1", "resolved": "https://registry.npmjs.org/ejs/-/ejs-6.0.1.tgz", diff --git a/package.json b/package.json index b2bee3e..7771026 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ "devDependencies": { "@types/ejs": "3.1.5", "@types/node": "24.19.2", + "acorn": "8.19.0", "typescript": "7.0.2" } } diff --git a/src/function-limits.test.ts b/src/function-limits.test.ts new file mode 100644 index 0000000..c7d067d --- /dev/null +++ b/src/function-limits.test.ts @@ -0,0 +1,159 @@ +// The limits and the counting are ESLint's defaults: `max-lines-per-function` (blank lines and +// comments count, an IIFE is not held to it) and `complexity`. Not ESLint itself: parsing TypeScript +// takes typescript-eslint, which needs the compiler API TypeScript 7 no longer ships. So acorn parses +// what Node's own type stripping leaves, which is the code Node runs. +import assert from "node:assert/strict"; +import { globSync, readFileSync } from "node:fs"; +import { stripTypeScriptTypes } from "node:module"; +import { test } from "node:test"; +import { parse } from "acorn"; + +const LIMITS = { lines: 50, complexity: 20 }; + +// Each function over a limit, at its measurement. A ceiling follows its function down, and a metric +// leaves the entry once it fits, so a function that shrank can't grow back. +const grandfathered: Record = { + "src/http/app.ts createApp": { lines: 318, complexity: 28 }, + "src/http/app.ts createApp > handleRequest": { lines: 168, complexity: 43 }, + "src/auth/routes.ts flowPage": { lines: 64 }, + "src/auth/routes.ts flowPage > anonymous": { lines: 62, complexity: 23 }, + "src/auth/flow-view.ts buildFlowView": { complexity: 24 }, + "src/auth/hydra-admin.ts createHydraAdmin": { lines: 87 }, + "src/auth/kratos-admin.ts createKratosAdmin": { lines: 68 }, + "src/auth/kratos-public.ts createKratosPublic": { lines: 60 }, + "src/plugin-host/discovery.ts discoverPlugins": { lines: 74, complexity: 22 }, + "src/plugin-host/discovery.ts shapeError": { complexity: 23 }, + "src/config.ts loadConfig": { lines: 52 }, + "release-tooling/dockerhub-overview.ts main": { lines: 57 }, + "examples/plugins/admin/admin-users.ts buildUserFormModel": { complexity: 21 }, +}; + +interface AstNode { + type: string; + loc: { start: { line: number }; end: { line: number } }; + [key: string]: unknown; +} + +interface Measured { + name: string; + lines: number; + complexity: number; +} + +const isNode = (value: unknown): value is AstNode => + typeof value === "object" && value !== null && typeof (value as { type?: unknown }).type === "string"; + +const FUNCTIONS = new Set(["FunctionDeclaration", "FunctionExpression", "ArrowFunctionExpression"]); +const BRANCHES = new Set([ + "IfStatement", "ConditionalExpression", "LogicalExpression", "ForStatement", "ForInStatement", + "ForOfStatement", "WhileStatement", "DoWhileStatement", "CatchClause", "AssignmentPattern", +]); + +function branches(node: AstNode): number { + if (BRANCHES.has(node.type)) return 1; + if (node.type === "SwitchCase") return node.test ? 1 : 0; + if (node.type === "AssignmentExpression") return ["&&=", "||=", "??="].includes(String(node.operator)) ? 1 : 0; + if (node.type === "MemberExpression" || node.type === "CallExpression") return node.optional === true ? 1 : 0; + return 0; +} + +function nameOf(fn: AstNode, parent: AstNode | undefined): string { + let id = fn.id; + if (!id && parent?.type === "VariableDeclarator") id = parent.id; + if (!id && (parent?.type === "Property" || parent?.type === "MethodDefinition")) id = parent.key; + return isNode(id) && typeof id.name === "string" ? id.name : "anonymous"; +} + +// A function is named by the path of names from the outermost one, e.g. `createApp > handleRequest`, +// so an entry above doesn't depend on line numbers. +function measure(source: string): Measured[] { + const found: Measured[] = []; + const visit = (node: AstNode, parent: AstNode | undefined, enclosing: Measured | undefined): void => { + let current = enclosing; + if (FUNCTIONS.has(node.type)) { + const name = nameOf(node, parent); + const iife = parent?.type === "CallExpression" && parent.callee === node; + const lines = iife ? 0 : node.loc.end.line - node.loc.start.line + 1; + current = { name: enclosing ? `${enclosing.name} > ${name}` : name, lines, complexity: 1 }; + found.push(current); + } else if (current) { + current.complexity += branches(node); + } + for (const value of Object.values(node)) { + for (const child of [value].flat()) if (isNode(child)) visit(child, node, current); + } + }; + const program: unknown = parse(stripTypeScriptTypes(source), { ecmaVersion: "latest", sourceType: "module", locations: true }); + assert.ok(isNode(program)); + visit(program, undefined, undefined); + return found; +} + +const root = new URL("..", import.meta.url); +// The drop-in mounts hold an operator's code, not this repo's. Tests are left out: each is a flat +// script, and a `describe` block wraps many of them. +const files = globSync("**/*.ts", { + cwd: root, + exclude: ["**/node_modules/**", "plugins/**", "config/**", "locales/**", "**/*.d.ts", "**/*.test.ts", "**/*.spec.ts"], +}); +// A list, not a map: two unnamed siblings share a key, and each must be checked. +const measured = files.flatMap((file) => + measure(readFileSync(new URL(file, root), "utf8")).map((m) => ({ ...m, key: `${file} ${m.name}` })), +); +const METRICS = ["lines", "complexity"] as const; + +test("measure counts lines and complexity as ESLint does", () => { + const source = [ + "function outer(a = 1): void {", + " if (a && b?.c) {}", + " for (const x of y) {}", + " for (const k in y) {}", + " for (;;) break;", + " while (a) a--;", + " do {} while (a);", + " try {} catch {}", + " switch (a) { case 1: break; default: }", + " a ||= 2; a &&= 3; a ??= 4;", + " b?.();", + " y.map(() => {});", + " const inner = () => (a ? 1 : 2);", + " const o = { prop() {}, key: function () {} };", + " (function () { if (a) {} })();", + "}", + "class K { method() {} }", + ].join("\n"); + assert.deepEqual(measure(source), [ + { name: "outer", lines: 16, complexity: 16 }, + { name: "outer > anonymous", lines: 1, complexity: 1 }, + { name: "outer > inner", lines: 1, complexity: 2 }, + { name: "outer > prop", lines: 1, complexity: 1 }, + { name: "outer > key", lines: 1, complexity: 1 }, + { name: "outer > anonymous", lines: 0, complexity: 2 }, + { name: "method", lines: 1, complexity: 1 }, + ]); +}); + +test(`every function fits ${LIMITS.lines} lines and complexity ${LIMITS.complexity}, or its grandfathered ceiling`, () => { + assert.ok(files.length > 50, "found too few files: is the glob broken?"); + const over = measured.flatMap((m) => + METRICS.filter((metric) => m[metric] > (grandfathered[m.key]?.[metric] ?? LIMITS[metric])).map( + (metric) => `${m.key}: ${metric} ${m[metric]}`, + ), + ); + assert.deepEqual(over, [], "split the function; never raise a ceiling"); +}); + +test("every grandfathered ceiling equals its function's measurement, which is over the limit", () => { + const stale = Object.entries(grandfathered).flatMap(([key, ceiling]) => { + const found = measured.filter((m) => m.key === key); + if (found.length === 0) return [`${key}: no such function; rename the entry if it moved, else delete it`]; + return METRICS.flatMap((metric) => { + const allowed = ceiling[metric]; + if (allowed === undefined) return []; + const now = Math.max(...found.map((m) => m[metric])); + if (now <= LIMITS[metric]) return [`${key}: ${metric} ${now} fits; delete it`]; + return now < allowed ? [`${key}: ${metric} ${now}; lower the ceiling to it`] : []; + }); + }); + assert.deepEqual(stale, []); +}); diff --git a/todo.md b/todo.md index 6c4759e..133e6d1 100644 --- a/todo.md +++ b/todo.md @@ -13,7 +13,6 @@ | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | |---|---|---|---|---|---|---|---|---|---|---|---| | 21 | 0.4.21 | | | **Fail the gate on a function that exceeds a size or complexity limit.** | 3 | 1 | 1 | 3 | 5 | code health by 4 | 7.2 | -| 32 | 0.4.21 | | | **Fail the gate on a stylesheet feature the browser floor lacks, unless it is guarded.** | 3 | 1 | 1 | 3 | 4 | code health by 2 | 6.0 | | 20 | 0.4.21 | | | **Redirect an anonymous hit on a session-needing flow page to sign-in, instead of a 500.** | 2 | 2 | 2 | 4 | 3 | 2 by 4 | 4.2 | | 4 | 0.4.21 | | | **Refuse a permission save whose baseline no longer matches Keto, so concurrent edits are not silently lost.** | 2 | 2 | 2 | 4 | 4 | 2 by 4 | 2.8 | | 7 | 0.4.21 | | | **Confirm a successful permission save, and report a partial failure instead of hiding it.** | 2 | 2 | 2 | 4 | 4 | 2 by 4 | 2.8 | @@ -21,6 +20,7 @@ | 34 | 0.4.21 | | | **Serve every stylesheet and script under a content-versioned URL with a year-long immutable `Cache-Control`, so a page view downloads none of them again.** | 4 | 2 | 3 | 7 | 6 | 8 by 6, 2 by 2 | 1.8 | | 3 | 0.4.21 | | | **Guard the group paths against self-lockout, or accept the gap explicitly.** | 3 | 2 | 2 | 5 | 6 | 2 by 6 | 1.4 | | 1 | 0.4.21 | | owner | **Publish every release to Docker Hub, and give the Hub page its short description.** | 3 | 3 | 2 | 6 | 5 | 1 by 3 | 1.3 | +| 32 | 0.4.21 | | | **Fail the gate on a stylesheet feature the browser floor lacks, unless it is guarded.** | 4 | 2 | 2 | 6 | 7 | code health by 2 | 1.1 | | 13 | 0.4.21 | | | **Accept a verification code pasted with surrounding whitespace.** | 4 | 2 | 3 | 7 | 7 | 2 by 8 | 1.0 | | 19 | 0.5.0 | | question | **Run the documented Docker commands on macOS and fix whatever misbehaves.** | 2 | 2 | 1 | 3 | 3 | 1 by 3 | 3.3 | | 16 | 0.5.0 | | question | **Decide whether the single Keto `Resource` namespace splits into per-domain namespaces.** | 3 | 2 | 3 | 6 | 6 | 7 by 2 | 0.5 | @@ -37,12 +37,6 @@ Only `tsc --strict` runs today. A new tool is a new dependency, so justify it. -### 32. Fail the gate on a stylesheet feature the browser floor lacks, unless it is guarded. - -Nothing checks README → "Two browser floors", and no E2E engine is old enough to show a break. -Lint every stylesheet, the examples' included, against Chromium 87, Safari 15 and Firefox 115, and -allow a feature only inside an `@supports` that guards it. - ### 20. Redirect an anonymous hit on a session-needing flow page to sign-in, instead of a 500. `flowPage` (`src/auth/routes.ts`) rethrows a 401, which Kratos answers to a settings-flow init with no @@ -114,6 +108,12 @@ description opens a README sentence and every command the overview shows appears fejkdata also syncs the page on any push to `main`, with its own token in an environment only `main` may use; decide whether that fits here, where `{{VERSION}}` must render a released tag. +### 32. Fail the gate on a stylesheet feature the browser floor lacks, unless it is guarded. + +Nothing checks README → "Two browser floors", and no E2E engine is old enough to show a break. +Lint every stylesheet, the examples' included, against Chromium 87, Safari 15 and Firefox 115, and +allow a feature only inside an `@supports` that guards it. + ### 13. Accept a verification code pasted with surrounding whitespace. A pasted code fails the browser's `pattern` check. No zero-JS fix exists: `pattern`