diff --git a/.feedpak-spec-ref b/.feedpak-spec-ref deleted file mode 100644 index 4ee7faf..0000000 --- a/.feedpak-spec-ref +++ /dev/null @@ -1,9 +0,0 @@ -# Pinned commit of github.com/got-feedback/feedpak-spec that this repo is -# verified against by the `feedpak-spec` CI job (tools/check_spec_conformance.py). -# -# Pinned by SHA rather than tracking the spec's default branch on purpose: a -# change over there must never turn CI red on an unrelated PR here. When the -# spec moves, bump this SHA in its own PR — if that PR is red, the spec changed -# in a way core does not satisfy, which is exactly the signal we want, delivered -# as a reviewable PR instead of a surprise. -15e13e02062842d6660897623b41e04fd99ece58 diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 1ceb6ac..2cbac7e 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,17 +1,17 @@ -## What - - - -## feedpak surface - - - -- [ ] This PR does **not** change how the app reads/writes feedpaks (manifest keys, pack files, folder layout) -- [ ] …or it does, and the spec change landed first via the [FEP process](https://github.com/got-feedback/feedpak-spec/blob/main/CONTRIBUTING.md) — FEP / spec PR: `got-feedback/feedpak-spec#___` — and `.feedpak-spec-ref` is bumped to the merged SHA in this PR - -## Checklist - -- [ ] `CHANGELOG.md` `[Unreleased]` updated (user-visible changes) -- [ ] Tests added/updated for new behaviour -- [ ] Commits are DCO signed off (`git commit -s`) +## What + + + +## feedpak surface + + + +- [ ] This PR does **not** change how the app reads/writes feedpaks (manifest keys, pack files, folder layout) +- [ ] …or it does, and the spec change landed first via the [FEP process](https://github.com/got-feedback/feedpak-spec/blob/main/CONTRIBUTING.md) — FEP / spec PR: `got-feedback/feedpak-spec#___` (once it merges, re-run this PR's checks and the gate goes green) + +## Checklist + +- [ ] `CHANGELOG.md` `[Unreleased]` updated (user-visible changes) +- [ ] Tests added/updated for new behaviour +- [ ] Commits are DCO signed off (`git commit -s`) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7c1ab42..0d722ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -133,8 +133,16 @@ jobs: # the only way a new key lands; core ingests the spec's example packs; packs # committed here pass the spec's reference validator. Motivated by # #933, where a manifest key (`original_audio`) shipped in core without ever - # reaching the spec. Pinned by SHA in .feedpak-spec-ref so a change over - # there can't redden an unrelated PR here. + # reaching the spec. + # + # The gate checks against the spec repo's HEAD, deliberately: the app must + # conform to the LIVING spec, always. The dev flow is self-serve — a gated + # PR opens a FEP, the spec PR merges, re-running this job goes green; no + # pin file to bump, nothing to maintain. Accepted trade-off: a BREAKING + # spec change (rare, deliberate, MAJOR per the spec's compatibility policy) + # reddens every PR here until core conforms — which is the correct + # org-wide signal that the app is out of conformance. The normal FEP is + # additive and can never redden this job. name: feedpak-spec runs-on: ubuntu-latest steps: @@ -152,31 +160,19 @@ jobs: python-version: '3.12' cache: 'pip' - - name: Read the pinned spec commit - id: spec - run: | - sha=$(grep -vE '^[[:space:]]*(#|$)' .feedpak-spec-ref | head -n1 | tr -d '[:space:]') - if [ -z "$sha" ]; then - echo "::error file=.feedpak-spec-ref::no commit SHA found in .feedpak-spec-ref" - exit 1 - fi - # actions/checkout resolves branches and tags in `ref` too, so a - # non-SHA here would silently un-pin the spec — the one thing this - # file exists to prevent. Demand a full 40-char SHA. - if ! printf '%s' "$sha" | grep -qE '^[0-9a-fA-F]{40}$'; then - echo "::error file=.feedpak-spec-ref::expected a full 40-character commit SHA, got '$sha' — a branch or tag name would defeat the pin" - exit 1 - fi - echo "sha=$sha" >> "$GITHUB_OUTPUT" - - - name: Check out feedpak-spec at the pinned commit + - name: Check out feedpak-spec at HEAD uses: actions/checkout@v4 with: repository: got-feedback/feedpak-spec - ref: ${{ steps.spec.outputs.sha }} + ref: main path: .feedpak-spec persist-credentials: false + - name: Record the spec commit this run verified against + # HEAD-tracking means CI results can differ across time on the same + # commit. Log the exact spec SHA so a red run is reproducible. + run: git -C .feedpak-spec rev-parse HEAD + - name: Install dependencies run: | python -m pip install --upgrade pip diff --git a/CHANGELOG.md b/CHANGELOG.md index 636a428..e863a8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,11 +20,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 list falls behind the codebase); (2) **allowlist-closed** — `feedpak-spec-exceptions.yml` never grows; (3) **forward** — core's `load_song()` ingests every example pack the spec ships; (4) **reverse** — every pack committed here passes the spec's own `tools/validate.py` (7/7 pass today). - The spec is pinned by SHA in `.feedpak-spec-ref` so a change over there can't redden an unrelated PR - here; bump it in its own PR, and a red result is the signal that core doesn't satisfy the new spec. + The gate verifies against the spec repo's **HEAD** — the app must conform to the living spec, and the + flow is self-serve: a gated PR opens a FEP, the spec PR merges, re-running checks goes green. Nothing to + pin, nothing to bump. Each run logs the spec SHA it verified against so results are reproducible. **There is no in-repo escape hatch, by design.** A blocked PR has exactly one route: land the key in the spec via the [FEP process](https://github.com/got-feedback/feedpak-spec/blob/main/CONTRIBUTING.md), then - bump `.feedpak-spec-ref` to the merged SHA in the same PR. `feedpak-spec-exceptions.yml` is a **closed + re-run the PR's checks — the gate verifies against the spec's HEAD, so it goes green once the key is real. `feedpak-spec-exceptions.yml` is a **closed grandfather list** for keys that predate the gate, not a bypass: a fourth check (**allowlist-closed**) diffs it against the base branch and fails any PR that *adds* an entry, so it may only shrink. `original_audio` is grandfathered there against #933 so the gate lands green and starts blocking the diff --git a/CLAUDE.md b/CLAUDE.md index c8486e3..b6332ef 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,702 +1,703 @@ -# FeedBack — AI Agent Guide - -FeedBack is a self-hosted web app for browsing, playing, and practicing interactive music notation, built around its own open `.sloppak` chart format. Charts come from importing Guitar Pro (GP5/GP8) or MusicXML, or from authoring in the built-in editor. It runs as a Docker container with a FastAPI backend (`server.py`), vanilla JavaScript frontend (`static/`), shared Python libraries (`lib/`), and an extensive plugin system (`plugins/`). There are no frontend frameworks — everything is plain JS, HTML, and Tailwind CSS. - -## Architecture Quick Reference - -``` -server.py FastAPI app — library API, WebSocket highway, plugin loading -static/ - app.js Main frontend — screens, library views, player, plugin loader - highway.js Canvas note highway renderer (createHighway factory) - index.html Single-page app shell - style.css Custom CSS loaded alongside Tailwind -lib/ - song.py Core data models (Note, Chord, Arrangement, Song) - sloppak.py Sloppak format support - loosefolder.py Loose-folder XML chart support - audio.py OGG/MP3 audio handling - retune.py Pitch-shifting logic - tunings.py Tuning name/offset utilities - gp2rs.py Guitar Pro to arrangement XML conversion - gp2midi.py Guitar Pro to MIDI -plugins/ - __init__.py Plugin discovery, loading, requirements install - / Each plugin is its own directory (often a git submodule) -tests/ - test_*.py pytest test suite -``` - -## Plugin System - -Plugins are the primary extension point. Each plugin lives in `plugins//` with a `plugin.json` manifest. Plugins are typically their own git repositories — see [CONTRIBUTING.md](CONTRIBUTING.md) for the licensing policy (curated plugins should be AGPL-3.0 or AGPL-compatible: MIT, BSD, Apache-2.0). - -```json -{ - "id": "my_plugin", - "name": "My Plugin", - "version": "1.0.0", - "private": false, - "type": "visualization", - "nav": { "label": "My Plugin", "screen": "plugin-my_plugin" }, - "screen": "screen.html", - "script": "screen.js", - "styles": "assets/plugin.css", - "routes": "routes.py", - "settings": { - "html": "settings.html", - "server_files": ["my_plugin.db", "my_plugin_models/"] - }, - "diagnostics": { - "server_files": ["my_plugin.diag.json"], - "callable": "diagnostics:collect" - } -} -``` - -All fields except `id` and `name` are optional. Plugins can have any combination of frontend (screen/script), backend (routes), and settings. - -`version` and `private` are advisory metadata — the plugin loader does not currently consume them, but plugins commonly include them for publishing/tooling purposes. - -`description`, `category`, and `icon` are **optional, additive v3 Pedalboard metadata** (surfaced in `/api/plugins`, consumed by the v3 Plugins page `static/v3/plugins-page.js`). `description` is a short one-sentence summary shown under the pedal name. `category` (`audio | creation | practice | game | tools`, free-form; unknown/absent → curated default → `"other"`) picks which pedalboard the plugin sits on. `icon` is an assets-relative thumbnail path (e.g. `"assets/thumb.png"`, ~square ~256×256, same containment rule as `styles`, served via `/api/plugins//assets/...`); if omitted the loader auto-detects `assets/thumb.png`, and plugins with no thumbnail get a default pedal graphic. All three are backward-compatible — omit them and the plugin still loads. See [docs/plugin-v3-ui.md](docs/plugin-v3-ui.md). - -`styles` is the **opt-in** for self-hosted CSS (Principle II — prebuilt Tailwind, no Play CDN). Core's `static/tailwind.min.css` only contains classes scanned from core source at build time, so a plugin installed at runtime (community / NAS) that uses classes core didn't scan — especially arbitrary values like `text-[11px]` — renders unstyled. Declaring `styles` makes the frontend inject one versioned `` into `` (covering the plugin's screen *and* its settings panel) pointing at the plugin's own compiled stylesheet. The value is a **plugin-root-relative path that must live under `assets/`** (e.g. `"assets/plugin.css"`) so it serves through the sandboxed `/api/plugins//assets/...` route. Build it with `corePlugins: { preflight: false }` (utilities only — core ships the single base reset; don't duplicate it) and **never** the Tailwind Play CDN. Plugins that use only core-guaranteed utilities, or ship no Tailwind, omit `styles` and are byte-for-byte unaffected. Full authoring guide + scaffold: [docs/plugin-styles.md](docs/plugin-styles.md). - -`settings.server_files` is the **opt-in** for the unified Settings export/import flow (feedBack#113). It's a list of relpaths under `context["config_dir"]` that the plugin wants included in user-triggered backups. A trailing `/` denotes a directory (recurse). Plugins that omit this field have no server-side files exported; their state lives entirely in browser `localStorage`, which is bundled wholesale on every export. Rules: -- Relpaths only. Absolute paths, drive letters, `..` segments, and backslashes are rejected at load time with a `[Plugin]` warning. -- The same allowlist is consulted at both export and import: a bundle that references a file the *importing host*'s manifest no longer declares is skipped with a warning (handles plugin updates between export and import). A bundle that references a file your host's manifest never declared is also skipped — no surprise writes. -- Files are encoded as `{"encoding": "json", "data": }` for `.json` files that parse cleanly (diff-friendly), `{"encoding": "base64", "data": "..."}` otherwise (sqlite, model blobs, IRs). -- Plugins own their internal data migration. Importing a bundle whose data schema predates your current code restores bytes verbatim — your plugin must cope at next load. -- Symlinks are skipped on export and never followed on import. - -`diagnostics` is the **opt-in** for the troubleshooting bundle (feedBack#166 — Settings → Export Diagnostics). Two independent fields: -- `diagnostics.server_files` — same allowlist semantics as `settings.server_files`: relpaths under `context["config_dir"]`, no `..`, no abs paths, no backslashes, no leading dots. Files listed here are copied verbatim into `plugins//` inside the bundle. Use this for snapshot-style state (small DB excerpts, model lists, last-error files). -- `diagnostics.callable` — `":"` (e.g. `"diagnostics:collect"`). Resolved lazily via `load_sibling` when the user clicks Export, then called as `func({"plugin_id": "...", "config_dir": Path(...)})`. Return `dict`/`list` → written to `plugins//callable.json`; `bytes` → `callable.bin`; `str` → `callable.txt`. Exceptions are caught and appended to the bundle's `manifest.notes` — a buggy plugin never crashes the export. - -Plugins that omit the field contribute nothing to the bundle from the backend side. Frontend plugins can independently push state via `window.feedBack.diagnostics.contribute(plugin_id, payload)` from their `screen.js` before the user hits Export. Bundle layout + per-file schemas: [docs/diagnostics-bundle-spec.md](docs/diagnostics-bundle-spec.md). - -Best practices: -- Embed your own `schema` field (e.g. `"my_plugin.diag.v1"`) in JSON returned by `callable` so future tooling can dispatch by version. -- Keep payloads small (< 100 KB). Diagnostics are not a backup channel — that's `settings.server_files`. -- Don't include user secrets, API keys, or session tokens. The bundle is shared with maintainers / posted to GitHub issues. - -`type` is an optional role hint (feedBack#36). Supported values: -- `"visualization"` — plugin provides a highway renderer. Declaring this makes the plugin eligible for the main-player viz picker AND splitscreen's per-panel picker. Must pair with a `window.feedBackViz_` factory exporting the setRenderer contract below. -- Absent → no declared role; plugin is loaded and its script runs, but it doesn't appear in role-specific UIs. - -**Backend routes** — `routes.py` must export a `setup(app, context)` function. The `context` dict provides: -- `config_dir` — persistent config path -- `get_dlc_dir()` — returns the DLC folder Path -- `extract_meta()` — metadata extraction callable -- `meta_db` — shared MetadataDB instance -- `library_providers` — shared library provider registry for source-aware browsing -- `register_library_provider(provider)` — register a plugin-provided library source. Providers expose `id`, `label`, optional `kind`/`capabilities`, and callable `query_page`, `query_artists`, `query_stats`, and `tuning_names` methods. Providers with `art.read` may also expose `get_art(song_id)` returning one of: a `Response` object (any media type, served as-is); raw `bytes` or `bytearray` (**assumed PNG** — use a `Response` or a `dict` with `content`+`media_type` keys for JPEG/WebP or other formats); a URL string (http/https → 302 redirect; other schemes are rejected with 400); a filesystem path string or `Path` (served as a file with auto-detected media type); or a `dict` with a `url`, `path`, or `content` key. Providers with `song.sync` may expose `sync_song(song_id)` returning `None` (success with no local file) or a `dict` — the dict is passed through as the JSON response and should include `filename`/`local_filename` if a local playable file was produced. -- `unregister_library_provider(provider_id)` — remove a plugin-provided library source by id. The built-in `local` provider cannot be removed. -- `get_sloppak_cache_dir()` — sloppak cache path -- `load_sibling(name)` — loads a sibling module from this plugin's directory under a unique, namespaced module name. See "Sibling imports" below. -- `log` — stdlib `logging.Logger` namespaced to `feedBack.plugin.`. Pre-configured with the app-wide level, format (including JSON mode), and correlation IDs. Use this for all backend plugin output instead of `print()`. See "Backend plugin logging" below. - -**Sibling imports — use `load_sibling`, not bare imports** (feedBack#33). The plugin loader inserts each plugin's directory onto `sys.path` so `from extractor import X` works, but Python caches imports by **module name** in `sys.modules`. Two plugins that each ship a top-level `extractor.py` (or any other generic name — `util.py`, `client.py`, `parser.py`, `config.py`, …) collide: whichever loads first wins, and the other plugin's `from extractor import X` either gets the wrong module or fails with `cannot import name 'X' from 'extractor'`. - -The fix is `context["load_sibling"](name)`, which loads the sibling under a namespaced module name (`plugin_.`, where plugin_id is bijectively encoded so reverse-DNS-style ids like `com.example.foo` work without colliding: `_` -> `_5f_`, `.` -> `_2e_`) so each plugin gets its own copy: - -```python -def setup(app, context): - helper = context["load_sibling"]("helper") - HelperClass = helper.HelperClass - # … -``` - -Notes: -- `name` is a bare module name — no `.py` suffix, no slashes, no `.`. The helper raises `ValueError` for path traversal / format issues and `ImportError` for missing files. -- Both single-file siblings (`extractor.py`) and package-form siblings (`extractor/__init__.py`) work. Package form wins when both exist (matches CPython's import-resolution precedence). -- Relative imports between siblings work — `from .shared import X` in a top-level helper, `from ..shared import X` from inside a sibling package. The synthetic parent package `plugin_` carries the plugin directory in its `__path__`. -- `from . import sibling` (attribute-style) also resolves: loaded children are exposed as attributes on the parent package. -- Repeat calls return the cached module. Concurrent first-time calls are serialized via per-module locks so no caller observes a half-initialized module. -- Bare `import sibling` from `routes.py` still works during the transition period, but the loader prints a startup warning when it detects two plugins shipping a same-named top-level module — covering both `.py` files and package directories. Migrate to `load_sibling` to silence the warning and immunize your plugin from future ecosystem collisions. (Don't mix bare imports and `load_sibling` for the same module — they'd execute the file twice and split module-level state.) - -**Frontend scripts** — `screen.js` runs in the global scope via a `