diff --git a/.github/workflows/fork-audit.yml b/.github/workflows/fork-audit.yml new file mode 100644 index 00000000..75de2bd3 --- /dev/null +++ b/.github/workflows/fork-audit.yml @@ -0,0 +1,76 @@ +name: fork-audit + +# Fork-only checks — see docs/fork-maintenance.md. Deliberately its own file +# (not a job added to upstream's ci.yml/ship-ci.yml) so pulling upstream +# changes to those workflows never conflicts with this policy's own +# enforcement of "keep core edits small and isolated". + +on: + pull_request: + branches: [main] + +permissions: + contents: read + pull-requests: read + +jobs: + + core-commit-labeling: + # Fails only on commits THIS PR introduces (relative to its base), so + # pre-existing history is never retroactively flagged. + name: core-commit-labeling + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Flag unlabeled core-touching commits introduced by this PR + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + commits=$(git rev-list "$BASE_SHA".."$HEAD_SHA") + + fail=0 + for sha in $commits; do + files=$(git show --name-only --pretty=format:"" "$sha") + core_hit=$(printf '%s\n' "$files" | grep -Ev '^(plugins/|docs/|tests/|scripts/|specs/|\.specify/)|^(CHANGELOG\.md|VERSION)$' || true) + [ -z "$core_hit" ] && continue + + subject=$(git show -s --format=%s "$sha") + if ! printf '%s' "$subject" | grep -qE '^(core|hook|sync)(\([^)]*\))?:'; then + echo "::error::Commit $sha ('$subject') touches core path(s) but its subject isn't prefixed core:/hook:/sync: — see docs/fork-maintenance.md (Rule 2). Files touched: $(printf '%s' "$core_hit" | tr '\n' ' ')" + fail=1 + fi + done + exit $fail + + upstream-drift: + # Advisory only — never fails the PR. Nudges toward Rule 4 (sync often, + # in small increments) instead of letting drift build up silently. + name: upstream-drift + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Compare PR base against got-feedback/feedBack:main + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: | + if ! git fetch --no-tags https://github.com/got-feedback/feedBack.git main:refs/remotes/canonical-upstream/main; then + echo "::warning::Unable to fetch got-feedback/feedBack:main; skipping upstream drift check." + exit 0 + fi + if ! behind=$(git rev-list --count "$BASE_SHA"..canonical-upstream/main); then + echo "::warning::Unable to calculate upstream drift; skipping check." + exit 0 + fi + echo "PR base is $behind commit(s) behind got-feedback/feedBack:main." + if [ "$behind" -gt 50 ]; then + echo "::warning::This branch's base is $behind commits behind got-feedback/feedBack:main. Consider running scripts/fork-sync.sh before adding more fork-only work (docs/fork-maintenance.md, Rule 4)." + fi diff --git a/docs/fork-maintenance.md b/docs/fork-maintenance.md new file mode 100644 index 00000000..248739ac --- /dev/null +++ b/docs/fork-maintenance.md @@ -0,0 +1,140 @@ +# Fork Maintenance Policy + +This repo (`get-flashbacks/feedBack`) is a personal downstream fork of the +canonical project, [`got-feedback/feedBack`](https://github.com/got-feedback/feedBack). +The goal of this document is to keep pulling upstream changes cheap forever, +instead of expensive once a year. + +Every manual edit to a file upstream also touches is **maintenance debt**: the +next `git merge`/`git rebase` from upstream has to reconcile it by hand. This +policy exists to keep that debt near zero. + +## Priority matrix + +When deciding how to make a change, work top to bottom — stop at the first +row that fits: + +| Priority | What it is | Example | +|---|---|---| +| **P0 — Upstream sync** | Pulling `got-feedback/feedBack:main` in, resolving conflicts | `scripts/fork-sync.sh` | +| **P1 — Hook injection** | A small, generic extension point added to core so plugin/fork logic can live outside core | 2-line event emit + a plugin file with the real logic | +| **P2 — Plugin-isolated feature** | Anything that lives entirely under `plugins//` | A new plugin directory | +| **P3 — Direct core edit** | Business logic written straight into `server.py`, `lib/`, `static/`, `Dockerfile`, etc. | Avoid; only for real upstream-worthy bug fixes | + +**Rule of thumb:** if a core edit is more than a few lines of business logic +(not a hook call, not a one-line bug fix), it almost always belongs in P1 or +P2 instead. + +## The four rules + +1. **Hook injection over inline edits.** If a plugin needs core to do + something it doesn't support yet, don't write the feature inside the core + file. Add the smallest possible hook/event/extension point to core, and + put the actual logic in the plugin. This repo already has a rich contract + for this — see `CLAUDE.md`'s "Plugin System" and "Plugin Best Practices" + sections (`context["log"]`, `load_sibling`, library providers, the + `setRenderer` / overlay / note-state-provider / chart-transform-provider + contracts, `window.feedBack.emit/on`, keyboard shortcut scopes, pane + registration, fader registration). Reach for one of those before touching + a core file. Real bug fixes to core (not feature logic) are the one + legitimate exception — see the P3 note below. + +2. **Segregate commits.** Every commit that touches a core path (see + "What counts as core" below) must start its subject line with one of: + - `core:` — a direct core edit (P3; keep it small, and prefer to also open + it as a PR upstream, see Rule 3) + - `hook:` — adding/expanding an extension point in core so a plugin can do + the rest (P1) + - `sync:` — merging/rebasing upstream changes in (P0) + + Commits that touch only `plugins/**` don't need a prefix (that's the + normal case and needs no special handling). This labeling is what lets + you `git log --grep '^core:'` or cherry-pick your minimal core diff onto a + fresh upstream tag when things diverge badly. + +3. **Upstream PRs retire debt.** Whenever a `core:`/`hook:` commit lands + here, ask: *is this useful to anyone else running FeedBack?* If yes, open + a PR against `got-feedback/feedBack:main` (see `CONTRIBUTING.md` for the + DCO/licensing requirements). Once it merges upstream, your local edit + becomes redundant on the next sync and your merge debt for that file + drops to zero. Track open upstream PRs in commit trailers, e.g. + `Upstream-PR: got-feedback/feedBack#1234`. + +4. **Sync before you build.** Before starting new fork-only work, pull + upstream first (`scripts/fork-sync.sh`). Small, frequent syncs (weekly) + are far cheaper than one large one. Treat an available upstream update or + security fix as higher priority than a new personal feature. + +## What counts as "core" + +Everything **except**: + +``` +plugins/** +docs/** +tests/** +scripts/** +specs/** +.specify/** +CHANGELOG.md +VERSION +``` + +That includes `server.py`, `main.py`, `lib/**`, `static/**`, `Dockerfile`, +`docker-compose*.yml`, `.github/workflows/**`, `pyproject.toml`, +`requirements*.txt`, `package.json`, etc. + +## Decision tree + +``` +Does upstream have an update or security fix? + -> P0: sync first (scripts/fork-sync.sh), before anything else. + +Does a plugin need something from core that isn't exposed? + -> P1: add a minimal hook/event to core (commit: "hook: ..."). + Consider a PR upstream if it's broadly useful (Rule 3). + +Is it a feature/fix specific to your own workflow? + -> P2: build it entirely inside plugins//. Zero core impact. + +Tempted to edit a core file directly for convenience? + -> P3 / avoid. If it's truly a bug fix (not new logic), it's OK, but + label the commit "core:" and consider sending it upstream — a real + bug fix is exactly the kind of thing got-feedback/feedBack wants back. +``` + +## Enforcement + +Policy that isn't checked erodes. Two mechanisms enforce this one: + +- **`.github/workflows/fork-audit.yml`** — a fork-only CI workflow (does not + touch or replace upstream's `ci.yml`/`ship-ci.yml`, to avoid creating a + conflict in the exact file this policy is trying to keep conflict-free): + - `core-commit-labeling` fails a PR if any commit it introduces (relative + to the PR base) touches a core path without a `core:`/`hook:`/`sync:` + prefix. + - `upstream-drift` is advisory-only: reports how many commits behind + `got-feedback/feedBack:main` this branch is, and warns (without failing) + once that count crosses a threshold, as a nudge for Rule 4. +- **`scripts/fork-sync.sh`** — sets up the `upstream` remote if missing and + fetches/reports drift, so P0 syncs are a one-command habit rather than a + thing you have to remember how to do. + +## Current state (as of the last audit) + +Recorded here so the next audit has a baseline to diff against, not as a +permanent record — update or delete this section once it's stale. + +- No `upstream` remote was configured in this fork as of 2026-08-02, despite + `CLAUDE.md` documenting the `origin`/`upstream` split as the intended + convention. `scripts/fork-sync.sh` fixes this on first run. +- Three fork-only commits existed on `main` at audit time, all direct core + edits with no `core:`/`hook:` labeling (predating this policy, so not + retroactively flagged by CI): + - `830a708` — Guitar Pro strum-direction import (`lib/gp2rs.py`, + `lib/gp2rs_gpx.py`). Real feature logic in a converter core owns; a + reasonable upstream PR candidate under Rule 3. + - `5b443e9` — null-check fix in `lib/routers/ws_highway.py`. A genuine bug + fix (P3's legitimate exception) — a good candidate to send upstream. + - `b9e7c3d` — `Dockerfile` FFmpeg asset fix. Build-only, low conflict risk, + also a reasonable upstream PR candidate. diff --git a/scripts/fork-sync.sh b/scripts/fork-sync.sh new file mode 100755 index 00000000..e2415e4f --- /dev/null +++ b/scripts/fork-sync.sh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +# Personal-fork sync helper — see docs/fork-maintenance.md (Rule 4: sync +# often, in small increments, before starting new fork-only work). +# +# Sets up the `upstream` remote (got-feedback/feedBack) if it's missing, +# fetches it, reports how far the current branch has drifted, and — unless +# --report-only is passed — merges upstream/main in. +set -euo pipefail + +UPSTREAM_URL="https://github.com/got-feedback/feedBack.git" +REPORT_ONLY=0 + +for arg in "$@"; do + case "$arg" in + --report-only) REPORT_ONLY=1 ;; + *) + echo "Usage: $0 [--report-only]" >&2 + exit 1 + ;; + esac +done + +if upstream_url=$(git remote get-url upstream 2>/dev/null); then + if [ "$upstream_url" != "$UPSTREAM_URL" ]; then + echo "Refusing to sync: 'upstream' remote does not point to the canonical" \ + "$UPSTREAM_URL. Fix or remove the existing remote and re-run." >&2 + exit 1 + fi +else + echo "No 'upstream' remote found — adding $UPSTREAM_URL" + git remote add upstream "$UPSTREAM_URL" +fi + +echo "Fetching upstream/main..." +git fetch upstream main:refs/remotes/upstream/main + +behind=$(git rev-list --count HEAD..upstream/main) +ahead=$(git rev-list --count upstream/main..HEAD) + +echo "This branch is $ahead commit(s) ahead and $behind commit(s) behind upstream/main." + +if [ "$behind" -eq 0 ]; then + echo "Already up to date with upstream/main." + exit 0 +fi + +if [ "$REPORT_ONLY" -eq 1 ]; then + echo "--report-only passed — not merging. Run without it to merge upstream/main in." + exit 0 +fi + +if [ "$behind" -gt 50 ]; then + echo "Warning: $behind commits behind. Consider syncing more often (Rule 4) —" \ + "this merge may involve more conflict resolution than usual." >&2 +fi + +echo "Merging upstream/main..." +git merge upstream/main -m "sync: merge upstream/main ($behind commit(s))"