§7 test-cleanup (todo §7); pass over the §7 test accretion. Written tests-first across four small commits, so unlike §6 there was no boilerplate triplication — the per-module matrices are one-contract-per-test and the reference plugin's shifts.test deliberately unit-tests its pure builders, so those stay. One genuine merge: dashboard.test had two sibling tests asserting the same contract (buildDashboardModel role-filters a gated nav source via composeNav) on two sources — the §5 Admin section + the §7 plugin fragments. Combined into one "dashboard role-filters the gated Admin section and plugin fragments, each independently" that also strengthens coverage with cross-gating assertions (admin doesn't see the plugin section, a scheduling:read holder doesn't see Admin) neither original checked; all prior assertions preserved, 3 model builds vs 4. Left separate (distinct functions/levels): the chrome-unit vs dashboard-unit vs app-HTTP plugin-nav tests, and the two app.test plugin integration tests (RouteResult shapes vs chrome+CSRF). Pure test refactor, no production code. 301 → 300 units; typecheck + tests green.
This commit is contained in:
@@ -72,31 +72,30 @@ test("dashboard applies the central menu config: branding + nav override (rename
|
|||||||
assert.ok(!labels.includes("Teams")); // "Teams" hidden
|
assert.ok(!labels.includes("Teams")); // "Teams" hidden
|
||||||
});
|
});
|
||||||
|
|
||||||
test("dashboard menu wires in the permission-gated Admin section (only for admins)", () => {
|
// The dashboard assembles its nav from two gated sources — the built-in Admin section and each
|
||||||
// An admin sees the Admin section with the four built-in screens.
|
// discovered plugin's fragment (§7) — and role-filters both via composeNav. One contract: a section
|
||||||
const admin = buildDashboardModel(new URL("http://x/"), ["admin"]);
|
// shows only for a holder of its permission, and each source is gated independently.
|
||||||
const adminNode = admin.nav.find((n) => n.label === "Admin");
|
test("dashboard role-filters the gated Admin section and plugin fragments, each independently", () => {
|
||||||
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)", () => {
|
|
||||||
const plugin = {
|
const plugin = {
|
||||||
apiVersion: "1.0.0", id: "scheduling",
|
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" }],
|
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 section = (m: ReturnType<typeof buildDashboardModel>, label: string) => m.nav.find((n) => n.label === label);
|
||||||
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")));
|
|
||||||
|
|
||||||
// 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]);
|
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", () => {
|
test("dashboard paginates: page 2 slices the next rows and preserves state in links", () => {
|
||||||
|
|||||||
2
todo.md
2
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/<id>/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] 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/<id>/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:<token>#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/<id>/`) but unusable (the shell `<head>` 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] 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:<token>#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/<id>/`) but unusable (the shell `<head>` 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.
|
- [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
|
## 8. Testing & CI
|
||||||
- [ ] node --test units across helpers / router / nav / auth (tests-first throughout).
|
- [ ] node --test units across helpers / router / nav / auth (tests-first throughout).
|
||||||
|
|||||||
Reference in New Issue
Block a user