From adb863c565288ec7c5b6afb5d9995b5014b8e170 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 19:39:41 +0000 Subject: [PATCH 1/4] core: add fork maintenance policy, sync helper, and audit CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Personal-fork governance: a priority matrix (sync > hooks > plugin isolation > direct core edits), commit-labeling rules, and a decision tree, plus the tooling to actually enforce it — scripts/fork-sync.sh sets up the missing upstream remote and reports/merges drift, and .github/workflows/fork-audit.yml fails new PR commits that touch core paths without a core:/hook:/sync: label (existing history is left alone) and warns when the branch falls far behind got-feedback/feedBack:main. Kept as new files rather than edits to ci.yml/ship-ci.yml/CLAUDE.md so the policy doesn't itself create the exact conflicts it's meant to avoid. --- .github/workflows/fork-audit.yml | 68 +++++++++++++++ docs/fork-maintenance.md | 140 +++++++++++++++++++++++++++++++ scripts/fork-sync.sh | 52 ++++++++++++ 3 files changed, 260 insertions(+) create mode 100644 .github/workflows/fork-audit.yml create mode 100644 docs/fork-maintenance.md create mode 100755 scripts/fork-sync.sh diff --git a/.github/workflows/fork-audit.yml b/.github/workflows/fork-audit.yml new file mode 100644 index 00000000..96f02d89 --- /dev/null +++ b/.github/workflows/fork-audit.yml @@ -0,0 +1,68 @@ +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@v4 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Flag unlabeled core-touching commits introduced by this PR + run: | + BASE_SHA="${{ github.event.pull_request.base.sha }}" + HEAD_SHA="${{ github.event.pull_request.head.sha }}" + 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@v4 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Compare PR base against got-feedback/feedBack:main + run: | + git fetch --no-tags https://github.com/got-feedback/feedBack.git main:refs/remotes/canonical-upstream/main + BASE_SHA="${{ github.event.pull_request.base.sha }}" + behind=$(git rev-list --count "$BASE_SHA"..canonical-upstream/main) + 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..bdab03fe --- /dev/null +++ b/scripts/fork-sync.sh @@ -0,0 +1,52 @@ +#!/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 ! git remote get-url upstream >/dev/null 2>&1; then + echo "No 'upstream' remote found — adding $UPSTREAM_URL" + git remote add upstream "$UPSTREAM_URL" +fi + +echo "Fetching upstream/main..." +git fetch 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))" From bee4c7a1e09352c8ecb5cf0b0b3f0bcfb2758fea Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 19:48:45 +0000 Subject: [PATCH 2/4] =?UTF-8?q?core:=20address=20CodeRabbit=20review=20?= =?UTF-8?q?=E2=80=94=20pin=20checkout,=20validate=20upstream=20remote?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pin both actions/checkout uses in fork-audit.yml to the immutable commit SHA (v4.4.0) instead of the mutable @v4 tag. fork-sync.sh: validate a pre-existing 'upstream' remote's URL before fetching/merging, instead of trusting any remote named 'upstream'. An unrelated remote with that name would otherwise get silently fetched and merged into the branch. --- .github/workflows/fork-audit.yml | 4 ++-- scripts/fork-sync.sh | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/fork-audit.yml b/.github/workflows/fork-audit.yml index 96f02d89..b34072be 100644 --- a/.github/workflows/fork-audit.yml +++ b/.github/workflows/fork-audit.yml @@ -21,7 +21,7 @@ jobs: name: core-commit-labeling runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: fetch-depth: 0 persist-credentials: false @@ -52,7 +52,7 @@ jobs: name: upstream-drift runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: fetch-depth: 0 persist-credentials: false diff --git a/scripts/fork-sync.sh b/scripts/fork-sync.sh index bdab03fe..d189e8d3 100755 --- a/scripts/fork-sync.sh +++ b/scripts/fork-sync.sh @@ -20,7 +20,13 @@ for arg in "$@"; do esac done -if ! git remote get-url upstream >/dev/null 2>&1; then +if upstream_url=$(git remote get-url upstream 2>/dev/null); then + if [ "$upstream_url" != "$UPSTREAM_URL" ]; then + echo "Refusing to sync: 'upstream' remote points to $upstream_url, not $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 From 0c2ebeec4e20f0033659c9ad2173c72529779744 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 19:54:52 +0000 Subject: [PATCH 3/4] core: fix credential leak, fail-safety, and fetch-refspec issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scripts/fork-sync.sh: - Don't print the mismatched 'upstream' remote URL to stderr — it could contain embedded credentials (https://user:token@...). Report only that it isn't canonical. - Fetch explicitly into refs/remotes/upstream/main instead of relying on the default refspec, so a pre-existing 'upstream' remote with a customized fetch refspec can't leave upstream/main stale. fork-audit.yml upstream-drift job: - GitHub Actions run steps default to `bash -eo pipefail`, so a failed fetch/rev-list would fail the job despite it being documented as advisory-only. Handle both failures explicitly and exit 0 with a warning instead of hard-failing on a transient network error. --- .github/workflows/fork-audit.yml | 10 ++++++++-- scripts/fork-sync.sh | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/fork-audit.yml b/.github/workflows/fork-audit.yml index b34072be..02040e1b 100644 --- a/.github/workflows/fork-audit.yml +++ b/.github/workflows/fork-audit.yml @@ -59,9 +59,15 @@ jobs: - name: Compare PR base against got-feedback/feedBack:main run: | - git fetch --no-tags https://github.com/got-feedback/feedBack.git main:refs/remotes/canonical-upstream/main + 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 BASE_SHA="${{ github.event.pull_request.base.sha }}" - behind=$(git rev-list --count "$BASE_SHA"..canonical-upstream/main) + 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)." diff --git a/scripts/fork-sync.sh b/scripts/fork-sync.sh index d189e8d3..e2415e4f 100755 --- a/scripts/fork-sync.sh +++ b/scripts/fork-sync.sh @@ -22,8 +22,8 @@ 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 points to $upstream_url, not $UPSTREAM_URL." \ - "Fix or remove the existing remote and re-run." >&2 + 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 @@ -32,7 +32,7 @@ else fi echo "Fetching upstream/main..." -git fetch 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) From 5a2c6c0680741d9e3c36c4bc8dd192f6166cc7b7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 19:59:57 +0000 Subject: [PATCH 4/4] core: pass PR SHAs through env instead of template interpolation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit github.event.pull_request.{base,head}.sha were interpolated directly into run: shell blocks. zizmor flags this as template-injection regardless of whether the specific field is attacker-controlled — pass them through env: and reference as shell variables instead, per GitHub's Actions hardening guidance. --- .github/workflows/fork-audit.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/fork-audit.yml b/.github/workflows/fork-audit.yml index 02040e1b..75de2bd3 100644 --- a/.github/workflows/fork-audit.yml +++ b/.github/workflows/fork-audit.yml @@ -27,9 +27,10 @@ jobs: 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: | - BASE_SHA="${{ github.event.pull_request.base.sha }}" - HEAD_SHA="${{ github.event.pull_request.head.sha }}" commits=$(git rev-list "$BASE_SHA".."$HEAD_SHA") fail=0 @@ -58,12 +59,13 @@ jobs: 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 - BASE_SHA="${{ github.event.pull_request.base.sha }}" if ! behind=$(git rev-list --count "$BASE_SHA"..canonical-upstream/main); then echo "::warning::Unable to calculate upstream drift; skipping check." exit 0