Consolidate tests (todo §2); merge HTTP static tests, fold 403 render into the live gated route, unify resolveViewPath cases

This commit is contained in:
2026-06-16 16:42:46 +02:00
parent 9489bd124b
commit a602f794d1
3 changed files with 15 additions and 23 deletions

View File

@@ -54,7 +54,7 @@ everything via Docker.
- [x] Wire branding into the app shell. → Completes the §2 branding chain (name/sub already flowed). `shell.ejs` now renders `brand.logo` as `<img class="brand-logo" alt="">` when set, else the default `#i-box` brand-mark; the `theme` local (already forwarded to the theme-switch) is now supplied. `buildDashboardModel` puts `menu.branding.logo` into `shell.brand` and `menu.branding.theme` into `shell.theme` (both omitted when unset, so a clean clone is unchanged → brand-mark + auto theme); `views/index.ejs` forwards `theme` to the shell. Added a `.brand-logo` CSS rule (22px, matches `.brand-mark` sizing). Tests-first: `shell.test.ts` (logo replaces the mark + default theme checked; no-logo ⇒ mark + auto) + extended `dashboard.test.ts` (logo→brand, theme→shell.theme) + an `app.test.ts` integration rendering `createApp({ menu })` end-to-end (logo `<img>` + `theme-dark` checked on `/`). Default-app shell rendering is byte-equivalent, so the visual E2E is unaffected; typecheck + 109 units green. The §2 plugin host is feature-complete (remaining §2 items are the project-wide review + comment/test cleanup).
- [x] Run the architecture _and_ the stability reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Ran both on all of `src/`, `views/`, `config/`, Docker/tsconfig. Verdict: architecture sound + disciplined, no crash/security defect in the current path (fail-loud, traversal guards, JWT/cookie defenses all confirmed). **Fixed now:** (1) HIGH — `PluginHooks` was typed+documented but never invoked; wired it (`src/hooks.ts`: `runBootHooks`/`runRequestHooks`/`runResponseHooks`) — `server.ts` runs `onBoot` after discovery before listen, `app.ts` runs `onRequest` (before routing, first non-void short-circuits, renders against its plugin) + `onResponse` (after handler, observer, throw→500); skipped entirely when no plugin declares a hook (hot path free); `hooks.test.ts` + an `app.test.ts` integration. (2) `discovery.ts` `fail` helper retyped `: void`. (3) Documented the template trust boundary in `docs/plugin-contract.md` (raw `html`/`*.html` fields; URL sinks escaped but not scheme-checked) + tightened the Hooks prose to the wired semantics. **Deferred (reviewer-scoped, not §2):** extract a shared `buildShellContext` out of `dashboard.ts` and route the built-in screens through `matchRoute`/`isAuthorized` → §5 (premature at one call site); a `safeUrl()` helper for href sinks → §4 (no untrusted URLs until upstream data flows); doc/type-duplication + non-local `§N` refs → the §2 comment-cleanup item; HEAD-render cost + dev empty-secret fallback → negligible. typecheck + 113 units green; boot smoke-tested.
- [x] Go over all comments in the code and the README and try to make it shorter and more information dense. Remove not strictly needed stuff. → Pass over the §2 accretion (the §0/§1 cleanup at line 21 stands). Tightened the verbose module-header blocks (`plugin.ts`, `discovery.ts`, `router.ts`, `dashboard.ts`) and collapsed the `checkApiVersion` rule comment to a one-liner that points at the contract doc (the if-chain + messages already document it). Removed now-stale forward-refs ("router wiring is the next §2 item", "rendered in the shell — next §2 item"). README: corrected the **Status** note (it undersold — §1 design system + the whole §2 plugin host are built, not just a scaffold), dropped the stale `_(planned)_`/"planned to extract" markers on **Building a plugin** and **Building blocks** (both shipped; auth guards still flagged §4), and named the real helpers. Left the security-rationale comments (jwt/cookie/static/paginate) and the EJS partials' config-doc headers intact — they carry vital info / are the only schema for untyped locals. No anchor links broke; typecheck + 113 units green.
- [ ] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us.
- [x] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us. → Reviewed all 24 test files. The suite already follows the deliberate per-module "matrix + edge" pattern from the §0/§1 merge (line 22), so most files carry no fat and force-merging distinct concerns would only hurt readability. Removed the genuine §2-era overlaps, all in `app.test.ts`: merged the two HTTP static tests into one (GET/HEAD + traversal/NUL→403), and dropped the standalone "renders the 403 error page" `ejs.renderFile` stopgap (its comment even said "403 has no first-party route yet") — the gated plugin route now exercises 403 over HTTP, so the template assertions (status + 403.ejs body + stylesheet link) moved there; also dropped the now-unused `ejs` import. Unified `view-resolver.test.ts`'s two `resolveViewPath` cases (resolve + reject) into one. 113 → 110 tests, zero coverage lost; typecheck + tests green.
## 3. Ory stack — compose + config
- [ ] `postgres` service (pinned tag); separate DB/schema per Kratos/Keto/Hydra.