§7 review checkpoint (todo §7); ran the architecture + product reviewers on the whole project and addressed findings, no Critical from either. Made permissions honest + decoupled the host from the plugin: new pure seedRoles + bootstrap discoverPlugins() seeds the demo admin admin(/ADMIN_ROLES) ∪ every discovered plugin's declared tokens, dropped the hardcoded scheduling:* from compose ADMIN_ROLES (clean-clone unchanged); docs now state a route/nav permission is a coarse role granted as Keto Role:<token>#members. Added src/plugin-api.ts — the stable author barrel the reference plugin now imports from instead of deep src/* (the contract boundary in code). Made per-plugin CSS usable: shell styles slot + plugins/scheduling/public/scheduling.css linked from the views. Reference now demonstrates hooks.onBoot validating SCHEDULING_UPSTREAM fail-loud (assertHttpUrl). Build ctx.chrome at most once per request (memoized). Doc honesty: fixed the false visual.spec coverage comment, softened the "every plugin ships a Playwright test" claim (authed flow = §8), added an Upstream contract block to the plugin README. Added LICENSE (MIT). Stability-reviewer APPROVE, no Critical/High; addressed both Low nits. typecheck + 301 units green. Deferred: internal route-table (M1)→§9, safeUrl()→§9, data-table empty-state + success-flash→§8/polish, apiVersion-literal enforcement (prose), permission→requireRole rename (future minor).
This commit is contained in:
@@ -20,9 +20,22 @@ The plugin holds **no state** — data lives upstream (README → *Stateless*).
|
||||
|
||||
## Upstream
|
||||
|
||||
Set `SCHEDULING_UPSTREAM` to your backend's base URL (it must expose `GET /shifts` and
|
||||
`POST /shifts`). The dev compose points it at a tiny in-memory mock (`examples/shifts-upstream/`)
|
||||
so `docker compose up` shows the plugin working out of the box.
|
||||
Set `SCHEDULING_UPSTREAM` to your backend's base URL. The dev compose points it at a tiny in-memory
|
||||
mock (`examples/shifts-upstream/`) so `docker compose up` shows the plugin working out of the box.
|
||||
A malformed/non-http URL fails the boot loudly (the plugin's `onBoot` hook).
|
||||
|
||||
### Upstream contract
|
||||
|
||||
Your backend must expose two routes; the plugin treats any non-2xx as a recoverable failure
|
||||
(the list degrades to a "try again" alert, the create re-renders the form keeping the input).
|
||||
|
||||
| Route | Request | Success | Response body |
|
||||
| --- | --- | --- | --- |
|
||||
| `GET /shifts` | `Accept: application/json` | `200` | JSON array of `{ id, title, assignee, start, end }` (all strings; missing fields coerce to `""`) |
|
||||
| `POST /shifts` | JSON body `{ title, assignee, start, end }` | `2xx` | ignored (the plugin POST-redirect-GETs back to the list) |
|
||||
|
||||
Domain rules (overlap, capacity, time ordering) live in your backend — reject with a 4xx and the
|
||||
form re-renders. The plugin only validates that `title` and `assignee` are non-empty.
|
||||
|
||||
## Granting access
|
||||
|
||||
|
||||
@@ -2,16 +2,21 @@
|
||||
// data, a CSRF-guarded form that forwards a write upstream, and permission-gated nav. Copy this
|
||||
// folder, rename it, point it at your own backend. Full contract: docs/plugin-contract.md.
|
||||
|
||||
import { definePlugin } from "../../src/plugin.ts";
|
||||
import { createShift, createUpstream, listShifts, newShiftForm, READ, SHIFTS_PATH, WRITE } from "./shifts.ts";
|
||||
import { definePlugin } from "../../src/plugin-api.ts";
|
||||
import { assertHttpUrl, createShift, createUpstream, listShifts, newShiftForm, READ, SHIFTS_PATH, WRITE } from "./shifts.ts";
|
||||
|
||||
// The upstream this plugin reads/writes — a stand-in for your real backend (the plugin is
|
||||
// stateless). Configure via env; the dev compose points it at a tiny mock (examples/shifts-upstream).
|
||||
const upstream = createUpstream(process.env["SCHEDULING_UPSTREAM"] ?? "http://shifts-upstream:4000");
|
||||
const upstreamUrl = process.env["SCHEDULING_UPSTREAM"] ?? "http://shifts-upstream:4000";
|
||||
const upstream = createUpstream(upstreamUrl);
|
||||
|
||||
export default definePlugin({
|
||||
apiVersion: "1.0.0", // the host contract this was built against — a literal, never HOST_API_VERSION
|
||||
|
||||
// onBoot runs after discovery, before the server listens: validate the plugin's own config so a
|
||||
// typo'd SCHEDULING_UPSTREAM fails the boot loudly instead of degrading every request later.
|
||||
hooks: { onBoot: () => assertHttpUrl(upstreamUrl, "SCHEDULING_UPSTREAM") },
|
||||
|
||||
// Merged into the global menu + filtered per user: the "Shifts" leaf shows only for a user holding
|
||||
// `scheduling:read`, so the whole "Scheduling" header disappears for everyone else.
|
||||
nav: [{
|
||||
|
||||
6
plugins/scheduling/public/scheduling.css
Normal file
6
plugins/scheduling/public/scheduling.css
Normal file
@@ -0,0 +1,6 @@
|
||||
/* Scheduling plugin styles — served at /public/scheduling/scheduling.css (per-plugin static),
|
||||
linked via the shell `styles` slot. The core design system already styles every building block,
|
||||
so a plugin only adds what's domain-specific, scoped under its own page class (.scheduling-page). */
|
||||
.scheduling-page .table-wrap {
|
||||
margin-top: var(--space-3, 0.75rem);
|
||||
}
|
||||
@@ -7,7 +7,7 @@ import type { RequestContext } from "../../src/context.ts";
|
||||
import { GuardError } from "../../src/guards.ts";
|
||||
import type { RouteResult } from "../../src/plugin.ts";
|
||||
import {
|
||||
buildFormModel, createShift, createUpstream, listShifts, newShiftForm, readInput,
|
||||
assertHttpUrl, buildFormModel, createShift, createUpstream, listShifts, newShiftForm, readInput,
|
||||
SHIFTS_PATH, type Shift, type ShiftInput, type ShiftsUpstream, UpstreamError, validate,
|
||||
} from "./shifts.ts";
|
||||
|
||||
@@ -33,6 +33,29 @@ const asView = (r: RouteResult | void) => {
|
||||
return r as { data: Record<string, unknown>; status?: number; view: string };
|
||||
};
|
||||
|
||||
// ---- upstream config validation (the onBoot hook) ----
|
||||
|
||||
test("assertHttpUrl accepts http(s) and fails loud on a malformed or non-http upstream URL", () => {
|
||||
assert.doesNotThrow(() => assertHttpUrl("http://shifts-upstream:4000", "SCHEDULING_UPSTREAM"));
|
||||
assert.doesNotThrow(() => assertHttpUrl("https://api.example.com/v1", "SCHEDULING_UPSTREAM"));
|
||||
assert.throws(() => assertHttpUrl("not a url", "SCHEDULING_UPSTREAM"), /SCHEDULING_UPSTREAM.*valid URL/); // unparseable
|
||||
assert.throws(() => assertHttpUrl("shifts-upstream:4000", "SCHEDULING_UPSTREAM"), /SCHEDULING_UPSTREAM.*http/); // missing // → parsed as a bogus scheme
|
||||
assert.throws(() => assertHttpUrl("ftp://host/x", "SCHEDULING_UPSTREAM"), /SCHEDULING_UPSTREAM.*http/); // wrong scheme
|
||||
});
|
||||
|
||||
test("the manifest's onBoot hook validates SCHEDULING_UPSTREAM (the binding, not just the helper)", async () => {
|
||||
const prev = process.env["SCHEDULING_UPSTREAM"];
|
||||
process.env["SCHEDULING_UPSTREAM"] = "nope://bad"; // read at import time below
|
||||
try {
|
||||
const manifest = (await import("./plugin.ts")).default;
|
||||
assert.equal(typeof manifest.hooks?.onBoot, "function");
|
||||
assert.throws(() => manifest.hooks!.onBoot!(), /SCHEDULING_UPSTREAM/); // bad upstream → boot fails loud
|
||||
} finally {
|
||||
if (prev === undefined) delete process.env["SCHEDULING_UPSTREAM"];
|
||||
else process.env["SCHEDULING_UPSTREAM"] = prev;
|
||||
}
|
||||
});
|
||||
|
||||
// ---- upstream client (fetch injected) ----
|
||||
|
||||
test("createUpstream.list fetches /shifts, asks for JSON, and maps the rows", async () => {
|
||||
|
||||
@@ -5,12 +5,8 @@
|
||||
// Handlers are factories bound to a ShiftsUpstream, and `fetch` is injectable, so they unit-test as
|
||||
// pure functions against a mock upstream with no network (docs/plugin-contract.md → dev/test story).
|
||||
|
||||
import { readFormBody } from "../../src/body.ts";
|
||||
import type { PageChrome } from "../../src/chrome.ts";
|
||||
import { CSRF_FIELD } from "../../src/csrf.ts";
|
||||
import { can, GuardError } from "../../src/guards.ts";
|
||||
import { parseListQuery } from "../../src/list-query.ts";
|
||||
import type { RouteHandler } from "../../src/plugin.ts";
|
||||
// One import from the host's plugin-api barrel — the stable author surface (see docs/plugin-contract.md).
|
||||
import { can, CSRF_FIELD, GuardError, type PageChrome, parseListQuery, readFormBody, type RouteHandler } from "../../src/plugin-api.ts";
|
||||
|
||||
export const SHIFTS_PATH = "/scheduling/shifts";
|
||||
export const READ = "scheduling:read"; // permission token gating the list + nav
|
||||
@@ -46,6 +42,18 @@ export interface ShiftsUpstream {
|
||||
list(): Promise<Shift[]>;
|
||||
}
|
||||
|
||||
// Fail loud at boot (the plugin's onBoot hook) on a malformed/non-http upstream URL — a config
|
||||
// typo surfaces at startup, not as a degraded page later. Reachability stays a runtime concern.
|
||||
export function assertHttpUrl(value: string, name: string): void {
|
||||
let url: URL;
|
||||
try {
|
||||
url = new URL(value);
|
||||
} catch {
|
||||
throw new Error(`${name} is not a valid URL: ${JSON.stringify(value)}`);
|
||||
}
|
||||
if (url.protocol !== "http:" && url.protocol !== "https:") throw new Error(`${name} must be an http(s) URL: ${JSON.stringify(value)}`);
|
||||
}
|
||||
|
||||
// REST client over the upstream service (a stand-in for the customer's real backend). `fetch` is
|
||||
// injectable so handlers test without a network; the base URL comes from the plugin's own env.
|
||||
export function createUpstream(baseUrl: string, fetchImpl: typeof fetch = fetch): ShiftsUpstream {
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
Data: chrome, title, breadcrumbs, form, formError?
|
||||
%><%
|
||||
const navHtml = include("partials/nav-tree", { nodes: chrome.nav });
|
||||
const body = include("partials/shift-form", { form, formError: locals.formError });
|
||||
const body = '<div class="scheduling-page">' + include("partials/shift-form", { form, formError: locals.formError }) + '</div>';
|
||||
-%>
|
||||
<%- include("partials/shell", {
|
||||
body,
|
||||
@@ -13,6 +13,7 @@
|
||||
breadcrumbs,
|
||||
csrfToken: chrome.csrfToken,
|
||||
nav: navHtml,
|
||||
styles: ["/public/scheduling/scheduling.css"],
|
||||
theme: chrome.theme,
|
||||
title,
|
||||
user: chrome.user,
|
||||
|
||||
@@ -15,11 +15,12 @@
|
||||
-%>
|
||||
<%- include("partials/shell", {
|
||||
actions,
|
||||
body: alertHtml + filtersHtml + tableHtml,
|
||||
body: '<div class="scheduling-page">' + alertHtml + filtersHtml + tableHtml + '</div>',
|
||||
brand: chrome.brand,
|
||||
breadcrumbs,
|
||||
csrfToken: chrome.csrfToken,
|
||||
nav: navHtml,
|
||||
styles: ["/public/scheduling/scheduling.css"],
|
||||
theme: chrome.theme,
|
||||
title,
|
||||
user: chrome.user,
|
||||
|
||||
Reference in New Issue
Block a user