diff --git a/src/dashboard.test.ts b/src/dashboard.test.ts index a89be6d..8c77d4f 100644 --- a/src/dashboard.test.ts +++ b/src/dashboard.test.ts @@ -72,31 +72,30 @@ test("dashboard applies the central menu config: branding + nav override (rename assert.ok(!labels.includes("Teams")); // "Teams" hidden }); -test("dashboard menu wires in the permission-gated Admin section (only for admins)", () => { - // An admin sees the Admin section with the four built-in screens. - const admin = buildDashboardModel(new URL("http://x/"), ["admin"]); - const adminNode = admin.nav.find((n) => n.label === "Admin"); - assert.ok(adminNode, "admin role → Admin section present"); - assert.deepEqual(adminNode!.children?.map((c) => c.href), ["/admin/users", "/admin/groups", "/admin/roles", "/admin/clients"]); - - // A non-admin (default []) never sees it — composeNav drops the gated header + its subtree. - const plain = buildDashboardModel(new URL("http://x/")); - assert.equal(plain.nav.find((n) => n.label === "Admin"), undefined); - assert.ok(!plain.nav.some((n) => n.children?.some((c) => c.href === "/admin/users"))); -}); - -test("dashboard merges discovered plugin nav fragments, permission-filtered (§7)", () => { +// The dashboard assembles its nav from two gated sources — the built-in Admin section and each +// discovered plugin's fragment (§7) — and role-filters both via composeNav. One contract: a section +// shows only for a holder of its permission, and each source is gated independently. +test("dashboard role-filters the gated Admin section and plugin fragments, each independently", () => { const plugin = { apiVersion: "1.0.0", id: "scheduling", nav: [{ children: [{ href: "/scheduling/shifts", id: "scheduling:shifts", label: "Shifts", permission: "scheduling:read" }], icon: "i-cal", id: "scheduling", label: "Scheduling" }], }; - // A holder of the plugin permission sees its section, reachable from "/". - const granted = buildDashboardModel(new URL("http://x/"), ["scheduling:read"], undefined, "", null, [plugin]); - assert.ok(granted.nav.some((n) => n.children?.some((c) => c.href === "/scheduling/shifts"))); + const section = (m: ReturnType, label: string) => m.nav.find((n) => n.label === label); - // Anonymous: the gated leaf (and so the whole Scheduling header) is filtered out. + // An admin sees the Admin section (the four built-in screens) but not the plugin's gated section. + const admin = buildDashboardModel(new URL("http://x/"), ["admin"], undefined, "", null, [plugin]); + assert.deepEqual(section(admin, "Admin")!.children?.map((c) => c.href), ["/admin/users", "/admin/groups", "/admin/roles", "/admin/clients"]); + assert.equal(section(admin, "Scheduling"), undefined); + + // A plugin-permission holder sees the plugin section (reachable from "/") but not Admin. + const member = buildDashboardModel(new URL("http://x/"), ["scheduling:read"], undefined, "", null, [plugin]); + assert.ok(section(member, "Scheduling")!.children?.some((c) => c.href === "/scheduling/shifts")); + assert.equal(section(member, "Admin"), undefined); + + // Anonymous sees neither — composeNav drops a gated header and its whole subtree. const anon = buildDashboardModel(new URL("http://x/"), [], undefined, "", null, [plugin]); - assert.equal(anon.nav.find((n) => n.label === "Scheduling"), undefined); + assert.equal(section(anon, "Admin"), undefined); + assert.equal(section(anon, "Scheduling"), undefined); }); test("dashboard paginates: page 2 slices the next rows and preserves state in links", () => { diff --git a/todo.md b/todo.md index c762cca..1bbcef7 100644 --- a/todo.md +++ b/todo.md @@ -114,7 +114,7 @@ everything via Docker. - [x] Verify the full plugin contract end-to-end against the README. → Cross-checked every contract claim in `README.md` (**Building a plugin**, **Where plugins live**, **Layout**) + `docs/plugin-contract.md` + `plugins/scheduling/README.md` against the implementation (`plugin.ts`/`discovery.ts`/`router.ts`/`view-resolver.ts`/`chrome.ts`/`context.ts`/`guards.ts`/`app.ts` `sendResult`) and the shipped reference plugin. **Contract holds end-to-end**: derived id+mount (no `id`/`basePath` in the manifest), `apiVersion` literal + `checkApiVersion` table, permission-gated nav (the "Scheduling" header vanishes without `scheduling:read`) + gated routes, every `RouteResult` shape incl. `view`+`status` and `redirect`-PRG, `ctx.chrome` → native app shell, `ctx.verifyCsrf` double-submit (403 on bad token), `can()`/`GuardError(403)`, the view-resolver rendering `plugins//views/*` while `include()`-ing **both** core partials (shell/filter-bar/data-table/field/alert/nav-tree — all present in `views/partials/`) and the plugin's own `partials/shift-form`, per-plugin static, upstream-fetch statelessness; all three reference-plugin icons (`i-cal`/`i-plus`/`i-user`) are registered in `src/icons.ts`. **One material doc drift found + fixed:** `docs/plugin-contract.md`'s reserved-id list was stale — it named 8 ids but `RESERVED_PLUGIN_IDS` has 10 (the §5 `admin` + §6 `oauth2` mounts were missing), so an author would wrongly think `plugins/admin/` or `plugins/oauth2/` was allowed when discovery refuses them (test-covered in `discovery.test.ts`). Updated the doc to list all 10 accurately (docs-only, no behaviour change). typecheck + 296 units green. - [x] Run the architecture and the product reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Ran both on the whole project (weighted to the §7 reference-plugin surfaces). Architecture: **no Critical**; Product: **no Critical** this checkpoint. **Fixed now (tests-first):** (1) HIGH (arch + product Blocker) — the contract claimed declaring `permissions` "seeds Keto" but nothing wired it, and the host hardcoded the plugin-specific `scheduling:read`/`write` into `ADMIN_ROLES` (`compose.yml` + `bootstrap.ts`) — core knowing about a plugin. Made the claim true *and* removed the coupling: new pure `seedRoles(adminRolesEnv, declaredTokens)` (union+dedup, trims both sides); `bootstrap.main()` now `discoverPlugins()` → seeds the demo admin `admin`(/`ADMIN_ROLES`) ∪ every discovered plugin's declared tokens; `compose.yml` `ADMIN_ROLES` default dropped to just `admin` (host names no plugin). Clean-clone behaviour is unchanged (admin still ends up with the scheduling tokens, now derived via discovery, not hardcoded — `readRoles` enumerates the Role namespace so a granted token reaches the JWT). Docs corrected: a route/nav `permission` **is a coarse role** granted as a Keto `Role:#members` tuple (vs the separate fine-grained `Resource` tier). (2) HIGH (arch) — no delineated plugin SDK: the reference plugin imported six deep `src/*` internals, none of them the documented-stable surface. Added `src/plugin-api.ts` — the one barrel a plugin imports (`definePlugin` + manifest/handler/ctx types + guards + body/CSRF/list-query helpers); repointed `plugins/scheduling/{plugin,shifts}.ts` to it, so the contract boundary now lives in code (the host can refactor internals freely behind it). (3) MEDIUM (arch L1 + product) — per-plugin CSS was documented (`/public//`) but unusable (the shell `` had no slot) and the reference shipped no `public/`. Added an optional `styles` slot to `shell.ejs` (extra stylesheet hrefs, default `[]`, backward-compatible) + `plugins/scheduling/public/scheduling.css` linked from both reference views (`.scheduling-page` scope) — closes the only unexercised contract feature + the anatomy-diagram drift. (4) MEDIUM (arch M4) — the reference demonstrated no hook; added `hooks.onBoot` validating `SCHEDULING_UPSTREAM` fail-loud at boot (`assertHttpUrl` — unparseable or non-http(s) aborts the boot, a typo no longer degrades every request), closing the hooks-coverage gap. (5) MEDIUM (arch M3) — `ctx.chrome` was built twice on a matched plugin request when any onRequest hook exists; now built at most once per request (memoized lazy `chrome()`), hot path still chrome-free. (6) HIGH/MEDIUM (product) — doc honesty: fixed the false `visual.spec.ts` comment ("covered by the full-stack suites" — it isn't), softened the contract's "every plugin ships *with* a Playwright test" to match reality (the reference's authed list/form happy-path is the §8 item), and added an **Upstream contract** block (routes/JSON shape/status-code expectations) to `plugins/scheduling/README.md`. Stability-reviewer run as a local PR: **APPROVE, no Critical/High**; addressed both Low nits (`seedRoles` now trims/dedups declared tokens too; added an `onBoot`-binding seam test on the real manifest). typecheck + **301 units** green (296 → 301). **Deferred (reviewer-scoped, not the §7 checkpoint):** the host **internal route-table** (fold the admin/oauth if-ladder into one `{method,prefix,handler}` table, derive `RESERVED_PLUGIN_IDS`/`allowedMethods` from it — arch M1) → **§9** (now the top structural item; standalone change, not a review-fix); the `safeUrl()` href helper + a reference row that renders an untrusted URL through it (arch L2 / product 🟢) → **§9** (first untrusted-URL flow / redirect-allowlist work); the data-table **empty state** + **success-flash** after writes (product 🟡, recurring §5/§6 deferral, now visible in the exemplar) → **§8/polish**; compile-time `apiVersion`-literal enforcement (arch M2) → accept prose-only; rename route/nav `permission` → `requireRole` to match the mechanism (arch H1.3) → a future minor `apiVersion` bump. - [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 §7 accretion (reference plugin + the §7-review host additions). The §7 modules were authored dense — the reference plugin (`plugin.ts`/`shifts.ts`/views/`examples/shifts-upstream`) is a deliberate teaching artifact, and the host bits (`context.ts`/`plugin-api.ts`/`bootstrap.ts`/`app.ts` §7 region) were written concise — so the wins are targeted: (1) tightened `chrome.ts`'s module header (7→5 lines) — dropped the parenthetical input-list (visible in `ChromeOptions`) and the nav-composition restatement (already carried by the `nav` field comment + `markCurrent`'s own comment). (2) Fixed a stale forward-ref in `docs/plugin-contract.md`: the `safeUrl()` helper note said "will land … §5/§7", but §7 deferred it to §9 (line 115) — now "planned for §9, with the redirect-URI allowlist work". Left intact: the reference plugin's instructive comments (it's the copy-target), the EJS view config-doc headers (the only schema for untyped locals), and the contract doc + plugin README (authored/refined concise in the §7 verify + review items). README **Building a plugin**/Layout/bootstrap prose was already tight (§7) — untouched; the README **Status**/`_(planned)_`/Layout refresh stays §9 (line 133). Docs/comments-only (per AGENTS.md, no stability reviewer); typecheck + 301 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. → Pass over the §7 test accretion (`shifts.test.ts`, `chrome.test.ts`, `plugin-api.test.ts`, `shell.test.ts`, + the §7 additions to `app`/`dashboard`/`bootstrap`). The §7 tests were written tests-first across four small commits, so unlike §6 (a triplicated degrade matrix) there was no boilerplate accretion — the per-module matrices are one-contract-per-test and the reference plugin's `shifts.test.ts` deliberately tests its pure builders in isolation (the dev/test pattern the contract preaches), so those stay. **One genuine "combine in a good way":** `dashboard.test.ts` had two sibling tests — "wires in the permission-gated Admin section" (§5) and "merges discovered plugin nav fragments, permission-filtered" (§7) — that assert the *same* contract (`buildDashboardModel` role-filters a gated nav source via `composeNav`) on two sources. Merged into one "dashboard role-filters the gated Admin section and plugin fragments, each independently", which also **strengthens** coverage: it now asserts cross-gating (an `admin` doesn't see the plugin section, a `scheduling:read` holder doesn't see Admin) that neither original checked — 3 model builds vs 4, all prior assertions preserved. Left separate (distinct code paths/levels, not fat): the chrome-unit vs dashboard-unit vs app-HTTP plugin-nav tests (three different functions — `buildPluginChrome`, `buildDashboardModel`, the rendered shell — each independently merges fragments), and the two `app.test.ts` plugin integration tests (RouteResult shapes/static/405 vs chrome+CSRF round-trip — different surfaces, own fixtures). Pure test refactor, no production code touched (per the §6 test-cleanup precedent, no stability reviewer). 301 → 300 units; typecheck + tests green. ## 8. Testing & CI - [ ] node --test units across helpers / router / nav / auth (tests-first throughout).