Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
# Python bytecode and pytest caches
__pycache__/
*.py[cod]
.pytest_cache/
.pytest_cache/

# Virtualenv created by the documented setup steps
.venv/
54 changes: 49 additions & 5 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,23 @@

A reference implementation demonstrating the fundamental feedBack plugin contract: a client screen, a server-persisted settings panel, and namespaced server routes. It is meant to be copied and renamed, not used as-is.

This file is the authoritative guide for development in this repo. `CLAUDE.md` covers template-vs-plugin context and points here instead of restating or correcting these rules. The normative plugin contract lives in the spec below.

## Contract sources

The contract has two sources, and they do not always agree:

| Source | Repository | Pinned revision | What it is |
|---|---|---|---|
| Specification (normative) | [`get-flashbacks/feedBack-plugin-spec`](https://github.com/get-flashbacks/feedBack-plugin-spec) — `spec/plugin-spec-v1.md`, `schemas/plugin.schema.json` | [`3d653d1`](https://github.com/get-flashbacks/feedBack-plugin-spec/tree/3d653d1ffedfeed7173f23359e48a964a80d84ac) | The MUST/SHOULD rules a plugin is validated against. |
| Host (observed) | [`get-flashbacks/feedBack`](https://github.com/get-flashbacks/feedBack) — `plugins/__init__.py`, `static/js/plugin-loader.js` | [`ff8984a`](https://github.com/get-flashbacks/feedBack/tree/ff8984ace37714be2eb00c61f1fe4c5b2bd4ecc3) | What the shipped app actually does. |

Both are org forks of the upstream `got-feedBack/*` repos. These revisions are the ones this template was audited against; bump them when you re-verify. To settle a contract question, fetch the named file at the pinned revision and read it rather than trusting prose — for example:

```bash
curl -s https://raw.githubusercontent.com/get-flashbacks/feedBack/ff8984ace37714be2eb00c61f1fe4c5b2bd4ecc3/plugins/__init__.py | grep -n has_screen
```

## Files

| File | Purpose |
Expand All @@ -29,16 +46,35 @@ A reference implementation demonstrating the fundamental feedBack plugin contrac

## Key conventions (feedBack plugin contract)

- **plugin.json id must match directory name** — `my-plugin` == `my-plugin/`
- **`id` is the plugin's identity.** It keys the nav entry, the settings store, the routes namespace, and the `plugin-<id>` screen container. Folder name and manifest `id` should match (see *Spec vs. shipped Host*).
- **Nav screen** uses `"nav": { "label": "My Plugin", "screen": "plugin-my-plugin" }`
- **Screen markup** is declared with a top-level `screen` key and lives in `screen.html`; the Host creates the `plugin-<id>` container from that key and injects the markup, then runs `script`. The current Host hardcodes the fetch path (`/api/plugins/<id>/screen.html`) and ignores the manifest value, so the file must be named `screen.html`.
- **A screen mounts only when the manifest declares a top-level `screen` key.** The Host creates the `plugin-<id>` container only when `bool(manifest.get("screen"))` (`has_screen`); `script` is injected independently, so a script-only manifest runs `screen.js` against a DOM where that container was never created. The current Host hardcodes the fetch to `/api/plugins/<id>/screen.html` and ignores the manifest value, so the file must be named `screen.html`.
- **screen.js finds its own root** via `document.getElementById('plugin-<id>')` — the Host injects markup into that container
- **Settings persist server-side**, not in browser storage — always go through the plugin's own routes
- **routes.py exports `setup(app, context)`** — receives the FastAPI app and a context dict with `config_dir` and `log`
- **Configuration validation happens once in `setup()`**, before any route is registered, so a broken config fails plugin load loudly rather than failing silently on first request
- **API path** is `/api/plugins/<plugin_id>/<route>`
- **No build step** — vanilla JS, no npm; the Python tests run under `pytest` (see `requirements-dev.txt`)

### Spec vs. shipped Host

The normative rules and the shipped Host do not always agree. When they differ, follow the Host for code that has to run today and the spec for anything that has to stay portable — and say which one you mean.

- **A folder/id mismatch is not discovery-fatal in the shipped Host.** Spec §5.2 says a directory is loaded "only when the directory name equals the manifest `id`", and §4.2 says a Host "MUST NOT treat it as bundled and SHOULD skip or warn". The shipped Host instead registers every subdirectory by its manifest `id` whatever the folder is called (`plugins/__init__.py`), and the folder name only feeds `_is_bundled()` for duplicate resolution. Match them anyway — the spec is stricter, and relying on the Host's leniency is not a documented contract.
- **The screen container is Host-gated.** Spec §6.1 describes mounting "for each ready plugin that declares a `screen`"; the Host implements exactly that (`has_screen == bool(manifest.get("screen"))` in `plugins/__init__.py`). The `screen` value itself is ignored — `plugin-loader.js` always fetches `/api/plugins/<id>/screen.html` — so the markup file must be named `screen.html`.

## Renaming this template into a real plugin

`my-plugin` is a placeholder id. Change every occurrence together, or the screen lookup and the `/api/plugins/<id>/...` routes will point at the old name:

1. Rename the directory to your new id: `git mv my-plugin your-plugin`.
2. `your-plugin/plugin.json` — `id` (must equal the folder name); also `name`, `version`, `description`, `type`, `icon`, and `nav.screen` (stays `plugin-<id>`).
3. `your-plugin/screen.js` — the `PLUGIN_ID` constant.
4. `your-plugin/settings.html` — the `PLUGIN_ID` constant.
5. `your-plugin/routes.py` — the `PLUGIN_ID` constant (it derives the route namespace).

Keep the id lowercase (`^[a-z0-9][a-z0-9_-]*$`). The id is permanent: changing it later orphans users' saved settings. The `tests/` suite discovers the plugin by scanning for `plugin.json` and reads the id from the manifest, so it keeps passing after a rename without edits.

## Known issues / code notes

- `screen.js` uses a `window[\`__${PLUGIN_ID}_setup\`]` guard to avoid double-initializing on re-hydration.
Expand All @@ -47,15 +83,22 @@ A reference implementation demonstrating the fundamental feedBack plugin contrac
- `routes.py` serializes the whole read–merge–write with an `asyncio.Lock` and persists via `_write_settings_atomically()` (temp file in the same directory + `os.replace`). The lock is per-process only — multiple workers sharing a `config_dir` would need a cross-process lock; `os.replace` still prevents torn files. The guarantee also assumes each handler runs to completion: cancelling the task mid-`asyncio.to_thread` releases the lock while the worker thread finishes anyway.
- The test fixture uses `with TestClient(app) as client:` on purpose: every request must share one event loop, otherwise a per-request loop rejects the `asyncio.Lock` bound by the first request.

## Verification checklist
## Verification and completion criteria

Automated (the same checks `custom-checks.yml` runs):
Exact commands, from the repo root (a venv keeps the test deps out of the system env):

```bash
python -m venv .venv && . .venv/bin/activate
pip install -r requirements-dev.txt
pytest -q
```

A change to this repo is complete when:

1. `pytest -q` passes.
2. The `custom-checks.yml` steps pass locally and in CI on the PR — manifest validation, `node --check my-plugin/screen.js`, and no `print(` in `routes.py`. `custom-checks.yml` carries the exact commands.
3. Every doc claim about the contract still holds against the pinned revisions in *Contract sources*.

Against a running Host with `my-plugin/` installed:

1. Plugin loads without server errors (`setup()` validates config before registering routes)
Expand All @@ -69,6 +112,7 @@ Against a running Host with `my-plugin/` installed:

## Common pitfalls

- **Folder name must equal `plugin.json` id**, including case
- **Folder name should equal `plugin.json` id**, including case — the spec requires it, even though the shipped Host tolerates a mismatch (see *Spec vs. shipped Host*)
- **Route collisions** — always prefix with `/api/plugins/<id>/`
- **A script-only manifest renders nothing** — declare a top-level `screen` key and name the file `screen.html` or the Host never creates the container
- **FastAPI POST routes** need `from fastapi import Request` and `async def route(request: Request)` reading the body via a bounded stream, not `await request.json()` directly, if you want the size cap to apply before full deserialization
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `tests/` and `requirements-dev.txt` — pytest coverage for the manifest/screen contract (declared files exist, the screen path the Host fetches, the nav container id) and the settings routes (GET defaults, valid-subset POST/persistence, invalid/unknown values, malformed JSON, and the 16 KiB body limit).
- `custom-checks.yml` now fails when the `screen` file or nav container reference is missing and runs `pytest`.
- `nav` field in `my-plugin/plugin.json` (`{ "label": "My Plugin", "screen": "plugin-my-plugin" }`) so the plugin registers a sidebar entry.
- `AGENTS.md` — reference guide for AI assistants/contributors covering the actual API shape, plugin conventions, known code notes, and a verification checklist.
- `CLAUDE.md` — agent guide covering the template-vs-plugin distinction, feedBack app context, and two corrections to `AGENTS.md` (the `screen`-key container gate, and the spec-vs-Host folder/id mismatch behavior).
- `AGENTS.md` — the authoritative development guide: the contract's spec and Host sources with the pinned revisions this template was verified against, the actual API shape, the plugin conventions with the spec-vs-Host differences called out, a copy/rename checklist, known code notes, and verification/completion criteria.
- `CLAUDE.md` — agent guide covering the template-vs-plugin distinction and feedBack app context; it defers to `AGENTS.md` rather than restating or correcting it.

### Changed

- Unified the docs around the actual plugin contract. `AGENTS.md` now folds in and owns the spec-vs-Host notes `CLAUDE.md` previously carried as corrections; the READMEs no longer claim a folder/id mismatch "will not load" (the spec requires them to match, but the shipped Host tolerates a mismatch), name the Host's real plugin-discovery directories, and state the settings store as `<config_dir>/<id>.json`. Spec links use the canonical `get-flashbacks/feedBack-plugin-spec` name. `.gitignore` now ignores the `.venv/` the documented setup creates.
- `my-plugin/plugin.json` `type` is now `"utility"` instead of `"visualization"`, matching what the example actually is; the `visualization` type is reserved for plugins that implement the `setRenderer` note-highway contract.

### Fixed
Expand Down
56 changes: 19 additions & 37 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,10 @@ so you can install it and watch it run before renaming. Treat every
identifier as a placeholder regardless:

- `my-plugin/plugin.json`'s `id: "my-plugin"` and the folder name
`my-plugin/` are meant to be changed together. The plugin-spec (see
below) states a folder/id mismatch is discovery-fatal, but the shipped
`get-flashbacks/feedBack` Host is more permissive: the loader registers
a plugin by its manifest `id` regardless of folder name, and a
mismatch only affects `bundled: true` duplicate-resolution behavior —
it isn't a silent skip there. Match them anyway; the spec is stricter,
and relying on the Host's leniency is not a documented contract.
`my-plugin/` are meant to be changed together. The spec requires them to
match; the shipped Host is more permissive. See `AGENTS.md` →
*Spec vs. shipped Host* for the exact behavior and *Renaming this template
into a real plugin* for the full checklist.
- Every route in `routes.py` is namespaced `/api/plugins/my-plugin/...` and
needs the same rename.
- The counter/settings behavior in `screen.js`/`settings.html` is a demo
Expand All @@ -35,6 +32,17 @@ abstract, without a rename already having happened, the most useful thing
is usually to ask what the real plugin is meant to be, since almost every
identifier in this tree is a placeholder.

## The authoritative development guide

`@AGENTS.md`

`AGENTS.md` is the single source of truth for this repo: the file table, the
demo's actual API shape, the feedBack plugin-contract conventions, the
spec-vs-Host differences (including the folder/id and screen-gate rules), the
copy/rename checklist, the verification and completion criteria, and common
pitfalls. This file does not restate or correct it — read `AGENTS.md` before
acting on anything in this repo.

## What feedBack is

feedBack is a self-hosted web app for browsing, playing, and practicing
Expand All @@ -53,12 +61,13 @@ files loaded at runtime.
- **Canonical app repo:** `got-feedBack/feedBack`. **This org's fork:**
`get-flashbacks/feedBack` (personal fork with fork-specific features).
- **The authoritative plugin contract** lives in
[`get-flashbacks/feedback-plugin-spec`](https://github.com/get-flashbacks/feedback-plugin-spec)
[`get-flashbacks/feedBack-plugin-spec`](https://github.com/get-flashbacks/feedBack-plugin-spec)
— this org's own normative fork of the upstream
[`got-feedBack/feedback-plugin-spec`](https://github.com/got-feedBack/feedback-plugin-spec).
[`got-feedBack/feedBack-plugin-spec`](https://github.com/got-feedBack/feedBack-plugin-spec).
This repo's own README already states the spec takes precedence over
anything here if the two conflict. Read the spec before trusting this
template's conventions for anything non-obvious.
template's conventions for anything non-obvious. `AGENTS.md` pins the
spec and Host revisions this template was audited against.
- feedBack's own `CLAUDE.md` (in the core repo) documents the full plugin
contract in far more depth than this template attempts to demonstrate:
the `setRenderer` visualization lifecycle, capability domains
Expand All @@ -68,30 +77,3 @@ files loaded at runtime.
detachable panes, and the v3 player-chrome contract. This template only
demonstrates the smallest possible slice (screen + settings + routes) —
don't assume it's a complete reference for anything beyond that slice.

## File map, API shape, conventions, checklist, pitfalls

@AGENTS.md

That file has the file table, the demo's actual API shape, the feedBack
plugin-contract conventions, known code notes, the verification
checklist, and common pitfalls — this file doesn't repeat that content.
Two corrections to it, both load-bearing enough to call out here rather
than silently fix in place:

- **The container-mount convention** ("screen.js finds its own root via
`document.getElementById('plugin-<id>')`") is gated on the manifest
declaring a top-level `screen` key (the Host's `has_screen` gate —
`plugins/__init__.py`'s `"has_screen": bool(manifest.get("screen"))`,
consumed by `static/js/plugin-loader.js`'s container-creation site).
`script` is injected independently of `has_screen`, so a script-only
manifest runs `screen.js` against a DOM where that container was never
created. This template now declares `"screen": "screen.html"` and ships
the file, so `plugin-my-plugin` is created and the example mounts. Note
the current Host hardcodes the fetch path — it always requests
`/api/plugins/<id>/screen.html` and ignores the manifest value — so the
markup file has to be named `screen.html`.
- **The folder/id mismatch consequence** ("a mismatch is a silent skip at
plugin discovery") is spec language, not what the shipped
`get-flashbacks/feedBack` Host actually does — see the note in "This
repo is a template, not a plugin" above.
Loading
Loading