diff --git a/docs/comprehension-history.md b/docs/comprehension-history.md index 93a263a..9e4e47f 100644 --- a/docs/comprehension-history.md +++ b/docs/comprehension-history.md @@ -106,3 +106,119 @@ Inherited architect, decided by: - Self-sufficiency: `e2e-tests/proxy.ts:14` `toKratos`: the predicate's contract is readable on its own line - Self-sufficiency: `e2e-tests/Dockerfile:11` `COPY tsconfig.json`: the relative `extends` path works in the checkout and in the image, and a comment names the link - Shape, Overall: `src/ui/icons.ts:4` `ICON_NAMES`: the registry matches its description, with no dead ids + +## 2026-10-31T12:28:23Z, PR #200 at 3592a30, against a8a9744 + +Ruling: same + +| Seat | Navigation | Locality | Shape | Self-sufficiency | Overall | +|---|---|---|---|---|---| +| Mid A | better | better | better | same | better | +| Inherited architect | worse | worse | better | same | same | +| Maintainability senior | same | same | better | same | same | +| Junior A | better | better | better | better | better | + +Mid A, decided by: + +- Navigation, Locality, Shape: `src/ui/chrome.ts:74` `shellUser`: the deleted `buildShellContext` and `buildDashboardModel` left two shell models side by side, and now every page reads one `chrome` +- Locality, Shape: `src/auth/routes.ts:58` `flowFailure`: a named unit shared by `flowPage` and `codeFlowSubmit`, where the old `flowPage` held the whole error ladder inline +- Shape: `src/auth/kratos-public.ts:117` `submitFlow`: takes a flow type and id instead of an action URL, and the Referer quirk is explained where it applies +- Locality: `src/http/static.ts:93` `versioned`: reads the module-level `hashes` map, so an asset href depends on what an earlier request hashed +- Navigation, Shape: `src/ui/stylesheet-floor.ts:30` `floorViolations`: a 249-line regex walker that only tests use sits in `src/ui/` beside runtime modules, though the README now points straight at it + +Inherited architect, decided by: + +- Navigation: `public/css/styles.css:25` `:root`: the old comment next to the palette answered the 3am question and the new text drops it +- Navigation: `src/ui/stylesheet-floor.ts:1` `floorViolations`: its header never says an unsupported value in a custom property blanks every `var()` reading it, so a cold reader walks the regex walker to find the cause +- Navigation, Shape: `src/ui/stylesheet-floor.ts:12` `import`: test-only tooling that pulls a devDependency sits beside runtime modules and is missing from the README file map +- Locality: `src/http/static.ts:64` `hashes`: a module-level map shared by every `publicAssets()` instance makes a fresh href depend on request history elsewhere +- Locality: `src/http/app.ts:188` `renderPage`: `assetHref` is appended last at three render sites, and only a comment keeps a fourth from emitting unversioned hrefs +- Shape: `src/ui/chrome.ts:74` `shellUser`: removes the second shell model that duplicated `PageChrome` +- Shape: `src/auth/routes.ts:58` `flowFailure`: one named unit shared by `flowPage` and `codeFlowSubmit`, each handler reads in a screen + +Maintainability senior, decided by: + +- Shape, Locality: `src/ui/chrome.ts:74` `shellUser`: the deleted `ShellModel` left two shapes for the same chrome, and now the shell has one input and one builder +- Shape, Locality: `src/auth/routes.ts:58` `flowFailure`: the old `flowPage` mixed about 60 lines of error mapping with `return_to` handling, and now `flowReturnTo` and `flowFailure` read on their own +- Locality: `src/http/static.ts:64` `hashes`: a hidden module-level cache filled by two readers, and `assetHref` is merged by hand at three render sites +- Shape, Navigation: `src/ui/stylesheet-floor.ts:119` `positive`: a 249-line regex walker that only a test uses sits among runtime view modules + +Junior A, decided by: + +- Navigation, Self-sufficiency: `src/ui/stylesheet-floor.ts:1` `floorViolations`: the README's "Two browser floors" points here and the header lists what is checked, where the base had a prose rule and a hand-written regex test +- Shape, Locality: `src/ui/chrome.ts:74` `shellUser`: one shell view model replaces `ShellModel` beside `PageChrome`, and its helpers sit beside their only caller +- Shape, Locality: `src/auth/routes.ts:58` `flowFailure`: named units replace about 60 inline lines in `flowPage`, and `codeFlowSubmit` reuses them +- Self-sufficiency: `src/auth/kratos-admin.ts:68` `createRecoveryCode`: the comment no longer claims Kratos mails the link +- Locality: `src/http/static.ts:64` `hashes`: a module-level mutable map backs asset versioning, hidden state the base didn't have + +## 2026-10-31T12:30:19Z, PR #200 at 3592a30 + +| Seat | Navigation | Locality | Shape | Self-sufficiency | Overall | +|---|---|---|---|---|---| +| Junior A | 7 | 6 | 6.5 | 6.5 | 6.5 | +| Mid A | 7 | 6 | 6.5 | 6.5 | 6.5 | +| Maintainability senior | 7 | 6 | 6.5 | 6.5 | 6.5 | +| Inherited architect | 7 | 5.5 | 5.5 | 7.5 | 6.5 | +| Mean | 7.00 | 5.88 | 6.25 | 6.75 | 6.50 | + +Junior A, hardest first: + +- Locality, Overall: `src/ui/stylesheet-floor.ts:119` `positive`: a regex-built `@supports` evaluator whose header says what it enforces but not how it decides a guard names a feature +- Locality, Overall: `src/http/app.ts:185` `handleRequest`: about 165 lines in one closure where each stage reads state an earlier stage set +- Self-sufficiency: `src/auth/routes.ts:58` `flowFailure`: its 4xx branches can't fire from `codeFlowSubmit` because `submitFlow` throws only on 5xx, and the call site doesn't say so + +Mid A, hardest first: + +- Locality, Overall: `src/ui/stylesheet-floor.ts:119` `positive`: a hand-written CSS walker with three regex passes per declaration, opaque at line level +- Locality, Overall: `src/http/app.ts:185` `handleRequest`: a fixed order of about 15 stages that nothing in the code enforces +- Self-sufficiency, Shape: `src/auth/login.ts:40` `readPermissions`: the comment names the tuple `#members` where the code uses `granted` +- Locality: `src/auth/routes.ts:121` `codeFlowSubmit`: one idea split across `flow-view.ts`, `routes.ts` and `kratos-public.ts` + +Maintainability senior, hardest first: + +- Locality, Overall: `src/http/app.ts:185` `handleRequest`: a 167-line closure of complexity 43 whose order across closure state nothing names as a contract +- Self-sufficiency, Locality: `src/auth/routes.ts:58` `flowFailure`: on the submit path every 4xx branch is dead and nothing at the call site says which can fire +- Locality: `src/ui/stylesheet-floor.ts:119` `positive`: the one module whose correctness can't be checked by reading +- Self-sufficiency: `src/auth/bootstrap.ts:31` `permissionTuple`: the comment says `#members` while the code writes `granted` + +Inherited architect, would restructure first: + +- Shape, Navigation: `src/auth/flow-view.ts:152` `formAction`: the flow-to-route mapping has two homes, here and `routes.ts`, so a code flow's route and its form target are decided in different files +- Shape, Navigation: `src/ui/stylesheet-floor.ts:30` `floorViolations`: a CSS linter that never runs at runtime sits among the runtime view helpers +- Locality, Shape: `src/http/app.ts:185` `handleRequest`: a level of its own inside `createApp` +- Shape: `src/http/builtin-routes.ts:23` `matchBuiltinRoute`: holds no routes, because the table is inline in `app.ts:176` and `buildAuthRoutes` + +## 2026-10-31T12:30:27Z, PR #200 rerun at 3592a30 + +| Seat | Navigation | Locality | Shape | Self-sufficiency | Overall | +|---|---|---|---|---|---| +| Junior A | 7 | 5.5 | 6 | 6.5 | 6 | +| Mid A | 7 | 6 | 6.5 | 6.5 | 6.5 | +| Maintainability senior | 7 | 5.5 | 6 | 6.5 | 6 | +| Inherited architect | 7 | 6 | 6 | 6.5 | 6.5 | +| Mean | 7.00 | 5.75 | 6.13 | 6.50 | 6.25 | + +Junior A, hardest first: + +- Locality, Overall: `src/ui/stylesheet-floor.ts:119` `positive`: a regex-built CSS walker that stayed opaque without tracing it by hand +- Locality, Overall: `src/http/app.ts:185` `handleRequest`: about 320 lines of interdependent locals, with a fixed stage order that only a top-to-bottom read shows +- Self-sufficiency, Shape: `src/auth/login.ts:40` `completeLogin`: the header says `#members` where the code uses `granted` + +Mid A, hardest first: + +- Locality, Overall: `src/http/app.ts:185` `handleRequest`: about 165 lines of order-dependent stages, with `renderPage` reassigned partway +- Locality, Overall: `src/ui/stylesheet-floor.ts:119` `positive`: how it decides a guard names a feature stayed opaque +- Self-sufficiency, Shape: `src/auth/bootstrap.ts:31` `seedPermissions`: the comment says `#members` where the code uses `granted` + +Maintainability senior, hardest first: + +- Locality, Overall: `src/http/app.ts:185` `handleRequest`: about 167 lines and complexity 43 in a fixed order nobody states +- Locality, Overall: `src/ui/stylesheet-floor.ts:119` `positive`: a hand-rolled regex CSS walker, partly opaque +- Locality, Shape: `src/plugin-host/plugin.ts:126` `RESERVED_PLUGIN_IDS`: a hand-kept mirror of the host's path segments that has already drifted, as it omits `error` +- Self-sufficiency, Shape: `src/auth/login.ts:40` `readPermissions`: the comment says `#members` where the code uses `granted` + +Inherited architect, would restructure first: + +- Locality, Overall: `src/http/app.ts:185` `handleRequest`: the most concerns and the highest complexity, already grandfathered, and every request-wide concern lands in it +- Shape, Navigation: `src/ui/stylesheet-floor.ts:15` `FLOOR`: the 3am rule's home is a test-only module under `ui/` +- Shape: `src/auth/routes.ts:275` `buildAuthRoutes`: the 19-module `src/auth/` mixes six concerns, and this file with `flow-view.ts` is where the next flow lands diff --git a/todo.md b/todo.md index 5d88bde..eabe39d 100644 --- a/todo.md +++ b/todo.md @@ -6,18 +6,27 @@ `Bar = 1` -`Next ID = 46` +`Next ID = 55` ## Items | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | |---|---|---|---|---|---|---|---|---|---|---|---| +| 46 | 0.4.21 | | security | **Keep a secret setting's raw value out of the boot error that rejects it.** | 1 | 1 | 4 | 5 | 2 | | 0.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 | +| 48 | 0.5.0 | | | **Correct the Keto tuple in the `login.ts` and `bootstrap.ts` comments to the `granted` relation the code writes.** | 1 | 1 | 1 | 1 | 1 | code health by 4 | 28.7 | +| 51 | 0.5.0 | | | **Move the test-only tooling out of the runtime folders `src/ui/` and `src/http/`, and keep the README file map true.** | 2 | 1 | 1 | 2 | 3 | code health by 4 | 18.0 | +| 52 | 0.5.0 | 51 | | **Make `positive` in `stylesheet-floor.ts` readable without tracing it by hand, and state why a custom property's value is checked.** | 2 | 1 | 1 | 2 | 3 | code health by 5 | 22.5 | +| 53 | 0.5.0 | | | **Say at `codeFlowSubmit` which branches of `flowFailure` can fire there.** | 1 | 1 | 1 | 1 | 1 | code health by 3 | 21.6 | +| 49 | 0.5.0 | | | **Render the admin's "group already exists" refusal from the catalog, in every locale.** | 1 | 2 | 1 | 2 | 1 | code health by 2 | 14.4 | | 6 | 0.5.0 | | | **Warn at discovery when a route or nav node gates on a permission no plugin declares.** | 2 | 1 | 1 | 2 | 2 | 7 by 3, 9 by 3 | 8.0 | | 23 | 0.5.0 | | | **Give list pages one view-model helper from query to pagination, instead of ~40 hand-written lines each.** | 3 | 1 | 2 | 4 | 5 | 1 by 4, 9 by 2, code health by 3 | 7.9 | | 43 | 0.5.0 | | | **Let a plugin version the scripts and other `/public/` files its own markup links, so they are cached as immutable too.** | 2 | 2 | 1 | 3 | 3 | 8 by 3, 7 by 2 | 5.0 | +| 47 | 0.5.0 | | | **Refuse the plugin id `error`, and make the drift guard cover the `/error` mount.** | 1 | 1 | 2 | 2 | 1 | 7 by 3 | 4.9 | | 28 | 0.5.0 | | | **Let a plugin add entries to the profile menu through the plugin API.** | 2 | 2 | 2 | 4 | 4 | 7 by 4, 1 by 2 | 4.2 | +| 50 | 0.5.0 | | | **Split `handleRequest` into named stages, so each fits the 50-line limit and the stage order is written once.** | 3 | 1 | 3 | 5 | 7 | code health by 7 | 3.7 | +| 54 | 0.5.0 | | | **List the `setting` conflict kind in the README's Conflict rules table.** | 1 | 1 | 1 | 1 | 1 | 9 by 2 | 3.1 | | 2 | 0.5.0 | | | **Rename the admin example plugin to a name that says what it administers.** | 3 | 1 | 2 | 4 | 6 | 9 by 3, code health by 2 | 2.6 | | 5 | 0.5.0 | | | **List grants that no installed plugin declares, with a remove action.** | 2 | 2 | 2 | 4 | 4 | 2 by 3 | 2.1 | | 41 | 0.5.0 | | | **Apply a permission save as one Keto call, so it cannot stop part-way.** | 2 | 2 | 2 | 4 | 3 | 2 by 2 | 2.1 | @@ -25,6 +34,13 @@ ## Details +### 46. Keep a secret setting's raw value out of the boot error that rejects it. + +`coerce` in `src/plugin-host/settings.ts` puts `got "${raw}"` (boolean, enum, number) and `: ${raw}` +(url) into the error it returns, whatever `secret` says. A secret of type `url`, such as a DSN with +a password, that fails `new URL` reaches the boot error and so the log. Name the variable and what it +accepts, never the value; test one secret of each type. + ### 19. Run the documented Docker commands on macOS and fix whatever misbehaves. Needs a Mac. Two suspects, both from `--user "$(id -u):$(id -g)"`: a macOS `id -g` is `20`, which is @@ -35,6 +51,36 @@ Needs a Mac. Two suspects, both from `--user "$(id -u):$(id -g)"`: a macOS `id - Ory's own examples model `Shift`, `Document` and so on. One global `Resource` bucket is the project's "no catch-all names" rule applied to namespaces. +### 48. Correct the Keto tuple in the `login.ts` and `bootstrap.ts` comments to the `granted` relation the code writes. + +`login.ts:40` and `bootstrap.ts:31` write `Permission:#members@user:`; the code, the OPL +file and the README use `granted`, and `members` is the Group relation. Raised by every comprehension +seat that read them. + +### 51. Move the test-only tooling out of the runtime folders `src/ui/` and `src/http/`, and keep the README file map true. + +`src/ui/stylesheet-floor.ts` imports a devDependency and only its test uses it; `src/http/app-harness.ts` +is a test fixture. Both sit beside runtime modules, and the README file map omits them. Seats also +named `src/http/builtin-routes.ts`, which holds no routes. + +### 52. Make `positive` in `stylesheet-floor.ts` readable without tracing it by hand, and state why a custom property's value is checked. + +Most scoring seats named `positive` (`:119`) with `walk` and `property` as opaque. Split it +into named steps, say in the header that an unsupported value in a custom property blanks every +`var()` that reads it, and keep the README's "Two browser floors" pointing here. It is the home of +the 3am answer. + +### 53. Say at `codeFlowSubmit` which branches of `flowFailure` can fire there. + +`submitFlow` throws only on a 5xx, so on the submit path only the 503 branch can fire; three seats +found that by opening a second file. + +### 49. Render the admin's "group already exists" refusal from the catalog, in every locale. + +`examples/plugins/admin/admin-groups.ts` returns the English string "A group with that name already +exists." where AGENTS.md puts every user-visible string in a catalog (`admin/i18n/en-US.ts`, then +`sv-SE.ts`). + ### 6. Warn at discovery when a route or nav node gates on a permission no plugin declares. Declaring stays optional on purpose, but the picker is built from declarations only, so such a route @@ -54,12 +100,31 @@ favicon and logo, and a plugin's `styles`. A file a plugin's own markup links, s the plugin API, hence a minor. Pick one with the plugin authors' panel, then document it in README → Reserved view locals. +### 47. Refuse the plugin id `error`, and make the drift guard cover the `/error` mount. + +`/error` is Kratos' error sink (`src/auth/routes.ts`), and plugin routes resolve before built-ins, so +a plugin with id `error` and a `/` route shadows it. Add `error` to `RESERVED_PLUGIN_IDS` and to the +mount list in `plugin.test.ts`'s drift guard, whose comment claims every built-in is derived. + ### 28. Let a plugin add entries to the profile menu through the plugin API. The profile dropdown stays a dropdown: Sign out is its only entry today, and it will gain more. `views/partials/shell.ejs` hand-rolls it (AGENTS.md → UI, the dropdown rule), so the entries need a manifest field that discovery checks, gated like a nav node. +### 50. Split `handleRequest` into named stages, so each fits the 50-line limit and the stage order is written once. + +The hardest place in all eight scoring seats of PR #200: `src/http/app.ts` `handleRequest` is about +165 lines at complexity 43, in `createApp`'s closure, and the order of its stages (headers, static, +canonical host, locale, session, CSRF, contexts, hooks, dispatch) is carried by comments only. Remove +it from `grandfathered` in `src/function-limits.test.ts`. Behaviour must not change; `app-*.test.ts` +is the net. Ships alone (R 5). + +### 54. List the `setting` conflict kind in the README's Conflict rules table. + +`findConflicts` (`plugin.ts`) emits `kind: "setting"` at `error` level and the table in README → +Conflict rules omits it. + ### 2. Rename the admin example plugin to a name that says what it administers. Candidates: `auth-admin`, `users-groups-admin`. The id is also the mount path, so the compose files,