mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-09-12 17:28:32 +00:00
Compare commits
19
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a7c20a973c | ||
|
|
ba796b0f27 | ||
|
|
ddc06ff1e7 | ||
|
|
ac5c5ad20d | ||
|
|
99b974a5a1 | ||
|
|
a60dcd10c2 | ||
|
|
5dcf39cd62 | ||
|
|
c485f02211 | ||
|
|
203f82b6fe | ||
|
|
0158286d06 | ||
|
|
1e2ce29cf6 | ||
|
|
b54b65d35c | ||
|
|
ab2e68a638 | ||
|
|
d806d12c22 | ||
|
|
32d723b774 | ||
|
|
ceb1e143cd | ||
|
|
d0626f5618 | ||
|
|
0dc9fd7ba8 | ||
|
|
22332bef22 |
@@ -0,0 +1,17 @@
|
||||
## What
|
||||
|
||||
<!-- What does this PR do, and why? Link the issue it addresses. -->
|
||||
|
||||
## feedpak surface
|
||||
|
||||
<!-- The feedpak spec is sacrosanct: the spec defines the format, this app implements it.
|
||||
Delete this section ONLY if your change doesn't touch how the app reads or writes packs. -->
|
||||
|
||||
- [ ] 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`)
|
||||
@@ -124,6 +124,94 @@ jobs:
|
||||
print(f"Validated {len(manifests)} manifest(s) — OK")
|
||||
EOF
|
||||
|
||||
feedpak-spec:
|
||||
# Guard that core stays faithful to the feedpak format spec, which lives in
|
||||
# its own repo (got-feedback/feedpak-spec) and is the contract third-party
|
||||
# packers and players build against. Four surface checks: core reads/writes
|
||||
# only manifest keys the spec declares (and the scanned-module list can't
|
||||
# fall behind); the exception allowlist never grows, so the FEP process is
|
||||
# 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.
|
||||
#
|
||||
# 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:
|
||||
- uses: actions/checkout@v4
|
||||
# This job runs repository code (tools/check_spec_conformance.py) and
|
||||
# never pushes; don't leave the token in git config for it.
|
||||
# fetch-depth: 0 so the base branch is available — the gate must prove
|
||||
# the exception allowlist didn't grow in this PR.
|
||||
with:
|
||||
persist-credentials: false
|
||||
fetch-depth: 0
|
||||
|
||||
- uses: actions/setup-python@v5
|
||||
with:
|
||||
python-version: '3.12'
|
||||
cache: 'pip'
|
||||
|
||||
- name: Check out feedpak-spec at HEAD
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
repository: got-feedback/feedpak-spec
|
||||
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
|
||||
pip install -r requirements.txt
|
||||
# CI-only: the spec's reference validator needs jsonschema. Not a
|
||||
# runtime dependency — this gate never runs on the serve/Docker path
|
||||
# (constitution Principle I). Pinned for the same reason the spec SHA
|
||||
# is: an upstream release must not turn this job red on a PR that
|
||||
# changed neither this repo nor the spec.
|
||||
pip install 'jsonschema==4.26.0'
|
||||
|
||||
- name: Fetch the base branch's exception allowlist
|
||||
id: baseline
|
||||
run: |
|
||||
# The allowlist is closed: it grandfathers keys that predate this gate
|
||||
# and may only shrink. Prove that by diffing against the base branch —
|
||||
# without this, anyone could append an entry and route around the FEP
|
||||
# process from inside this repo.
|
||||
#
|
||||
# Resolve the base rather than hardcoding `main`: ship-ci.yml also runs
|
||||
# this workflow for PRs into release/** and for pushes to release/**,
|
||||
# where a main baseline would diff against the wrong branch.
|
||||
# PR -> the branch it merges into
|
||||
# push -> the branch itself (its tip already contains the change, so
|
||||
# this is a no-op; enforcement happens at PR time)
|
||||
BASE="${{ github.event.pull_request.base.ref || github.ref_name }}"
|
||||
echo "diffing the allowlist against origin/$BASE"
|
||||
git fetch --no-tags --depth=1 origin "$BASE"
|
||||
if git cat-file -e FETCH_HEAD:feedpak-spec-exceptions.yml 2>/dev/null; then
|
||||
git show FETCH_HEAD:feedpak-spec-exceptions.yml > "$RUNNER_TEMP/baseline-exceptions.yml"
|
||||
echo "args=--baseline-exceptions $RUNNER_TEMP/baseline-exceptions.yml" >> "$GITHUB_OUTPUT"
|
||||
else
|
||||
# Only true until the PR that introduces this gate lands.
|
||||
echo "args=--bootstrap-allowlist" >> "$GITHUB_OUTPUT"
|
||||
fi
|
||||
|
||||
- name: Check feedpak spec conformance
|
||||
run: python tools/check_spec_conformance.py --spec .feedpak-spec ${{ steps.baseline.outputs.args }}
|
||||
|
||||
lint:
|
||||
# Maintainer/CI-only size + module-hygiene gate (constitution Principle I:
|
||||
# dev tooling, never on the serve/Docker path — same category as
|
||||
|
||||
+35
-2
@@ -8,13 +8,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
- **Badge ceremony in the venue** — earning a genre badge now stages a moment:
|
||||
the crowd layer erupts (new public `v3VenueCrowd.celebrate()` — instant
|
||||
ecstatic loop bypassing the stability/dwell hysteresis, plus a cheer stinger;
|
||||
a no-op without a venue pack) and a full-screen overlay drops the bronze
|
||||
stamp with a shine sweep and a confetti burst over whatever screen is active
|
||||
(badges land right after `stats:recorded`, while the player is still up).
|
||||
Click or wait ~4s to dismiss; `prefers-reduced-motion` gets the existing
|
||||
chime + notification only. The stamp still slams into the passport book on
|
||||
next open, unchanged.
|
||||
- **Career passports (backend)** — the badge-journey layer on top of career stars.
|
||||
New career-plugin endpoints: `GET /api/plugins/career/passports` (per-instrument
|
||||
passport walls: genre badges computed on read from `song_stats` × the library's
|
||||
effective genre — Bronze = N genre songs at K★, data-driven in
|
||||
`plugins/career/passports.json`, default 5 songs at 2★ — plus qualifying-song
|
||||
"ticket stubs", the library genre list, and drill status), `POST
|
||||
/passports/commit` (instrument commitment), `POST /passports/open` (open a genre
|
||||
"ticket stubs", the library genre list, and drill status), `POST /passports/commit`
|
||||
(instrument commitment), `POST /passports/open` (open a genre
|
||||
passport), and `POST /drill-state` (intake for the relayed Virtuoso
|
||||
`virtuoso.progress` snapshot, so drill requirements can gate badges
|
||||
server-side). Badges are never stored; the only persisted state (commitments,
|
||||
@@ -35,6 +44,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
localStorage snapshot to the drill-state intake on `virtuoso:progress` bus
|
||||
events (debounced, plus a one-time bootstrap), closing the
|
||||
fires-into-a-void seam without touching the virtuoso plugin.
|
||||
- **CI gate: core must stay faithful to the feedpak spec (`feedpak-spec` job).** feedpak is published as
|
||||
an open format with its own repo, normative spec, JSON Schemas, and reference validator — but nothing
|
||||
stopped core from reading a manifest key the spec never defined, which is exactly what happened with
|
||||
`original_audio` (#583 → #933). `tools/check_spec_conformance.py` now enforces four surface properties
|
||||
in CI: (1) **key-coverage** — every manifest key core reads *or writes* is declared in the spec's
|
||||
`manifest.schema.json`, found by walking the AST of `lib/sloppak.py`, `lib/enrichment.py`, and
|
||||
`lib/songmeta.py`, `lib/gp2notation.py`, and `lib/routers/ws_highway.py` (writes are gated too — including
|
||||
`setdefault()` — and reported separately: a key core writes lands in every pack we emit, so an undeclared
|
||||
one seeds the ecosystem with non-spec data; a **readers-complete** guard fails the build if that module
|
||||
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 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
|
||||
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
|
||||
*next* instance immediately; the gate takes no position on how #933 resolves (the expected outcome is
|
||||
removing the key, since the spec already carries the mixdown as a stem — not adopting it). Docs:
|
||||
[docs/feedpak-spec-gate.md](docs/feedpak-spec-gate.md).
|
||||
|
||||
### Removed
|
||||
- **The classic v2 UI shell is gone — v3 is the only UI (R3a).** `static/index.html`, the
|
||||
|
||||
@@ -588,6 +588,21 @@ tab, key/scale annotations, etc.). Published as **feedpak**; this codebase still
|
||||
**sloppak** name internally — same on-disk format. [docs/sloppak-spec.md](docs/sloppak-spec.md) is
|
||||
a local pointer + code map.
|
||||
|
||||
**The spec is sacrosanct — read it BEFORE changing how this app reads or writes packs.** The
|
||||
spec repo defines the format; this app merely implements it ("a change is not part of the format
|
||||
until it lands here" — feedpak-spec/GOVERNANCE.md). Any new manifest key, file, or directory the
|
||||
app touches must land in the spec **first**, via the
|
||||
[FEP process](https://github.com/got-feedback/feedpak-spec/blob/main/CONTRIBUTING.md) (proposal
|
||||
issue → one spec PR updating spec + schemas + example + changelog → then re-run your PR's checks
|
||||
here; the gate verifies against the spec's HEAD, so it goes green the moment your key is real).
|
||||
CI enforces this: the `feedpak-spec` job
|
||||
([docs/feedpak-spec-gate.md](docs/feedpak-spec-gate.md)) fails any PR whose code touches a
|
||||
manifest key the spec doesn't declare, and there is **no in-repo bypass** — the exceptions
|
||||
file is a closed grandfather list that only shrinks. If the format seems to be missing something
|
||||
you need, that's a FEP conversation, not a workaround. (Cautionary tale: `original_audio`, #933 —
|
||||
shipped without a spec entry, and third-party packers reverse-engineered a folder convention out
|
||||
of a code comment.)
|
||||
|
||||
**Key code:**
|
||||
- `lib/sloppak.py` — format detection, zip/directory resolution, metadata extraction, song loading
|
||||
- `lib/sloppak_convert.py` — sloppak assembly pipeline, Demucs stem splitting
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
# The feedpak spec-conformance gate
|
||||
|
||||
`tools/check_spec_conformance.py`, run in CI as the `feedpak-spec` job.
|
||||
|
||||
## Why
|
||||
|
||||
feedpak is published as an **open format**: its own repo
|
||||
([got-feedback/feedpak-spec](https://github.com/got-feedback/feedpak-spec)), a normative spec, JSON
|
||||
Schemas, and a reference validator. That is a promise to everyone outside this codebase — third-party
|
||||
packers, converters, and players build against the spec, and the spec is meant to be the complete and
|
||||
authoritative description of a pack.
|
||||
|
||||
The moment core reads a manifest key the spec doesn't define, that promise breaks silently:
|
||||
|
||||
- A spec-compliant pack is no longer guaranteed to be a fully-working pack.
|
||||
- The reference validator can't warn authors about a key it has never heard of — it will happily green-light
|
||||
the key, and every misspelling of it.
|
||||
- The format's real definition drifts into our source tree. In the case that motivated this gate
|
||||
([#933](https://github.com/got-feedback/feedback/issues/933)), third-party tooling started emitting an
|
||||
`original/` directory that no code anywhere requires — the convention was reverse-engineered from an
|
||||
example in a *code comment*.
|
||||
|
||||
The rule this gate enforces: **any manifest key core reads _or writes_ must be in the spec before core
|
||||
ships code that depends on it.** Spec first, implementation second. Writes are not exempt — a key core
|
||||
writes lands in every pack we emit, so an undeclared one seeds the ecosystem with non-spec data.
|
||||
|
||||
Note that "get it into the spec" is not automatically the right fix for an existing violation — for
|
||||
`original_audio` it isn't. The spec already carries the pre-separation mixdown as a stem
|
||||
(`{id: full, file: stems/full.ogg}`), so that key added a *second, redundant* location for audio to a format
|
||||
that already had one, and the resolution is to remove it rather than bless it. The gate takes no position on
|
||||
which way a violation resolves; it only insists that one of the two happens deliberately, in the open,
|
||||
before the code merges.
|
||||
|
||||
## What it checks
|
||||
|
||||
We can't mechanically prove core *interprets* a key the way the spec means. We can prove four surface
|
||||
properties, and they cover the drift that actually occurs.
|
||||
|
||||
| Layer | Check | Catches |
|
||||
|---|---|---|
|
||||
| 1. key-coverage | Every manifest key core reads **or writes** is declared in the spec's `manifest.schema.json`. | Core growing a key the spec never defined — the #933 class. |
|
||||
| 2. allowlist-closed | `feedpak-spec-exceptions.yml` has not **grown** relative to the base branch. | Someone routing around the FEP process by allowlisting their own new key. |
|
||||
| 3. forward | Core's `load_song()` ingests every example pack the spec ships. | The spec adding or tightening something core ignores or breaks on. |
|
||||
| 4. reverse | Every pack committed to this repo passes the spec's `tools/validate.py`. | Core (or a contributor) committing a pack the spec would reject. |
|
||||
|
||||
Layer 1 works by walking the AST of the modules listed in `READERS` and collecting every literal key touched
|
||||
on a manifest dict (`manifest.get("x")`, `manifest["x"]`, and the wrapped
|
||||
`(load_manifest(p) or {}).get("x")` form used in `lib/enrichment.py`).
|
||||
|
||||
**Reads and writes are both checked, and reported differently.** A key core *writes*
|
||||
(`manifest["x"] = v`, as `lib/songmeta.py` does) is spec surface pointed outward: it puts a key into every
|
||||
pack we emit, so an undeclared one seeds the ecosystem with non-spec data. Subscripts are classified by AST
|
||||
context — `Store` is a write, `Load` is a read — so `manifest["year"] = ...` is not miscounted as a read.
|
||||
|
||||
## When it fails
|
||||
|
||||
You added a manifest key the spec doesn't define. **There is exactly one way forward, and it is not in this
|
||||
repo.**
|
||||
|
||||
Land the key in the spec through the **feedpak Enhancement Proposal (FEP)** process
|
||||
([feedpak-spec/CONTRIBUTING.md](https://github.com/got-feedback/feedpak-spec/blob/main/CONTRIBUTING.md)):
|
||||
|
||||
1. **Open a FEP issue** on `got-feedback/feedpak-spec` — the problem, the proposed on-disk shape (manifest
|
||||
key and/or side-file), backward compatibility, and the version bump it implies.
|
||||
2. **Discuss**, until it has a clear shape and rough consensus.
|
||||
3. **Land one PR there** that updates the normative spec (`spec/feedpak-v1.md`), the relevant JSON
|
||||
Schema(s), an example in `examples/` that exercises it, and the changelog — *together*. A PR touching
|
||||
only one of those is incomplete.
|
||||
4. **Back here**, just re-run your PR's checks. The gate verifies against the spec's HEAD, so the moment
|
||||
your key is genuinely part of the format, your PR goes green — nothing to bump, nothing to maintain.
|
||||
|
||||
That's deliberately the only route — no experimental prefix, no self-serve allowlist — and it's usually a
|
||||
quick one for additive keys. The reason it's worth the round-trip: the gate checks the whole repo against
|
||||
the living spec, so if non-conformance ever lands, it shows up as red CI on *every* teammate's open PR, and
|
||||
only the person who introduced it can clear it. Going through the FEP keeps your change clean and keeps
|
||||
everyone else unblocked.
|
||||
|
||||
The spec's own governance says the same thing:
|
||||
|
||||
> This repository defines the format only. Applications that read or write feedpak ... track this spec as a
|
||||
> dependency; they do not drive it. **A change is not part of the format until it lands here.**
|
||||
> — [feedpak-spec/GOVERNANCE.md](https://github.com/got-feedback/feedpak-spec/blob/main/GOVERNANCE.md)
|
||||
|
||||
### `feedpak-spec-exceptions.yml` is a closed grandfather list, not a hatch
|
||||
|
||||
It exists solely because `original_audio` predates the gate. **CI fails any PR that adds an entry** (layer 2
|
||||
diffs it against the base branch), so the list can only ever shrink. Entries are debt, each carries a
|
||||
tracking issue, and each disappears when the underlying key is removed from core. The gate also fails on a
|
||||
*stale* entry — the spec caught up, or core stopped touching the key — so the file cannot quietly become
|
||||
somewhere drift accumulates.
|
||||
|
||||
Deleting an entry does not, by itself, get you past the gate: layer 1 still fails while core reads the key.
|
||||
The entry goes when the **code** goes.
|
||||
|
||||
## Tracking the spec's HEAD
|
||||
|
||||
The gate checks out `feedpak-spec` at **HEAD**, on purpose: the app must conform to the *living* spec, and
|
||||
nobody should have to maintain a pin. The dev flow is fully self-serve — gated PR → FEP → spec merge →
|
||||
re-run checks → green.
|
||||
|
||||
Two properties to know about:
|
||||
|
||||
- **The normal FEP is additive** (a new optional key), which only ever makes the gate *looser* — it cannot
|
||||
redden anyone's PR. Only a **breaking** spec change (removing/renaming a key the app uses, tightening the
|
||||
validator against committed packs) turns PRs red repo-wide — and per the spec's compatibility policy that
|
||||
is a rare, deliberate MAJOR event, exactly when an org-wide "the app is out of conformance" signal is the
|
||||
right outcome. The CI job logs the exact spec SHA each run verified against, so a red run is reproducible.
|
||||
- **CI results can change over time on the same commit** — that is inherent to tracking a living contract,
|
||||
and it is the point: green means "conformant *now*", not "conformant when written".
|
||||
|
||||
## Limitations
|
||||
|
||||
Known, and worth fixing in follow-ups rather than blocking on:
|
||||
|
||||
- **Layer 1's receiver detection is heuristic.** Locals *assigned from* `load_manifest(...)` are discovered
|
||||
flow-aware whatever they're called (chart.py's `m` taught us that), and the inline
|
||||
`(load_manifest(p) or {}).get(...)` form is recognised — but a manifest that arrives as a **function
|
||||
parameter** is only recognised by name (`MANIFEST_VARS`: `manifest`, `mf`). A parameter called something
|
||||
else would slip. The hardening step is to route all manifest access through a single declared
|
||||
`KNOWN_MANIFEST_KEYS` registry in `lib/sloppak.py`; the gate then compares registry against schema exactly
|
||||
instead of inferring.
|
||||
- **Layer 1 covers top-level keys only.** Nested structure (`arrangements[].file`, `.id`, `.notation`) isn't
|
||||
checked. Extending to it means walking the schema's `$ref` subschemas.
|
||||
- **Layer 1 recognises `get`, `setdefault`, subscripts, and the known gap-fill helper** as key access.
|
||||
`update()` and `pop()` aren't used against a feedpak manifest anywhere in the tree, so they're deliberately
|
||||
not special-cased rather than speculatively handled. `readers-complete` reuses the same scanner
|
||||
(`keys_touched()`), so this blind spot is shared, not doubled: a module using only unrecognised access forms
|
||||
would evade both.
|
||||
- **Layer 4 can't catch unknown keys**, because `manifest.schema.json` sets `additionalProperties: true` and
|
||||
the reference validator deliberately "treats unknown keys/files as forward-compatible". Fixing this
|
||||
properly belongs in the spec (tighten the schema, or give the validator a `--strict` mode). Until then,
|
||||
layer 1 is the only thing standing between us and the next `original_audio`.
|
||||
@@ -0,0 +1,50 @@
|
||||
# CLOSED grandfather list — manifest keys core reads or writes that predate the
|
||||
# spec-conformance gate and that the feedpak spec does not define.
|
||||
#
|
||||
# Please don't add entries here — CI will flag any PR that grows this list, so
|
||||
# it can only shrink over time. That's by design, not distrust: the moment the
|
||||
# app touches a key the spec doesn't define, every teammate's PR starts failing
|
||||
# the conformance gate too, and whoever added the key is the only person who
|
||||
# can fix it. The FEP process below avoids putting anyone in that spot. The
|
||||
# feedpak spec's own governance is explicit:
|
||||
#
|
||||
# "This repository defines the format only. Applications that read or write
|
||||
# feedpak ... track this spec as a dependency; they do not drive it.
|
||||
# A change is not part of the format until it lands here."
|
||||
# — got-feedback/feedpak-spec, GOVERNANCE.md
|
||||
#
|
||||
# So a new manifest key goes through the feedpak Enhancement Proposal (FEP)
|
||||
# process — see feedpak-spec/CONTRIBUTING.md:
|
||||
#
|
||||
# 1. Open a FEP issue on got-feedback/feedpak-spec describing the problem, the
|
||||
# on-disk shape, backward compatibility, and the version bump implied.
|
||||
# 2. Land one PR there updating the normative spec, the JSON Schemas, an
|
||||
# example that exercises it, and the changelog — together.
|
||||
# 3. Back here, re-run this PR's checks. The gate verifies against the spec's
|
||||
# HEAD, so once your key is in the spec, the gate goes green.
|
||||
#
|
||||
# That's the supported route — and usually a quick one for additive keys. If
|
||||
# your PR is blocked by this gate, a FEP will get you unblocked properly; an
|
||||
# entry here won't (CI rejects it).
|
||||
#
|
||||
# Entries below exist ONLY because they predate the gate. Each is debt with a
|
||||
# tracking issue, and each disappears when its issue is fixed. The gate also
|
||||
# fails if an entry goes stale — the spec caught up, or core no longer reads or
|
||||
# writes the key — so this file cannot quietly become a place drift hides.
|
||||
|
||||
exceptions:
|
||||
- key: original_audio
|
||||
issue: https://github.com/got-feedback/feedback/issues/933
|
||||
reason: >-
|
||||
Added by #583 (the full mix played while every stem fader sits at unity,
|
||||
since demucs recombination is lossy). Core, lib/enrichment.py, and the
|
||||
stems plugin all depend on it, but it never went through a FEP and the
|
||||
spec does not define it — the drift this gate exists to prevent.
|
||||
|
||||
The resolution is REMOVAL, not a FEP: the spec already carries the mixdown
|
||||
as a stem ({id: full, file: stems/full.ogg}), so this key added a second,
|
||||
redundant location for audio to a format that already had one. See #933.
|
||||
|
||||
Grandfathered so the gate can land green and start blocking the *next*
|
||||
instance immediately, rather than blocking on #933. This entry goes away
|
||||
when core no longer reads or writes the key.
|
||||
@@ -409,3 +409,60 @@
|
||||
.pp-slam, .pp-stamp-page::after { opacity: 1; }
|
||||
.pp-stamp-hidden { opacity: 0.92; }
|
||||
}
|
||||
|
||||
/* Badge ceremony (body-level overlay — shows over the player) */
|
||||
.pp-ceremony-overlay {
|
||||
position: fixed;
|
||||
inset: 0;
|
||||
z-index: 220;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
background: rgba(3, 7, 18, 0.55);
|
||||
backdrop-filter: blur(1.5px);
|
||||
animation: pp-ceremony-in 0.3s ease-out;
|
||||
cursor: pointer;
|
||||
}
|
||||
.pp-ceremony-out { opacity: 0; transition: opacity 0.3s ease-out; }
|
||||
.pp-confetti { position: absolute; inset: 0; width: 100%; height: 100%; pointer-events: none; }
|
||||
.pp-ceremony-card {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
align-items: center;
|
||||
gap: 0.5rem;
|
||||
text-align: center;
|
||||
}
|
||||
.pp-ceremony-stamp {
|
||||
position: relative;
|
||||
overflow: hidden;
|
||||
background: rgba(239, 230, 208, 0.97);
|
||||
transform: rotate(var(--pp-rot)) scale(1.25);
|
||||
animation: pp-slam 0.55s cubic-bezier(0.2, 0.8, 0.3, 1) 0.15s backwards;
|
||||
margin-top: 0;
|
||||
}
|
||||
.pp-ceremony-stamp::before {
|
||||
content: '';
|
||||
position: absolute;
|
||||
inset: -40%;
|
||||
background: linear-gradient(115deg, transparent 42%, rgba(255, 255, 255, 0.55) 50%, transparent 58%);
|
||||
transform: translateX(-120%);
|
||||
animation: pp-shine 1.1s ease-out 0.75s forwards;
|
||||
pointer-events: none;
|
||||
}
|
||||
@keyframes pp-shine {
|
||||
to { transform: translateX(120%); }
|
||||
}
|
||||
@keyframes pp-ceremony-in {
|
||||
from { opacity: 0; }
|
||||
to { opacity: 1; }
|
||||
}
|
||||
.pp-ceremony-title {
|
||||
margin-top: 1rem;
|
||||
font-size: 1.15rem;
|
||||
font-weight: 700;
|
||||
letter-spacing: 0.18em;
|
||||
text-transform: uppercase;
|
||||
color: #f0e2c3;
|
||||
text-shadow: 0 2px 8px rgba(0, 0, 0, 0.8);
|
||||
}
|
||||
.pp-ceremony-sub { font-size: 0.8rem; color: #d1d5db; text-shadow: 0 1px 4px rgba(0, 0, 0, 0.8); }
|
||||
|
||||
+105
-3
@@ -34,6 +34,8 @@
|
||||
let _ppRelayTimer = 0;
|
||||
let _ppBook = null; // {inst, gkey} of the open spread
|
||||
let _ppReturnFocus = null; // element to refocus when the book closes
|
||||
let _ppCeremonyQueue = []; // badges awaiting their ceremony overlay
|
||||
let _ppCeremonyActive = false;
|
||||
let _ppBootstrapped = false;
|
||||
let _ppNotified = {}; // badges chimed this session (slam still pending)
|
||||
|
||||
@@ -308,9 +310,10 @@
|
||||
lsSet(PP_SEEN_KEY, JSON.stringify(seen));
|
||||
}
|
||||
|
||||
// New badge → chime + notification once per session; the stamp SLAM plays
|
||||
// when the passport is next opened (and only then is the badge marked
|
||||
// seen, so a pending slam survives a reload).
|
||||
// New badge → chime + notification + the venue ceremony, once per
|
||||
// session; the stamp SLAM plays when the passport is next opened (and
|
||||
// only then is the badge marked seen, so a pending slam survives a
|
||||
// reload).
|
||||
function detectNewBadges(view) {
|
||||
const seen = seenBadges();
|
||||
for (const inst of Object.keys(view.instruments || {})) {
|
||||
@@ -326,10 +329,109 @@
|
||||
message: `${p.genre} — Bronze, ready to stamp into your ${ppLabel(inst)} passport.`,
|
||||
});
|
||||
}
|
||||
badgeCeremony(inst, p);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function reducedMotion() {
|
||||
try { return window.matchMedia('(prefers-reduced-motion: reduce)').matches; } catch (_) { return false; }
|
||||
}
|
||||
|
||||
// The badge moment: the crowd erupts first (if a venue pack is live —
|
||||
// badges land post-stats:recorded while the player is still on screen),
|
||||
// then a body-level overlay. It CANNOT live in #pp-overlay: #plugin-career
|
||||
// is display:none during playback.
|
||||
function badgeCeremony(inst, p) {
|
||||
// Reduced motion: the chime + fbNotify already delivered the news —
|
||||
// no overlay, and no app-initiated crowd eruption either.
|
||||
if (reducedMotion()) return;
|
||||
const crowd = window.v3VenueCrowd;
|
||||
if (crowd && typeof crowd.celebrate === 'function') {
|
||||
try { crowd.celebrate(); } catch (_) { /* crowd layer optional */ }
|
||||
}
|
||||
if (!document.body || typeof document.createElement !== 'function') return;
|
||||
// Several badges can land in one refresh (first load, drill-snapshot
|
||||
// bootstrap): queue the ceremonies and play them back to back.
|
||||
_ppCeremonyQueue.push({ inst, p });
|
||||
if (!_ppCeremonyActive) setTimeout(drainCeremonies, 300);
|
||||
}
|
||||
|
||||
function drainCeremonies() {
|
||||
if (_ppCeremonyActive) return;
|
||||
const queued = _ppCeremonyQueue.shift();
|
||||
if (!queued) return;
|
||||
_ppCeremonyActive = true;
|
||||
showCeremonyOverlay(queued.inst, queued.p, () => {
|
||||
_ppCeremonyActive = false;
|
||||
setTimeout(drainCeremonies, 250);
|
||||
});
|
||||
}
|
||||
|
||||
function showCeremonyOverlay(inst, p, done) {
|
||||
const el = document.createElement('div');
|
||||
el.id = 'pp-ceremony';
|
||||
el.className = 'pp-ceremony-overlay';
|
||||
el.innerHTML = `
|
||||
<canvas class="pp-confetti"></canvas>
|
||||
<div class="pp-ceremony-card">
|
||||
<div class="pp-stamp pp-stamp-page pp-ceremony-stamp" style="--pp-rot:${ppJitter(p.genre_key, 7).toFixed(1)}deg">
|
||||
<span class="pp-stamp-genre">${esc(p.genre.toUpperCase())}</span>
|
||||
<span class="pp-stamp-tier">BRONZE</span>
|
||||
</div>
|
||||
<div class="pp-ceremony-title">Badge earned</div>
|
||||
<div class="pp-ceremony-sub">${esc(p.genre)} — ${esc(ppLabel(inst))} passport</div>
|
||||
</div>`;
|
||||
let timer = 0;
|
||||
let closed = false;
|
||||
const dismiss = () => {
|
||||
if (closed) return;
|
||||
closed = true;
|
||||
clearTimeout(timer);
|
||||
el.classList.add('pp-ceremony-out');
|
||||
setTimeout(() => { el.remove(); done(); }, 350);
|
||||
};
|
||||
el.addEventListener('click', dismiss);
|
||||
document.body.appendChild(el);
|
||||
timer = setTimeout(dismiss, 4200);
|
||||
confettiBurst(el.querySelector('.pp-confetti'));
|
||||
}
|
||||
|
||||
function confettiBurst(canvas) {
|
||||
if (!canvas || typeof canvas.getContext !== 'function') return;
|
||||
const ctx = canvas.getContext('2d');
|
||||
if (!ctx) return;
|
||||
canvas.width = canvas.clientWidth;
|
||||
canvas.height = canvas.clientHeight;
|
||||
const colors = ['#d9a253', '#b45309', '#facc15', '#06b6d4', '#e5e7eb'];
|
||||
const parts = Array.from({ length: 42 }, () => ({
|
||||
x: canvas.width / 2 + (Math.random() - 0.5) * 90,
|
||||
y: canvas.height * 0.42,
|
||||
vx: (Math.random() - 0.5) * 9,
|
||||
vy: -(4 + Math.random() * 7),
|
||||
rot: Math.random() * Math.PI,
|
||||
vr: (Math.random() - 0.5) * 0.3,
|
||||
w: 5 + Math.random() * 5,
|
||||
h: 3 + Math.random() * 4,
|
||||
c: colors[(Math.random() * colors.length) | 0],
|
||||
}));
|
||||
let frames = 0;
|
||||
(function tick() {
|
||||
if (!canvas.isConnected || frames++ > 240) return;
|
||||
ctx.clearRect(0, 0, canvas.width, canvas.height);
|
||||
for (const q of parts) {
|
||||
q.x += q.vx; q.y += q.vy; q.vy += 0.18; q.rot += q.vr;
|
||||
ctx.save();
|
||||
ctx.translate(q.x, q.y);
|
||||
ctx.rotate(q.rot);
|
||||
ctx.fillStyle = q.c;
|
||||
ctx.fillRect(-q.w / 2, -q.h / 2, q.w, q.h);
|
||||
ctx.restore();
|
||||
}
|
||||
requestAnimationFrame(tick);
|
||||
}());
|
||||
}
|
||||
|
||||
// Relay the Virtuoso drill snapshot (localStorage doc, not the thin bus
|
||||
// payload) to the server intake, debounced across event bursts.
|
||||
function relayDrillState() {
|
||||
|
||||
@@ -88,6 +88,33 @@ test('detectNewBadges notifies once per badge, never after it is seen', () => {
|
||||
assert.equal(w2.notifications.length, 0);
|
||||
});
|
||||
|
||||
test('a new badge triggers the crowd celebrate() exactly once', () => {
|
||||
const w = load();
|
||||
let calls = 0;
|
||||
w.v3VenueCrowd = { celebrate: () => { calls += 1; } };
|
||||
const view = { instruments: { guitar: { passports: [
|
||||
{ genre_key: 'blues', genre: 'Blues', badge: 'earned' }] } } };
|
||||
w.__careerPassportTest.detectNewBadges(view);
|
||||
assert.equal(calls, 1);
|
||||
// Same session, same view: no re-celebration.
|
||||
w.__careerPassportTest.detectNewBadges(view);
|
||||
assert.equal(calls, 1);
|
||||
});
|
||||
|
||||
test('ceremony degrades when the crowd layer is absent or throws', () => {
|
||||
const w = load();
|
||||
const view = { instruments: { guitar: { passports: [
|
||||
{ genre_key: 'blues', genre: 'Blues', badge: 'earned' }] } } };
|
||||
// No v3VenueCrowd at all (already exercised elsewhere, explicit here).
|
||||
w.__careerPassportTest.detectNewBadges(view);
|
||||
assert.equal(w.notifications.length, 1);
|
||||
// celebrate() throwing must not break detection.
|
||||
const w2 = load();
|
||||
w2.v3VenueCrowd = { celebrate: () => { throw new Error('no pack'); } };
|
||||
w2.__careerPassportTest.detectNewBadges(view);
|
||||
assert.equal(w2.notifications.length, 1);
|
||||
});
|
||||
|
||||
test('seenBadges tolerates corrupt stored values', () => {
|
||||
for (const bad of ['null', '[1,2]', '"x"', '{{{']) {
|
||||
const w = load({ 'feedBack-career-badges-seen': bad });
|
||||
|
||||
@@ -58,6 +58,15 @@
|
||||
candidate = null;
|
||||
lastSwitchAt = -Infinity;
|
||||
},
|
||||
// Commit a state NOW, bypassing stability/dwell (badge ceremony).
|
||||
// Stamping lastSwitchAt makes the dwell window hold the forced
|
||||
// state before the real perf machine can reassert.
|
||||
force(state, nowMs) {
|
||||
if (!CROWD_STATES.includes(state)) return;
|
||||
current = state;
|
||||
candidate = null;
|
||||
lastSwitchAt = nowMs;
|
||||
},
|
||||
// Feed the latest perf state; returns the new crowd state when a
|
||||
// transition commits, else null.
|
||||
update(perfState, nowMs) {
|
||||
@@ -564,6 +573,26 @@
|
||||
if (dev && !_manifest) setManifest(dev);
|
||||
}
|
||||
|
||||
// Badge-ceremony hook (career passports): the crowd erupts NOW — ecstatic
|
||||
// loop bypassing stability/dwell (the dwell window then holds it while
|
||||
// the real perf state waits its turn) plus a cheer. Degrades to a no-op
|
||||
// without a pack / outside the player, like every other entry point.
|
||||
function celebrate() {
|
||||
if (!_venueActive || !_manifest || !_videos[0]) return false;
|
||||
machine.force('ecstatic', now());
|
||||
if (_stingerUntilEnded || _introActive) {
|
||||
// A stinger/intro owns the idle layer (likely the end-of-song
|
||||
// accuracy cheer — the crowd is already reacting); queue the
|
||||
// ecstatic loop for when it ends, same as onPerformanceState.
|
||||
_pendingLoop = 'ecstatic';
|
||||
} else {
|
||||
showLoop('ecstatic', FADE_MS);
|
||||
_lastStingerAt = -Infinity; // a badge earn always gets its cheer
|
||||
playStinger('cheer');
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function getState() {
|
||||
return {
|
||||
venueActive: _venueActive,
|
||||
@@ -589,6 +618,7 @@
|
||||
setVenueActive,
|
||||
bindRuntime,
|
||||
getState,
|
||||
celebrate,
|
||||
};
|
||||
|
||||
if (root) root.v3VenueCrowd = api;
|
||||
|
||||
@@ -128,3 +128,22 @@ test('venue-scene-3d activates/deactivates the crowd layer', () => {
|
||||
assert.match(src, /syncCrowd\(false\)/);
|
||||
assert.match(src, /v3VenueCrowd/);
|
||||
});
|
||||
|
||||
test('machine.force commits instantly and dwell holds the forced state', () => {
|
||||
const m = crowd.createCrowdMachine();
|
||||
m.force('ecstatic', 100000);
|
||||
assert.equal(m.current, 'ecstatic');
|
||||
// The real perf state cannot reassert until the dwell window passes.
|
||||
m.update('smoke', 100000 + crowd.STABLE_MS);
|
||||
assert.equal(m.update('smoke', 100000 + crowd.DWELL_MS - 1), null);
|
||||
assert.equal(m.current, 'ecstatic');
|
||||
assert.equal(m.update('smoke', 100000 + crowd.DWELL_MS), 'bored');
|
||||
// Bogus states are ignored.
|
||||
m.force('confused', 200000);
|
||||
assert.equal(m.current, 'bored');
|
||||
});
|
||||
|
||||
test('celebrate() is exported and no-ops without a manifest/active venue', () => {
|
||||
assert.equal(typeof crowd.celebrate, 'function');
|
||||
assert.equal(crowd.celebrate(), false);
|
||||
});
|
||||
|
||||
@@ -0,0 +1,203 @@
|
||||
"""Regression tests for the feedpak spec-conformance gate.
|
||||
|
||||
The gate (tools/check_spec_conformance.py) is what keeps the app from drifting
|
||||
away from the feedpak spec, so the gate itself must not be weakenable by a
|
||||
quiet refactor: these tests pin its load-bearing behaviours — read/write
|
||||
classification, the closed allowlist, and the duplicate/malformed-entry
|
||||
rejections. If one of these fails, the spec's protection regressed.
|
||||
"""
|
||||
import importlib.util
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
_SPEC_GATE = Path(__file__).resolve().parent.parent / "tools" / "check_spec_conformance.py"
|
||||
_spec = importlib.util.spec_from_file_location("check_spec_conformance", _SPEC_GATE)
|
||||
gate = importlib.util.module_from_spec(_spec)
|
||||
_spec.loader.exec_module(gate)
|
||||
|
||||
|
||||
def _touch(tmp_path, source):
|
||||
p = tmp_path / "mod.py"
|
||||
p.write_text(textwrap.dedent(source), encoding="utf-8")
|
||||
return gate.keys_touched(p)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------- keys_touched
|
||||
|
||||
def test_get_is_a_read(tmp_path):
|
||||
reads, writes = _touch(tmp_path, 'x = manifest.get("title")')
|
||||
assert reads == {"title"} and writes == set()
|
||||
|
||||
|
||||
def test_subscript_load_is_a_read(tmp_path):
|
||||
reads, writes = _touch(tmp_path, 'x = manifest["artist"]')
|
||||
assert reads == {"artist"} and writes == set()
|
||||
|
||||
|
||||
def test_subscript_store_is_a_write_not_a_read(tmp_path):
|
||||
# The original scan ignored ctx and scored this as a read (lib/songmeta.py
|
||||
# pattern). A regression here reopens the emitted-key blind spot.
|
||||
reads, writes = _touch(tmp_path, 'manifest["year"] = 1999')
|
||||
assert writes == {"year"} and reads == set()
|
||||
|
||||
|
||||
def test_setdefault_is_a_write(tmp_path):
|
||||
# lib/gp2notation.py stamps feedpak_version this way; a subscript-only scan
|
||||
# missed it entirely.
|
||||
reads, writes = _touch(tmp_path, 'manifest.setdefault("feedpak_version", "1.2.0")')
|
||||
assert writes == {"feedpak_version"} and reads == set()
|
||||
|
||||
|
||||
def test_load_manifest_wrapped_get_is_seen(tmp_path):
|
||||
# lib/enrichment.py idiom: (load_manifest(p) or {}).get("key")
|
||||
reads, writes = _touch(
|
||||
tmp_path, 'rel = (sloppak_mod.load_manifest(p) or {}).get("original_audio")'
|
||||
)
|
||||
assert "original_audio" in reads
|
||||
|
||||
|
||||
def test_flow_aware_receiver_any_name(tmp_path):
|
||||
# lib/routers/chart.py binds `m = load_manifest(p) or {}` — a fixed name
|
||||
# list missed it and the module's reads went entirely unscanned. Locals
|
||||
# assigned from load_manifest must be receivers whatever they're called.
|
||||
reads, writes = _touch(
|
||||
tmp_path,
|
||||
"""
|
||||
pak_info = sloppak_mod.load_manifest(p) or {}
|
||||
x = pak_info.get("stems")
|
||||
pak_info["genres"] = ["metal"]
|
||||
""",
|
||||
)
|
||||
assert reads == {"stems"} and writes == {"genres"}
|
||||
|
||||
|
||||
def test_plain_dict_named_m_is_not_a_receiver(tmp_path):
|
||||
# Flow-awareness must not make every short local a manifest: `m` bound to
|
||||
# something other than load_manifest stays out of the scan.
|
||||
reads, writes = _touch(tmp_path, 'm = {}\nx = m.get("title")')
|
||||
assert reads == set() and writes == set()
|
||||
|
||||
|
||||
def test_unrelated_dicts_are_ignored(tmp_path):
|
||||
reads, writes = _touch(tmp_path, 'x = config.get("title"); settings["artist"] = 1')
|
||||
assert reads == set() and writes == set()
|
||||
|
||||
|
||||
def test_non_literal_keys_are_ignored(tmp_path):
|
||||
reads, writes = _touch(tmp_path, 'x = manifest.get(key_var); manifest[key_var] = 1')
|
||||
assert reads == set() and writes == set()
|
||||
|
||||
|
||||
def test_manifest_key_read_helper_is_seen(tmp_path):
|
||||
# lib/routers/song.py uses this helper for gap-fill proposals. If helpers
|
||||
# are invisible, adding a new literal key through that path bypasses both
|
||||
# key-coverage and readers-complete.
|
||||
reads, writes = _touch(
|
||||
tmp_path,
|
||||
'_gap_fill_manifest_absent(manifest, "album")\n'
|
||||
'_gap_fill_manifest_absent(manifest, dynamic_key)\n',
|
||||
)
|
||||
assert reads == {"album"} and writes == set()
|
||||
|
||||
|
||||
# ------------------------------------------------------------ exceptions file
|
||||
|
||||
def test_duplicate_exception_key_is_rejected():
|
||||
doc = """
|
||||
exceptions:
|
||||
- key: original_audio
|
||||
issue: https://example.com/1
|
||||
- key: original_audio
|
||||
issue: https://example.com/2
|
||||
"""
|
||||
with pytest.raises(SystemExit):
|
||||
gate._parse_exceptions(textwrap.dedent(doc), "test")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("doc", [
|
||||
"- just\n- a\n- list\n", # list at top level
|
||||
"exceptions: not-a-list\n", # scalar where list expected
|
||||
"exceptions:\n - just-a-string\n", # non-mapping entry
|
||||
"exceptions: [\n", # invalid YAML
|
||||
])
|
||||
def test_malformed_exceptions_fail_legibly(doc):
|
||||
# Malformed shapes must exit with a ::error::, not an AttributeError
|
||||
# traceback — CI output has to say what to fix.
|
||||
with pytest.raises(SystemExit):
|
||||
gate._parse_exceptions(doc, "test")
|
||||
|
||||
|
||||
def test_exception_without_issue_is_rejected():
|
||||
# No tracking issue, no exception — entries are debt and debt is tracked.
|
||||
doc = """
|
||||
exceptions:
|
||||
- key: original_audio
|
||||
"""
|
||||
with pytest.raises(SystemExit):
|
||||
gate._parse_exceptions(textwrap.dedent(doc), "test")
|
||||
|
||||
|
||||
# --------------------------------------------------------- allowlist is CLOSED
|
||||
|
||||
def _yml(tmp_path, name, keys):
|
||||
p = tmp_path / name
|
||||
entries = "".join(
|
||||
f" - key: {k}\n issue: https://example.com/{k}\n" for k in keys
|
||||
)
|
||||
p.write_text("exceptions:\n" + (entries or " []\n"), encoding="utf-8")
|
||||
return p
|
||||
|
||||
|
||||
def test_allowlist_may_not_grow(tmp_path, monkeypatch):
|
||||
# THE core property: adding an entry must fail, or the FEP process has an
|
||||
# in-repo bypass and the gate is a speed bump with a signed excuse note.
|
||||
baseline = _yml(tmp_path, "base.yml", ["original_audio"])
|
||||
current = _yml(tmp_path, "current.yml", ["original_audio", "sneaky_new_key"])
|
||||
monkeypatch.setattr(gate, "EXCEPTIONS_FILE", current)
|
||||
assert gate.check_allowlist_closed(baseline, bootstrap=False) is False
|
||||
|
||||
|
||||
def test_allowlist_may_shrink(tmp_path, monkeypatch):
|
||||
baseline = _yml(tmp_path, "base.yml", ["original_audio"])
|
||||
current = _yml(tmp_path, "current.yml", [])
|
||||
monkeypatch.setattr(gate, "EXCEPTIONS_FILE", current)
|
||||
assert gate.check_allowlist_closed(baseline, bootstrap=False) is True
|
||||
|
||||
|
||||
def test_allowlist_steady_state_passes(tmp_path, monkeypatch):
|
||||
baseline = _yml(tmp_path, "base.yml", ["original_audio"])
|
||||
current = _yml(tmp_path, "current.yml", ["original_audio"])
|
||||
monkeypatch.setattr(gate, "EXCEPTIONS_FILE", current)
|
||||
assert gate.check_allowlist_closed(baseline, bootstrap=False) is True
|
||||
|
||||
|
||||
def test_bootstrap_skips_the_diff(tmp_path, monkeypatch):
|
||||
current = _yml(tmp_path, "current.yml", ["original_audio"])
|
||||
monkeypatch.setattr(gate, "EXCEPTIONS_FILE", current)
|
||||
assert gate.check_allowlist_closed(None, bootstrap=True) is True
|
||||
|
||||
|
||||
# ------------------------------------------------------------ readers-complete
|
||||
|
||||
def test_readers_list_matches_the_codebase():
|
||||
# If this fails, a module started touching feedpak manifests without being
|
||||
# added to READERS — its keys are going unchecked. Same check CI runs.
|
||||
assert gate.check_readers_complete() is True
|
||||
|
||||
|
||||
def test_no_undeclared_keys_beyond_the_grandfathered(monkeypatch):
|
||||
# Every key core touches is either spec-declared or grandfathered with a
|
||||
# tracking issue. New keys go through the FEP process, full stop.
|
||||
reads, writes = set(), set()
|
||||
for rel in gate.READERS:
|
||||
r, w = gate.keys_touched(gate.REPO / rel)
|
||||
reads |= r
|
||||
writes |= w
|
||||
grandfathered = set(gate.load_exceptions())
|
||||
# Not asserting against the spec here (no spec checkout in unit tests) —
|
||||
# asserting the *shape*: the only non-spec keys tolerated are grandfathered,
|
||||
# and today that is exactly {original_audio}.
|
||||
assert grandfathered == {"original_audio"}
|
||||
assert "original_audio" in reads
|
||||
@@ -0,0 +1,537 @@
|
||||
#!/usr/bin/env python3
|
||||
"""feedpak spec-conformance gate.
|
||||
|
||||
feedpak is an open, versioned format with its own normative spec, JSON Schemas,
|
||||
and reference validator (https://github.com/got-feedback/feedpak-spec). That
|
||||
makes the spec a contract with everyone outside this repo: third-party packers,
|
||||
converters, and players build against it. When core reads a manifest key the
|
||||
spec never defined, the contract quietly breaks — a spec-compliant pack stops
|
||||
being a fully-working pack, and the format's real definition migrates into our
|
||||
source tree. See #933 for the instance that motivated this gate.
|
||||
|
||||
We cannot mechanically prove core *interprets* a key the way the spec means. We
|
||||
can prove four surface properties, and those cover the drift that actually
|
||||
happens:
|
||||
|
||||
1. key-coverage — every manifest key core reads OR WRITES is declared by the
|
||||
spec. (Guarded by check_readers_complete(), so the list of
|
||||
scanned modules cannot quietly fall behind the codebase.)
|
||||
2. allowlist-closed— feedpak-spec-exceptions.yml never grows. It grandfathers
|
||||
keys that predate this gate; it is not a way to merge a
|
||||
new one. The only route for a new key is the FEP process.
|
||||
3. forward — core ingests the spec's own example packs.
|
||||
4. reverse — packs committed here satisfy the spec's reference validator.
|
||||
|
||||
Dev/CI tooling only: never imported on the serve or Docker path (constitution
|
||||
Principle I — same category as scripts/build-tailwind.sh). `jsonschema` is
|
||||
therefore a CI-only dependency, not a runtime requirement.
|
||||
|
||||
Usage:
|
||||
python tools/check_spec_conformance.py --spec <path-to-feedpak-spec-checkout>
|
||||
|
||||
Exit status is 0 only when every layer passes.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import ast
|
||||
import json
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
REPO = Path(__file__).resolve().parent.parent
|
||||
|
||||
# Modules that read or write a feedpak manifest dict. Explicit rather than
|
||||
# globbed, because `manifest` is an overloaded name in this codebase: the
|
||||
# loose-folder format (lib/loosefolder.py) and the diagnostics bundle
|
||||
# (lib/diagnostics_bundle.py) both have their own unrelated `manifest`, and
|
||||
# scanning those would flag *their* keys as feedpak drift.
|
||||
#
|
||||
# A hand-maintained list is itself a blind spot, so check_readers_complete()
|
||||
# below re-derives the set and fails if this list has fallen behind. A missing
|
||||
# file here is a hard error too, so a rename cannot silently disable the scan.
|
||||
READERS = [
|
||||
"lib/sloppak.py",
|
||||
"lib/enrichment.py",
|
||||
"lib/songmeta.py",
|
||||
"lib/gp2notation.py", # rewrites manifest.yaml; stamps feedpak_version
|
||||
"lib/routers/ws_highway.py", # reads `authors` off a feedpak manifest
|
||||
"lib/routers/chart.py", # Get-info panel: binds `m = load_manifest(...)`
|
||||
"lib/routers/song.py", # enrichment gap-fill: reads the manifest directly
|
||||
]
|
||||
|
||||
# Where check_readers_complete() looks for modules READERS may have missed.
|
||||
READER_SEARCH = ["lib/**/*.py", "server.py"]
|
||||
|
||||
# A module is handling a *feedpak* manifest (rather than some other manifest) if
|
||||
# it shows one of these signals. lib/loosefolder.py and lib/diagnostics_bundle.py
|
||||
# score zero on all of them, which is what keeps their keys out of the scan.
|
||||
FEEDPAK_SIGNALS = re.compile(r"import sloppak|from sloppak|load_manifest|manifest\.yaml|feedpak")
|
||||
|
||||
# Locals assumed to hold a manifest dict by NAME. This is only the fallback for
|
||||
# manifests that arrive as function parameters (ws_highway's `manifest` arg);
|
||||
# locals ASSIGNED from load_manifest(...) are discovered flow-aware in
|
||||
# keys_touched(), whatever they are called — chart.py's `m` taught us that a
|
||||
# name list alone silently misses real readers.
|
||||
MANIFEST_VARS = {"manifest", "mf"}
|
||||
|
||||
# Helper functions that take `(manifest, "literal_key", ...)` and read the
|
||||
# manifest for that key. Keep this narrow: only helpers whose first argument is
|
||||
# the manifest dict and whose second argument is a top-level manifest key belong
|
||||
# here.
|
||||
MANIFEST_KEY_READ_HELPERS = {"_gap_fill_manifest_absent"}
|
||||
|
||||
# Packs committed to this repo, checked against the spec's reference validator.
|
||||
PACK_GLOBS = ["content/starter/*.feedpak", "docs/**/*.sloppak", "docs/**/*.feedpak"]
|
||||
|
||||
EXCEPTIONS_FILE = REPO / "feedpak-spec-exceptions.yml"
|
||||
|
||||
# How a new manifest key gets into core. There is no in-repo shortcut, by design:
|
||||
# the spec's own governance says "a change is not part of the format until it
|
||||
# lands here", and the FEP process is how it lands.
|
||||
FEP = (
|
||||
"New manifest keys go through the feedpak Enhancement Proposal process "
|
||||
"(https://github.com/got-feedback/feedpak-spec/blob/main/CONTRIBUTING.md): land a PR on "
|
||||
"feedpak-spec that updates the normative spec, the JSON Schemas, an example, and the "
|
||||
"changelog together — then re-run this PR's checks; the gate verifies against the "
|
||||
"spec's HEAD, so once your key is in the spec, this PR goes green. It matters beyond "
|
||||
"this PR: the whole repo is checked against the living spec, so non-conformance that "
|
||||
"slips in shows up as red CI on every teammate's PR until it's resolved — sorting it "
|
||||
"out here keeps everyone else unblocked."
|
||||
)
|
||||
|
||||
|
||||
def _fail(msg: str) -> None:
|
||||
print(f"::error::{msg}")
|
||||
|
||||
|
||||
def _manifest_locals(tree: ast.AST) -> set[str]:
|
||||
"""Names of locals assigned from `load_manifest(...)` anywhere in `tree`.
|
||||
|
||||
Flow-aware receiver discovery: chart.py binds `m = load_manifest(p) or {}`,
|
||||
and a fixed name list (`manifest`, `mf`) silently missed it — the module's
|
||||
reads went entirely unscanned. Whatever the local is called, an assignment
|
||||
whose right-hand side mentions load_manifest marks it as a manifest dict.
|
||||
"""
|
||||
names: set[str] = set()
|
||||
for node in ast.walk(tree):
|
||||
if not isinstance(node, (ast.Assign, ast.AnnAssign)):
|
||||
continue
|
||||
try:
|
||||
rhs = ast.unparse(node.value) if node.value else ""
|
||||
except Exception:
|
||||
continue
|
||||
if "load_manifest" not in rhs:
|
||||
continue
|
||||
targets = node.targets if isinstance(node, ast.Assign) else [node.target]
|
||||
for t in targets:
|
||||
if isinstance(t, ast.Name):
|
||||
names.add(t.id)
|
||||
return names
|
||||
|
||||
|
||||
def _is_manifest_receiver(node: ast.expr, receivers: set[str]) -> bool:
|
||||
"""True when `node` evaluates to a manifest dict.
|
||||
|
||||
Covers named receivers (fixed names + flow-discovered locals) plus the
|
||||
inline wrapped form used in lib/enrichment.py:
|
||||
`(sloppak_mod.load_manifest(p) or {}).get("key")`.
|
||||
"""
|
||||
if isinstance(node, ast.Name) and node.id in receivers:
|
||||
return True
|
||||
try:
|
||||
src = ast.unparse(node)
|
||||
except Exception:
|
||||
return False
|
||||
return "load_manifest" in src
|
||||
|
||||
|
||||
def keys_touched(path: Path) -> tuple[set[str], set[str]]:
|
||||
"""Literal top-level manifest keys `path` reads and writes, separately.
|
||||
|
||||
Writes matter as much as reads: `manifest["k"] = v` means core *emits* `k`
|
||||
into a pack it ships, so an undeclared key there puts non-spec surface into
|
||||
the wild — the same drift, pointed outward. `manifest["k"]` in a subscript
|
||||
is a read only when its context is a Load; an `ast.walk` that ignores `ctx`
|
||||
would score `manifest["year"] = ...` (lib/songmeta.py) as a read.
|
||||
"""
|
||||
reads: set[str] = set()
|
||||
writes: set[str] = set()
|
||||
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
|
||||
receivers = MANIFEST_VARS | _manifest_locals(tree)
|
||||
for node in ast.walk(tree):
|
||||
if (
|
||||
isinstance(node, ast.Call)
|
||||
and isinstance(node.func, ast.Attribute)
|
||||
# `setdefault("k", v)` writes k when absent — lib/gp2notation.py
|
||||
# stamps feedpak_version that way, and a subscript-only scan misses
|
||||
# it entirely, letting an emitted key slip past the gate.
|
||||
and node.func.attr in ("get", "setdefault")
|
||||
and _is_manifest_receiver(node.func.value, receivers)
|
||||
and node.args
|
||||
and isinstance(node.args[0], ast.Constant)
|
||||
and isinstance(node.args[0].value, str)
|
||||
):
|
||||
bucket = writes if node.func.attr == "setdefault" else reads
|
||||
bucket.add(node.args[0].value)
|
||||
elif (
|
||||
isinstance(node, ast.Call)
|
||||
and isinstance(node.func, ast.Name)
|
||||
and node.func.id in MANIFEST_KEY_READ_HELPERS
|
||||
and len(node.args) >= 2
|
||||
and _is_manifest_receiver(node.args[0], receivers)
|
||||
and isinstance(node.args[1], ast.Constant)
|
||||
and isinstance(node.args[1].value, str)
|
||||
):
|
||||
reads.add(node.args[1].value)
|
||||
elif (
|
||||
isinstance(node, ast.Subscript)
|
||||
and _is_manifest_receiver(node.value, receivers)
|
||||
and isinstance(node.slice, ast.Constant)
|
||||
and isinstance(node.slice.value, str)
|
||||
):
|
||||
target = writes if isinstance(node.ctx, ast.Store) else reads
|
||||
target.add(node.slice.value)
|
||||
return reads, writes
|
||||
|
||||
|
||||
def _parse_exceptions(text: str, origin: str) -> dict[str, str]:
|
||||
"""Parse an exceptions document into {key: tracking issue}."""
|
||||
import yaml # runtime dep (PyYAML is already in requirements.txt)
|
||||
|
||||
try:
|
||||
data = yaml.safe_load(text) or {}
|
||||
except yaml.YAMLError as e:
|
||||
_fail(f"{origin}: not valid YAML — {e}")
|
||||
sys.exit(1)
|
||||
# A malformed shape (list/string at top level, non-mapping entry) must fail
|
||||
# with a CI-legible error, not an AttributeError traceback.
|
||||
if not isinstance(data, dict):
|
||||
_fail(f"{origin}: top level must be a mapping with an 'exceptions' list, got {type(data).__name__}")
|
||||
sys.exit(1)
|
||||
entries = data.get("exceptions") or []
|
||||
if not isinstance(entries, list):
|
||||
_fail(f"{origin}: 'exceptions' must be a list, got {type(entries).__name__}")
|
||||
sys.exit(1)
|
||||
out: dict[str, str] = {}
|
||||
for entry in entries:
|
||||
if not isinstance(entry, dict):
|
||||
_fail(f"{origin}: each exception must be a mapping with 'key' and 'issue', got {type(entry).__name__}")
|
||||
sys.exit(1)
|
||||
key, issue = entry.get("key"), entry.get("issue")
|
||||
if not key or not issue:
|
||||
_fail(f"{origin}: every exception needs both 'key' and 'issue'")
|
||||
sys.exit(1)
|
||||
# A duplicate would silently take the last issue link, quietly retargeting
|
||||
# the debt this file exists to track. Fail instead.
|
||||
if key in out:
|
||||
_fail(
|
||||
f"{origin}: '{key}' is listed more than once. "
|
||||
f"Keep one entry per key so the tracking issue is unambiguous."
|
||||
)
|
||||
sys.exit(1)
|
||||
out[key] = issue
|
||||
return out
|
||||
|
||||
|
||||
def load_exceptions() -> dict[str, str]:
|
||||
"""Map of grandfathered key -> tracking issue URL, as of this working tree."""
|
||||
if not EXCEPTIONS_FILE.exists():
|
||||
return {}
|
||||
return _parse_exceptions(
|
||||
EXCEPTIONS_FILE.read_text(encoding="utf-8"), EXCEPTIONS_FILE.name
|
||||
)
|
||||
|
||||
|
||||
def check_allowlist_closed(baseline: Path | None, bootstrap: bool) -> bool:
|
||||
"""The allowlist is CLOSED: it may shrink, never grow.
|
||||
|
||||
`feedpak-spec-exceptions.yml` grandfathers keys that predate this gate. It is
|
||||
not a way to merge a new one. Without this check the gate would be a speed
|
||||
bump with a signed excuse note — anyone could append an entry and route
|
||||
around the FEP process from inside this repo, which is exactly the drift that
|
||||
produced #933.
|
||||
|
||||
So: removing an entry is fine (that's the debt being paid down); adding one
|
||||
fails the build, and the error points at the FEP process instead.
|
||||
"""
|
||||
if bootstrap:
|
||||
print(" allowlist-closed: bootstrapping (no baseline on the base branch) — skipped")
|
||||
return True
|
||||
if baseline is None:
|
||||
print(" allowlist-closed: no baseline supplied (local run) — skipped")
|
||||
return True
|
||||
|
||||
if not baseline.is_file():
|
||||
_fail(
|
||||
f"--baseline-exceptions {baseline} does not exist. CI derives this from the base "
|
||||
f"branch; for a local run, omit the flag to skip the allowlist diff."
|
||||
)
|
||||
return False
|
||||
base_keys = set(
|
||||
_parse_exceptions(baseline.read_text(encoding="utf-8"), f"{EXCEPTIONS_FILE.name} (base)")
|
||||
)
|
||||
now_keys = set(load_exceptions())
|
||||
added = sorted(now_keys - base_keys)
|
||||
removed = sorted(base_keys - now_keys)
|
||||
|
||||
for key in added:
|
||||
_fail(
|
||||
f"{EXCEPTIONS_FILE.name}: this PR adds an exception for '{key}', and the allowlist "
|
||||
f"can't take new entries — it only grandfathers keys that predate the gate. {FEP}"
|
||||
)
|
||||
if removed:
|
||||
print(f" allowlist shrank (debt paid down): {', '.join(removed)}")
|
||||
print(f" allowlist-closed: {'FAILED' if added else 'OK'}")
|
||||
return not added
|
||||
|
||||
|
||||
def check_readers_complete() -> bool:
|
||||
"""READERS must not fall behind the codebase.
|
||||
|
||||
The key-coverage scan is only as good as the list of modules it scans, and a
|
||||
hand-maintained list rots: `lib/routers/ws_highway.py` and
|
||||
`lib/gp2notation.py` both touched feedpak manifests for a while without being
|
||||
on it. So re-derive the set — any module that both touches manifest keys and
|
||||
shows a feedpak signal must be listed — and fail if one is missing.
|
||||
|
||||
This is a guard on the gate itself, not on the format.
|
||||
"""
|
||||
listed = set(READERS)
|
||||
missing: list[str] = []
|
||||
for pattern in READER_SEARCH:
|
||||
for path in sorted(REPO.glob(pattern)):
|
||||
rel = path.relative_to(REPO).as_posix()
|
||||
if rel in listed:
|
||||
continue
|
||||
src = path.read_text(encoding="utf-8", errors="replace")
|
||||
if not FEEDPAK_SIGNALS.search(src):
|
||||
continue
|
||||
# Same scanner the coverage check uses — a separate "does it touch
|
||||
# keys" regex diverged from it once already (`m = load_manifest(...)`
|
||||
# in chart.py matched neither `manifest` nor `mf`, so the module
|
||||
# went unlisted AND unscanned). One detector, one truth.
|
||||
try:
|
||||
reads, writes = keys_touched(path)
|
||||
except SyntaxError:
|
||||
continue
|
||||
if reads or writes:
|
||||
missing.append(rel)
|
||||
|
||||
for rel in missing:
|
||||
_fail(
|
||||
f"{rel} touches feedpak manifest keys but is not in READERS "
|
||||
f"({Path(__file__).name}) — its keys are going unchecked. Add it."
|
||||
)
|
||||
print(f" scanning {len(listed)} modules; readers-complete: {'FAILED' if missing else 'OK'}")
|
||||
return not missing
|
||||
|
||||
|
||||
def check_key_coverage(spec: Path) -> bool:
|
||||
"""Layer 1 — core must not read or write a manifest key the spec does not declare."""
|
||||
schema = json.loads((spec / "schemas" / "manifest.schema.json").read_text(encoding="utf-8"))
|
||||
declared = set(schema.get("properties") or {})
|
||||
if not declared:
|
||||
_fail("spec manifest.schema.json declares no properties — wrong path or bad checkout?")
|
||||
return False
|
||||
|
||||
reads: set[str] = set()
|
||||
writes: set[str] = set()
|
||||
for rel in READERS:
|
||||
path = REPO / rel
|
||||
if not path.exists():
|
||||
_fail(f"reader {rel} not found — was it renamed? Update READERS in {Path(__file__).name}.")
|
||||
return False
|
||||
try:
|
||||
r, w = keys_touched(path)
|
||||
except SyntaxError as e:
|
||||
# A module that doesn't parse can't be scanned — but it also can't
|
||||
# pass pytest, so this is belt-and-braces for a CI-legible message
|
||||
# rather than a traceback if this job runs first.
|
||||
_fail(f"could not scan {rel}: {e}")
|
||||
return False
|
||||
reads |= r
|
||||
writes |= w
|
||||
|
||||
exceptions = load_exceptions()
|
||||
ok = True
|
||||
|
||||
def _undeclared(keys: set[str]) -> list[str]:
|
||||
return sorted((keys - declared) - set(exceptions))
|
||||
|
||||
for key in _undeclared(reads):
|
||||
_fail(f"core reads manifest key '{key}', which the feedpak spec does not define. {FEP}")
|
||||
ok = False
|
||||
|
||||
for key in _undeclared(writes):
|
||||
_fail(
|
||||
f"core writes manifest key '{key}', which the feedpak spec does not define — that "
|
||||
f"puts non-spec surface into every pack we emit. {FEP}"
|
||||
)
|
||||
ok = False
|
||||
|
||||
# A stale exception is its own bug: it means the spec caught up and nobody
|
||||
# cleaned up, so the allowlist slowly becomes a place drift hides.
|
||||
touched = reads | writes
|
||||
for key, issue in exceptions.items():
|
||||
if key in declared:
|
||||
_fail(
|
||||
f"'{key}' is listed in {EXCEPTIONS_FILE.name} but the spec now declares it. "
|
||||
f"Remove the exception and close {issue}."
|
||||
)
|
||||
ok = False
|
||||
elif key not in touched:
|
||||
_fail(
|
||||
f"'{key}' is listed in {EXCEPTIONS_FILE.name} but core no longer reads or writes "
|
||||
f"it. Remove the exception."
|
||||
)
|
||||
ok = False
|
||||
|
||||
print(f" spec declares {len(declared)} keys; core reads {len(reads)}, writes {len(writes)}")
|
||||
if exceptions:
|
||||
print(f" grandfathered (tracked debt): {', '.join(sorted(exceptions))}")
|
||||
print(f" key-coverage: {'OK' if ok else 'FAILED'}")
|
||||
return ok
|
||||
|
||||
|
||||
def check_forward(spec: Path) -> bool:
|
||||
"""Layer 3 — core must ingest every example pack the spec ships."""
|
||||
examples_dir = spec / "examples"
|
||||
if not examples_dir.is_dir():
|
||||
_fail(f"{examples_dir} is missing — wrong path or bad checkout?")
|
||||
return False
|
||||
# rglob, not iterdir: the contract is "every example pack the spec ships", so
|
||||
# a pack nested under examples/<group>/ must not slip through.
|
||||
#
|
||||
# Deliberately NOT filtered by is_file(): a feedpak is dual-form — a zip
|
||||
# (`foo.feedpak`) *or* a directory (`foo.feedpak/`) — and the spec's own
|
||||
# examples ship as directories today. An is_file() guard here would silently
|
||||
# match zero packs. Matching on the suffix covers both forms, and rglob does
|
||||
# not smuggle in a pack's innards because files inside a pack don't carry a
|
||||
# pack suffix.
|
||||
examples = sorted(
|
||||
p for p in examples_dir.rglob("*")
|
||||
if p.suffix in (".feedpak", ".sloppak")
|
||||
)
|
||||
if not examples:
|
||||
_fail("spec ships no example packs — wrong path or bad checkout?")
|
||||
return False
|
||||
|
||||
sys.path.insert(0, str(REPO / "lib"))
|
||||
try:
|
||||
import sloppak # noqa: E402 (path must be set first — flat imports, no package)
|
||||
except Exception as e:
|
||||
_fail(
|
||||
f"could not import core's sloppak loader ({type(e).__name__}: {e}). "
|
||||
f"Are requirements.txt deps installed?"
|
||||
)
|
||||
return False
|
||||
|
||||
ok = True
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
cache = Path(tmp)
|
||||
for pack in examples:
|
||||
try:
|
||||
loaded = sloppak.load_song(pack.name, pack.parent, cache)
|
||||
except Exception as e:
|
||||
_fail(
|
||||
f"core failed to load the spec's own example pack {pack.name}: "
|
||||
f"{type(e).__name__}: {e}. A spec-valid pack must load."
|
||||
)
|
||||
ok = False
|
||||
continue
|
||||
if not loaded.song.arrangements:
|
||||
_fail(f"core loaded {pack.name} but found no arrangements")
|
||||
ok = False
|
||||
continue
|
||||
print(f" loaded {pack.name}: {len(loaded.song.arrangements)} arrangement(s)")
|
||||
print(f" forward: {'OK' if ok else 'FAILED'}")
|
||||
return ok
|
||||
|
||||
|
||||
def check_reverse(spec: Path) -> bool:
|
||||
"""Layer 4 — packs committed here must pass the spec's reference validator."""
|
||||
packs = sorted({p for g in PACK_GLOBS for p in REPO.glob(g)})
|
||||
if not packs:
|
||||
print(" reverse: no committed packs — skipped")
|
||||
return True
|
||||
|
||||
try:
|
||||
proc = subprocess.run(
|
||||
[sys.executable, str(spec / "tools" / "validate.py"), *[str(p) for p in packs]],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
# The validator takes seconds for all committed packs; a pathological
|
||||
# pack or validator bug must fail the job, not hang the runner until
|
||||
# the Actions-level timeout.
|
||||
timeout=300,
|
||||
)
|
||||
except subprocess.TimeoutExpired:
|
||||
_fail("the spec's reference validator did not finish within 300s — pathological pack or validator bug?")
|
||||
print(" reverse: FAILED")
|
||||
return False
|
||||
sys.stdout.write("".join(f" {ln}\n" for ln in proc.stdout.splitlines() if ln.strip()))
|
||||
if proc.returncode != 0:
|
||||
_fail(
|
||||
"a pack committed to this repo does not satisfy the feedpak spec "
|
||||
"(see the reference validator output above)."
|
||||
)
|
||||
if proc.stderr.strip():
|
||||
sys.stderr.write(proc.stderr)
|
||||
print(f" reverse: {'OK' if proc.returncode == 0 else 'FAILED'}")
|
||||
return proc.returncode == 0
|
||||
|
||||
|
||||
def main() -> int:
|
||||
ap = argparse.ArgumentParser(description=__doc__.splitlines()[0])
|
||||
ap.add_argument(
|
||||
"--spec",
|
||||
required=True,
|
||||
type=Path,
|
||||
help="path to a feedpak-spec checkout (CI checks out the spec repo's HEAD)",
|
||||
)
|
||||
ap.add_argument(
|
||||
"--baseline-exceptions",
|
||||
type=Path,
|
||||
help="the exceptions file as it exists on the base branch. Supplied by CI so the "
|
||||
"allowlist can be proven to have not grown. Omit for a local run.",
|
||||
)
|
||||
ap.add_argument(
|
||||
"--bootstrap-allowlist",
|
||||
action="store_true",
|
||||
help="the base branch has no exceptions file yet (this PR introduces the gate), so "
|
||||
"there is nothing to diff against. CI passes this only in that case.",
|
||||
)
|
||||
args = ap.parse_args()
|
||||
|
||||
spec = args.spec.resolve()
|
||||
if not (spec / "schemas" / "manifest.schema.json").exists():
|
||||
_fail(f"{spec} does not look like a feedpak-spec checkout")
|
||||
return 1
|
||||
|
||||
print("[1/4] key-coverage — core reads/writes only keys the spec declares")
|
||||
# Both run, always: a stale READERS list and an undeclared key are separate
|
||||
# failures, and reporting only the first would hide the second. Hence two
|
||||
# calls and an explicit `and` over the results, not a short-circuiting one.
|
||||
readers_ok = check_readers_complete()
|
||||
coverage_ok = check_key_coverage(spec)
|
||||
ok1 = readers_ok and coverage_ok
|
||||
print("[2/4] allowlist-closed — the grandfather list may shrink, never grow")
|
||||
ok2 = check_allowlist_closed(args.baseline_exceptions, args.bootstrap_allowlist)
|
||||
print("[3/4] forward — core ingests the spec's example packs")
|
||||
ok3 = check_forward(spec)
|
||||
print("[4/4] reverse — committed packs satisfy the reference validator")
|
||||
ok4 = check_reverse(spec)
|
||||
|
||||
if ok1 and ok2 and ok3 and ok4:
|
||||
print("\nfeedpak spec conformance: OK")
|
||||
return 0
|
||||
print("\nfeedpak spec conformance: FAILED")
|
||||
return 1
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
Reference in New Issue
Block a user