Loosen plugin id rule (todo §2); allow digits and dashes anywhere (^[a-z0-9-]+$)

This commit is contained in:
2026-06-16 11:53:14 +02:00
parent 1623a81ddc
commit 09d616ddd3
4 changed files with 12 additions and 10 deletions

View File

@@ -33,8 +33,8 @@ plugins/scheduling/ # folder name = the plugin id → mounted at /schedulin
**Identity comes from the folder.** The folder name *is* the plugin `id`, and the mount path is
`/<id>` — neither is written in the manifest, so they can't drift or be claimed twice. The id
must be **kebab-case** (`isValidPluginId`: lowercase letters in dash-separated segments — no
digits, uppercase, or leading/trailing/double dashes); the host rejects a malformed folder name
must be **URL/path-safe** (`isValidPluginId`: lowercase `az`, digits, and dashes — dashes
anywhere; no uppercase, underscores, dots, or slashes); the host rejects a malformed folder name
at discovery. The id also namespaces the plugin's `views/`, its `/public/<id>/` assets, and (by
convention) its nav/permission tokens.

View File

@@ -36,9 +36,11 @@ test("definePlugin returns the manifest unchanged — id/mount come from the fol
assert.equal(scheduling.routes?.length, 3);
});
test("isValidPluginId accepts kebab-case folder names and rejects everything else", () => {
for (const ok of ["scheduling", "people", "people-directory"]) assert.ok(isValidPluginId(ok), ok);
for (const bad of ["People", "people_dir", "people-", "-people", "people--dir", "people1", "", "a/b"]) {
test("isValidPluginId accepts lowercase/digits/dashes anywhere and rejects everything else", () => {
for (const ok of ["scheduling", "people-directory", "people2", "v2", "people--dir", "-people", "people-", "a-1-b", "1"]) {
assert.ok(isValidPluginId(ok), ok);
}
for (const bad of ["People", "people_dir", "a/b", "a.b", "a b", ""]) {
assert.ok(!isValidPluginId(bad), bad);
}
});

View File

@@ -71,10 +71,10 @@ export function definePlugin(manifest: PluginManifest): PluginManifest {
return manifest;
}
// A plugin id (its folder name) — lowercase letters in dash-separated segments: no digits,
// uppercase, or leading/trailing/double dashes. Tight on purpose: the id forms the mount path
// `/<id>`, the view/static namespace, and the central-override target.
const PLUGIN_ID = /^[a-z]+(?:-[a-z]+)*$/;
// A plugin id (its folder name) — lowercase az, digits, and dashes, dashes allowed anywhere.
// Rejects uppercase, underscores, dots, slashes, spaces: the id forms the mount path `/<id>`,
// the view/static namespace, and the central-override target, so it must stay URL/path-safe.
const PLUGIN_ID = /^[a-z0-9-]+$/;
export function isValidPluginId(id: string): boolean {
return PLUGIN_ID.test(id);

View File

@@ -45,7 +45,7 @@ everything via Docker.
- [x] Add to principles that we should have full E2E coverage in the Playwright tests - make sure they can run in parallel to get up some speed. → Added **Full, parallel E2E** core principle (AGENTS.md §6 + README): every user-facing flow gets a Playwright test shipped with it, tests stay side-effect-free so the suite runs `fullyParallel` (already set; verified 7 tests / 7 workers). Led by example: added E2E coverage for the 404 page (the one user-facing gap). Fixed the documented run command to `--build` (the runner bakes in `e2e/`, so spec edits were silently ignored without it).
## 2. Plugin host
- [x] **Specify the plugin contract** (big job, do first — it's the product's main API surface). Write it down as the authoritative reference: the full manifest shape; the `RequestContext` handed to handlers and what's guaranteed stable; **contract versioning** (a `apiVersion`/`engines`-style field so a plugin declares the host it targets, and the host refuses or warns on mismatch); **conflict rules** (two plugins claiming the same `basePath`, nav slot, or `permission` name → defined, loud resolution, not last-write-wins); the **local dev/test story** (how an author runs + tests one plugin in isolation against the host). Audience is experienced devs: optimise for a powerful, predictable, clearly-documented API. Crash-isolation (a bad plugin can't take down the host) is a *nice-to-have*, not a blocker — fail loud at boot/discovery over sandboxing at runtime. It is a target that plugins should be able to overload as much as possible. Hooks on actions in the system is not bad either, if it is possible. → `src/plugin.ts` is the typed, machine-readable contract (single source of truth: authored `PluginManifest` + folder-derived `Plugin`, `Route`/`RouteResult`/`RouteHandler`, `PermissionDecl`, `PluginHooks`, `definePlugin()`, `HOST_API_VERSION`) plus the pure rules the §2 host enforces — `isValidPluginId` (kebab-case folder name), `checkApiVersion` (semver via `parseSemver`/official regex, no dep: same major+minor→ok, older minor→warn, newer minor/major-mismatch/malformed→refuse) and `findConflicts` (id/route = error, duplicate nav-id = error, shared permission token = warn; never last-write-wins). Identity is the folder: id = folder name, mount = `/<id>` — neither is in the manifest, so mount-path uniqueness is structural (no basePath rule). `apiVersion` is a literal a plugin pins (never imports `HOST_API_VERSION`). nav `icon` = Lucide sprite id. `docs/plugin-contract.md` is the prose reference (anatomy/identity, manifest fields, handler/RouteResult, `RequestContext` stability guarantee, nav/permission namespacing, versioning, conflicts, hooks, dev/test story). README links it. Tests-first (`plugin.test.ts`); typecheck + 82 units green. Discovery/router/view-resolver/static stay as the next §2 items that wire this to FS+HTTP.
- [x] **Specify the plugin contract** (big job, do first — it's the product's main API surface). Write it down as the authoritative reference: the full manifest shape; the `RequestContext` handed to handlers and what's guaranteed stable; **contract versioning** (a `apiVersion`/`engines`-style field so a plugin declares the host it targets, and the host refuses or warns on mismatch); **conflict rules** (two plugins claiming the same `basePath`, nav slot, or `permission` name → defined, loud resolution, not last-write-wins); the **local dev/test story** (how an author runs + tests one plugin in isolation against the host). Audience is experienced devs: optimise for a powerful, predictable, clearly-documented API. Crash-isolation (a bad plugin can't take down the host) is a *nice-to-have*, not a blocker — fail loud at boot/discovery over sandboxing at runtime. It is a target that plugins should be able to overload as much as possible. Hooks on actions in the system is not bad either, if it is possible. → `src/plugin.ts` is the typed, machine-readable contract (single source of truth: authored `PluginManifest` + folder-derived `Plugin`, `Route`/`RouteResult`/`RouteHandler`, `PermissionDecl`, `PluginHooks`, `definePlugin()`, `HOST_API_VERSION`) plus the pure rules the §2 host enforces — `isValidPluginId` (URL-safe folder name: lowercase/digits/dashes), `checkApiVersion` (semver via `parseSemver`/official regex, no dep: same major+minor→ok, older minor→warn, newer minor/major-mismatch/malformed→refuse) and `findConflicts` (id/route = error, duplicate nav-id = error, shared permission token = warn; never last-write-wins). Identity is the folder: id = folder name, mount = `/<id>` — neither is in the manifest, so mount-path uniqueness is structural (no basePath rule). `apiVersion` is a literal a plugin pins (never imports `HOST_API_VERSION`). nav `icon` = Lucide sprite id. `docs/plugin-contract.md` is the prose reference (anatomy/identity, manifest fields, handler/RouteResult, `RequestContext` stability guarantee, nav/permission namespacing, versioning, conflicts, hooks, dev/test story). README links it. Tests-first (`plugin.test.ts`); typecheck + 82 units green. Discovery/router/view-resolver/static stay as the next §2 items that wire this to FS+HTTP.
- [ ] Discovery: scan `plugins/`, import each `plugin.ts` default export, validate.
- [ ] Router: match method+path under `basePath`, resolve path params, run permission gate, call handler with context.
- [ ] Per-plugin view resolver (`plugins/<id>/views/*.ejs`).