mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-11 03:09:57 +00:00
ci: resolve the allowlist baseline against the real base branch
The allowlist-closed diff hardcoded `origin main`, but ship-ci.yml also runs
this workflow for PRs into release/** and for pushes to release/**, where a
main baseline diffs against the wrong branch and can fail changes that have
nothing to do with the allowlist. It now resolves the base:
PR -> github.event.pull_request.base.ref (the branch it merges into)
push -> github.ref_name (the branch itself; its tip already contains the
change, so the diff is a no-op — enforcement happens at PR time)
Also from review, all documentation drift introduced by my own earlier
commits:
- The layer count said "three" in the module docstring, the workflow comment,
the docs, and the changelog. There are four (allowlist-closed was added).
- The changelog listed three scanned modules; there are five.
- The docs and changelog stated the rule for keys core *reads*, omitting
writes — which are equally gated, and land in every pack we emit.
- The CI summary line labelled grandfathered keys "pending spec", implying
adoption is the only resolution. For original_audio it is not: the fix is
removal. Relabelled "grandfathered (tracked debt)".
- feedpak-spec-exceptions.yml said an entry clears when core "stops reading"
the key; the rule is "no longer reads or writes".
- Replaced a bitwise `&` over two bools with two named results and an
explicit `and` — both checks must run (a stale READERS list and an
undeclared key are separate failures; short-circuiting would hide one), and
`&` reads like a typo.
Signed-off-by: topkoa <topkoa@gmail.com>
This commit is contained in:
+242
-231
@@ -1,231 +1,242 @@
|
||||
name: ci
|
||||
|
||||
# Runs only as a reusable workflow invoked by ship-ci.yml (for PRs into main
|
||||
# and release/**). It deliberately has no standalone pull_request trigger: a
|
||||
# direct run would publish unprefixed "<job>" checks, but the org rulesets
|
||||
# require the "ci / <job>" names produced when ship-ci.yml calls this workflow.
|
||||
on:
|
||||
workflow_call:
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: read
|
||||
checks: read
|
||||
|
||||
jobs:
|
||||
|
||||
test:
|
||||
name: test
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
- uses: actions/setup-python@v5
|
||||
with:
|
||||
python-version: '3.12'
|
||||
cache: 'pip'
|
||||
|
||||
- name: Guard against print() / traceback.print_exc() in server.py, lib/, and bundled plugin routes
|
||||
run: |
|
||||
# git grep: tracked files only — no .pyc / __pycache__ noise from
|
||||
# later pytest runs. Covers both audited patterns from #155 / #242.
|
||||
# `(^|[^A-Za-z0-9_])` anchor avoids matching suffixes like `myprint(`;
|
||||
# POSIX leaves `\b` undefined, so we use an explicit character class.
|
||||
hits=$(git grep -nE '(^|[^A-Za-z0-9_])(print|traceback\.print_exc)[[:space:]]*\(' \
|
||||
-- server.py lib/ \
|
||||
$(git ls-files 'plugins/*/routes.py') || true)
|
||||
if [ -n "$hits" ]; then
|
||||
echo "$hits"
|
||||
first=$(printf '%s\n' "$hits" | head -n1)
|
||||
file=$(printf '%s' "$first" | cut -d: -f1)
|
||||
line=$(printf '%s' "$first" | cut -d: -f2)
|
||||
echo "::error file=${file},line=${line}::print() or traceback.print_exc() found in server.py, lib/, or a bundled plugin routes.py. Use the feedBack logger (lib/logging_setup.py) — see issues #155 / #242."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
- name: Install dependencies
|
||||
run: |
|
||||
python -m pip install --upgrade pip
|
||||
pip install -r requirements.txt -r requirements-test.txt
|
||||
|
||||
- name: Run pytest
|
||||
run: pytest
|
||||
|
||||
- name: Run JS plugin-API tests
|
||||
run: node --test tests/js/*.test.js 'tests/plugins/*/js/*.test.js' 'plugins/*/tests/*.test.js'
|
||||
|
||||
tailwind-fresh:
|
||||
# Guard that the committed static/tailwind.min.css is in sync with source.
|
||||
# The Play CDN's runtime JIT was removed (feedBack-desktop#110); a prebuilt
|
||||
# stylesheet only contains classes the scanner saw at build time, so stale
|
||||
# CSS silently ships unstyled elements. Rebuild and fail on any diff.
|
||||
name: tailwind-fresh
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
- uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: '20'
|
||||
|
||||
- name: Rebuild Tailwind CSS
|
||||
run: bash scripts/build-tailwind.sh
|
||||
|
||||
- name: Verify committed static/tailwind.min.css is fresh
|
||||
run: |
|
||||
# Hard-fail (matches the print() guard convention) — do NOT auto-commit.
|
||||
if ! git diff --quiet -- static/tailwind.min.css; then
|
||||
echo "::error file=static/tailwind.min.css::static/tailwind.min.css is stale. Run 'bash scripts/build-tailwind.sh' and commit the regenerated file."
|
||||
git diff -- static/tailwind.min.css
|
||||
exit 1
|
||||
fi
|
||||
|
||||
manifest-validation:
|
||||
name: manifest-validation
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
- uses: actions/setup-python@v5
|
||||
with:
|
||||
python-version: '3.12'
|
||||
|
||||
- name: Validate plugin manifests
|
||||
run: |
|
||||
python - <<'EOF'
|
||||
import json, sys
|
||||
from pathlib import Path
|
||||
|
||||
errors = []
|
||||
manifests = sorted(Path("plugins").glob("*/plugin.json"))
|
||||
|
||||
for manifest in manifests:
|
||||
try:
|
||||
data = json.loads(manifest.read_text())
|
||||
except json.JSONDecodeError as e:
|
||||
errors.append(f"{manifest}: invalid JSON — {e}")
|
||||
continue
|
||||
for field in ("id", "name"):
|
||||
if field not in data:
|
||||
errors.append(f"{manifest}: missing required field '{field}'")
|
||||
pid = data.get("id", "")
|
||||
if pid and pid != pid.lower():
|
||||
errors.append(f"{manifest}: 'id' must be lowercase, got '{pid}'")
|
||||
# The plugin loader treats each plugins/<dir> as a Python module,
|
||||
# so the manifest 'id' must match its directory name.
|
||||
dirname = manifest.parent.name
|
||||
if pid and pid != dirname:
|
||||
errors.append(f"{manifest}: 'id' ({pid!r}) must match directory name ({dirname!r})")
|
||||
|
||||
if errors:
|
||||
for e in errors:
|
||||
print(f"::error::{e}")
|
||||
sys.exit(1)
|
||||
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. Three surface checks: core reads only
|
||||
# manifest keys the spec declares; 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.
|
||||
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: 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
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
repository: got-feedback/feedpak-spec
|
||||
ref: ${{ steps.spec.outputs.sha }}
|
||||
path: .feedpak-spec
|
||||
persist-credentials: false
|
||||
|
||||
- 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.
|
||||
git fetch --no-tags --depth=1 origin main
|
||||
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
|
||||
# scripts/build-tailwind.sh). max-lines WARNS (the 1,500-line size ratchet;
|
||||
# non-blocking), while import-x/no-unresolved + no-cycle HARD-ERROR on the
|
||||
# ES-module graphs the refactor produces. Exemptions: docs/size-exemptions.md.
|
||||
name: lint
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
# npm ci runs third-party postinstall scripts; don't leave the token in
|
||||
# git config for them (this job never pushes).
|
||||
with:
|
||||
persist-credentials: false
|
||||
|
||||
- uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: '20'
|
||||
|
||||
- name: Install dependencies
|
||||
run: npm ci
|
||||
|
||||
- name: ESLint (size norm + module hygiene)
|
||||
run: npm run lint
|
||||
name: ci
|
||||
|
||||
# Runs only as a reusable workflow invoked by ship-ci.yml (for PRs into main
|
||||
# and release/**). It deliberately has no standalone pull_request trigger: a
|
||||
# direct run would publish unprefixed "<job>" checks, but the org rulesets
|
||||
# require the "ci / <job>" names produced when ship-ci.yml calls this workflow.
|
||||
on:
|
||||
workflow_call:
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: read
|
||||
checks: read
|
||||
|
||||
jobs:
|
||||
|
||||
test:
|
||||
name: test
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
- uses: actions/setup-python@v5
|
||||
with:
|
||||
python-version: '3.12'
|
||||
cache: 'pip'
|
||||
|
||||
- name: Guard against print() / traceback.print_exc() in server.py, lib/, and bundled plugin routes
|
||||
run: |
|
||||
# git grep: tracked files only — no .pyc / __pycache__ noise from
|
||||
# later pytest runs. Covers both audited patterns from #155 / #242.
|
||||
# `(^|[^A-Za-z0-9_])` anchor avoids matching suffixes like `myprint(`;
|
||||
# POSIX leaves `\b` undefined, so we use an explicit character class.
|
||||
hits=$(git grep -nE '(^|[^A-Za-z0-9_])(print|traceback\.print_exc)[[:space:]]*\(' \
|
||||
-- server.py lib/ \
|
||||
$(git ls-files 'plugins/*/routes.py') || true)
|
||||
if [ -n "$hits" ]; then
|
||||
echo "$hits"
|
||||
first=$(printf '%s\n' "$hits" | head -n1)
|
||||
file=$(printf '%s' "$first" | cut -d: -f1)
|
||||
line=$(printf '%s' "$first" | cut -d: -f2)
|
||||
echo "::error file=${file},line=${line}::print() or traceback.print_exc() found in server.py, lib/, or a bundled plugin routes.py. Use the feedBack logger (lib/logging_setup.py) — see issues #155 / #242."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
- name: Install dependencies
|
||||
run: |
|
||||
python -m pip install --upgrade pip
|
||||
pip install -r requirements.txt -r requirements-test.txt
|
||||
|
||||
- name: Run pytest
|
||||
run: pytest
|
||||
|
||||
- name: Run JS plugin-API tests
|
||||
run: node --test tests/js/*.test.js 'tests/plugins/*/js/*.test.js' 'plugins/*/tests/*.test.js'
|
||||
|
||||
tailwind-fresh:
|
||||
# Guard that the committed static/tailwind.min.css is in sync with source.
|
||||
# The Play CDN's runtime JIT was removed (feedBack-desktop#110); a prebuilt
|
||||
# stylesheet only contains classes the scanner saw at build time, so stale
|
||||
# CSS silently ships unstyled elements. Rebuild and fail on any diff.
|
||||
name: tailwind-fresh
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
- uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: '20'
|
||||
|
||||
- name: Rebuild Tailwind CSS
|
||||
run: bash scripts/build-tailwind.sh
|
||||
|
||||
- name: Verify committed static/tailwind.min.css is fresh
|
||||
run: |
|
||||
# Hard-fail (matches the print() guard convention) — do NOT auto-commit.
|
||||
if ! git diff --quiet -- static/tailwind.min.css; then
|
||||
echo "::error file=static/tailwind.min.css::static/tailwind.min.css is stale. Run 'bash scripts/build-tailwind.sh' and commit the regenerated file."
|
||||
git diff -- static/tailwind.min.css
|
||||
exit 1
|
||||
fi
|
||||
|
||||
manifest-validation:
|
||||
name: manifest-validation
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
- uses: actions/setup-python@v5
|
||||
with:
|
||||
python-version: '3.12'
|
||||
|
||||
- name: Validate plugin manifests
|
||||
run: |
|
||||
python - <<'EOF'
|
||||
import json, sys
|
||||
from pathlib import Path
|
||||
|
||||
errors = []
|
||||
manifests = sorted(Path("plugins").glob("*/plugin.json"))
|
||||
|
||||
for manifest in manifests:
|
||||
try:
|
||||
data = json.loads(manifest.read_text())
|
||||
except json.JSONDecodeError as e:
|
||||
errors.append(f"{manifest}: invalid JSON — {e}")
|
||||
continue
|
||||
for field in ("id", "name"):
|
||||
if field not in data:
|
||||
errors.append(f"{manifest}: missing required field '{field}'")
|
||||
pid = data.get("id", "")
|
||||
if pid and pid != pid.lower():
|
||||
errors.append(f"{manifest}: 'id' must be lowercase, got '{pid}'")
|
||||
# The plugin loader treats each plugins/<dir> as a Python module,
|
||||
# so the manifest 'id' must match its directory name.
|
||||
dirname = manifest.parent.name
|
||||
if pid and pid != dirname:
|
||||
errors.append(f"{manifest}: 'id' ({pid!r}) must match directory name ({dirname!r})")
|
||||
|
||||
if errors:
|
||||
for e in errors:
|
||||
print(f"::error::{e}")
|
||||
sys.exit(1)
|
||||
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. Pinned by SHA in .feedpak-spec-ref so a change over
|
||||
# there can't redden an unrelated PR here.
|
||||
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: 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
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
repository: got-feedback/feedpak-spec
|
||||
ref: ${{ steps.spec.outputs.sha }}
|
||||
path: .feedpak-spec
|
||||
persist-credentials: false
|
||||
|
||||
- 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
|
||||
# scripts/build-tailwind.sh). max-lines WARNS (the 1,500-line size ratchet;
|
||||
# non-blocking), while import-x/no-unresolved + no-cycle HARD-ERROR on the
|
||||
# ES-module graphs the refactor produces. Exemptions: docs/size-exemptions.md.
|
||||
name: lint
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
# npm ci runs third-party postinstall scripts; don't leave the token in
|
||||
# git config for them (this job never pushes).
|
||||
with:
|
||||
persist-credentials: false
|
||||
|
||||
- uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: '20'
|
||||
|
||||
- name: Install dependencies
|
||||
run: npm ci
|
||||
|
||||
- name: ESLint (size norm + module hygiene)
|
||||
run: npm run lint
|
||||
|
||||
Reference in New Issue
Block a user