test fixes and improvements

.github/workflows/ci-apply.yml

@@ -80,9 +80,40 @@ jobs:
             exit 1
           }
 
-          gh api "repos/${REPO}/commits/${RUN_HEAD_SHA}/pulls" > pulls.json
-          [ "$(jq 'length' pulls.json)" = "1" ] \
-            || skip "Not exactly one PR associated with ${RUN_HEAD_SHA}; skipping."
+          # The sha -> PR association is eventually consistent: a run that
+          # starts seconds after the contributor's push can legitimately see
+          # an empty list. Poll before giving up.
+          #
+          # Both terminal outcomes below FAIL rather than `skip`. `skip` exits
+          # 0, which paints the whole job green - and a green PR Apply that
+          # pushed nothing is exactly how an unstamped branch merged to main
+          # and broke it (PR #479, reverted in #485). `skip` is reserved for
+          # states that self-heal (head moved -> a new run follows) or are
+          # none of our business (PR closed, not targeting main). "I could not
+          # identify the PR" is neither: it means the fixups were dropped on
+          # the floor, and that must be visible.
+          for attempt in 1 2 3 4 5; do
+            gh api "repos/${REPO}/commits/${RUN_HEAD_SHA}/pulls" > pulls.json
+            N_PULLS="$(jq 'length' pulls.json)"
+            [ "$N_PULLS" = "0" ] || break
+            echo "No PR associated with ${RUN_HEAD_SHA} yet (attempt ${attempt}/5); retrying in 10s."
+            sleep 10
+          done
+
+          if [ "$N_PULLS" = "0" ]; then
+            echo "No PR associated with ${RUN_HEAD_SHA} after 5 attempts." >&2
+            echo "Formatting/metadata fixups were NOT applied to the branch." >&2
+            exit 1
+          fi
+
+          # More than one PR sharing a head sha is the ambiguity HARD RULE 3
+          # exists for: pushing would risk writing to the wrong branch. Fail
+          # closed, but fail loudly - silence here is what bit us before.
+          if [ "$N_PULLS" != "1" ]; then
+            echo "${N_PULLS} PRs share head ${RUN_HEAD_SHA}; cannot resolve which one this run belongs to." >&2
+            echo "Failing closed: nothing was applied. A maintainer must apply the fixups manually." >&2
+            exit 1
+          fi
 
           PR_NUMBER="$(jq -r '.[0].number' pulls.json)"
           [[ "$PR_NUMBER" =~ ^[0-9]+$ ]] || skip "Bad PR number; skipping."
@@ -231,6 +262,7 @@ jobs:
           git apply --check "$PATCH"
 
       - name: Apply fixups patch and commit
+        id: fixups
         if: steps.pr.outputs.proceed == '1'
         working-directory: pr
         env:
@@ -260,9 +292,11 @@ jobs:
           if ! git diff --cached --quiet; then
             git commit -m "[ci] apply-plugin-metadata-and-formatting"
             git push "https://x-access-token:${GH_TOKEN}@github.com/${FORK}.git" "HEAD:${HEAD_REF}"
+            echo "pushed=1" >> "$GITHUB_OUTPUT"
           fi
 
       - name: Apply Version Metadata using the TRUSTED script only (never the fork's copy)
+        id: vermeta
         if: steps.pr.outputs.proceed == '1'
         working-directory: pr
         env:
@@ -280,7 +314,52 @@ jobs:
           if ! git diff --cached --quiet; then
             git commit -m "[ci] apply-version-metadata"
             git push "https://x-access-token:${GH_TOKEN}@github.com/${FORK}.git" "HEAD:${HEAD_REF}"
+            echo "pushed=1" >> "$GITHUB_OUTPUT"
+          fi
+
+      # THE MERGE GATE. PR Check is not one: it GENERATES the metadata into its
+      # own workspace and then tests that generated tree, so it goes green on a
+      # branch whose committed manifests are still empty. That is precisely how
+      # #479 merged and broke main - Check was green, Apply had silently
+      # skipped, and nothing anywhere asserted the branch was actually stamped.
+      #
+      # The invariant that does hold: a correctly-applied PR is a FIXPOINT -
+      # re-running the pipeline over it produces no changes. So this reports
+      # success only when both steps above found nothing left to push. When
+      # they did push, the new head sha retriggers Check -> Apply, and that run
+      # sets success on the sha a maintainer would actually merge.
+      #
+      # Make `metadata-fixpoint` a required status check on main. Then every
+      # path that leaves fixups unapplied - a skip, a hard failure, a crash -
+      # leaves the status unset and the PR unmergeable, instead of green.
+      - name: Report metadata fixpoint status
+        if: always() && steps.pr.outputs.proceed == '1'
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          REPO: ${{ github.repository }}
+          HEAD_SHA: ${{ steps.pr.outputs.head_sha }}
+          FIXUPS_PUSHED: ${{ steps.fixups.outputs.pushed }}
+          VERMETA_PUSHED: ${{ steps.vermeta.outputs.pushed }}
+          FIXUPS_RESULT: ${{ steps.fixups.outcome }}
+          VERMETA_RESULT: ${{ steps.vermeta.outcome }}
+          RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
+        run: |
+          set -euo pipefail
+          if [ "$FIXUPS_RESULT" != "success" ] || [ "$VERMETA_RESULT" != "success" ]; then
+            STATE="failure"
+            DESC="Formatting/metadata could not be applied; see the PR Apply run."
+          elif [ -n "$FIXUPS_PUSHED" ] || [ -n "$VERMETA_PUSHED" ]; then
+            STATE="failure"
+            DESC="Formatting/metadata was just applied; waiting for the re-run on the new commit."
+          else
+            STATE="success"
+            DESC="Formatting and generated metadata are up to date on this commit."
           fi
+          gh api --method POST "repos/${REPO}/statuses/${HEAD_SHA}" \
+            -f state="$STATE" \
+            -f context="metadata-fixpoint" \
+            -f description="$DESC" \
+            -f target_url="$RUN_URL"
 
       - name: On mechanical failure, notify the contributor
         if: failure() && steps.pr.outputs.pr_number != ''

.github/workflows/ci-check.yml

@@ -17,6 +17,15 @@ name: PR Check
 # DO NOT switch this to `pull_request_target` and do not add `permissions:
 # contents: write` here "to save a round trip" - that reintroduces the
 # pwn-request hole this split exists to close.
+#
+# THIS JOB IS NOT A MERGE GATE. It GENERATES the formatting and metadata into
+# its own workspace and then tests THAT tree, so it goes green on a branch
+# whose committed manifests are still empty - which is exactly how #479 merged
+# and broke main. Do not add a "patch must be empty" check here either: the
+# first run on any real PR legitimately produces a patch, and failing would
+# stop ci-apply.yml (`conclusion == 'success'`) from ever applying it. The gate
+# is the `metadata-fixpoint` commit status that ci-apply.yml sets; make that
+# the required check on main.
 
 on:
   pull_request:

.github/workflows/ci.yml

@@ -7,6 +7,26 @@ name: CI
 # here unmodified/strict against real, permanent git history (unlike
 # ci-check.yml, which can't yet resolve a commit sha for a brand-new plugin
 # version and runs leniently instead).
+#
+# It must run the SAME metadata pipeline as the PR path (ci-check.yml ->
+# ci-apply.yml): autopep8, then auto_apply_plugin_metadata.py, then
+# auto_apply_version_metadata.py. Content can reach main without ever going
+# through the PR workflows - a direct maintainer push, or a merge of a PR
+# whose checks failed - so a gap here is not covered anywhere else.
+#
+# Concretely, without the plugin-metadata step a push that bumps a plugin's
+# `plugman` version lands the new bytes with no matching manifest entry. The
+# manifest goes on advertising the OLD version's md5sum, so the in-game
+# manager's checksum check rejects every download of that plugin, and
+# test_latest_version fails on this and every later run. That is what 8628c97
+# (finder 1.0 -> 4.1) did to main; it stayed broken until PR #485 reverted the
+# whole change, which restored the old bytes rather than recording the new
+# ones.
+#
+# The auto-commit steps below push with the default GITHUB_TOKEN, and GitHub
+# does not start a workflow run for those pushes (there is no ci.yml run for
+# b1be5d5, the "[ci] apply-formatting" commit). So this job sees each push
+# exactly once and does not re-enter itself.
 
 on:
   push:
@@ -44,6 +64,61 @@ jobs:
         with:
           commit_message: "[ci] apply-formatting"
 
+      # Resolved BEFORE any of the auto-commit steps below rewrite HEAD, so it
+      # stays the tip main had when this push arrived. github.event.before is
+      # absent/zeroed on a branch's first push and dangling after a force
+      # push, hence the fallbacks; HEAD^ is the pushed head's first parent,
+      # which for a merge commit is the previous main tip.
+      - name: Resolve push base
+        env:
+          BEFORE_SHA: ${{ github.event.before }}
+        run: |
+          set -euo pipefail
+          BASE="$BEFORE_SHA"
+          if [ -z "$BASE" ] || ! git cat-file -e "${BASE}^{commit}" 2>/dev/null; then
+            # 4b825dc.. is git's empty tree - diffs as "everything is new" and
+            # simply fails to resolve as a manifest ref, which the metadata
+            # script handles by falling back to origin/main.
+            BASE="$(git rev-parse HEAD^ 2>/dev/null || echo 4b825dc642cb6eb9a060e54bf8d69288fbee4904)"
+          fi
+          echo "$BASE" > "${RUNNER_TEMP}/base_sha.txt"
+          echo "Push base: $BASE"
+
+      # The counterpart of ci-check.yml's plugin-metadata step. Diffed against
+      # HEAD (not the push head) so plugin files that only the autopep8 commit
+      # above touched are included - their bytes changed, so their checksums
+      # must be recomputed too.
+      #
+      # PLUGMAN_BASE_REF is REQUIRED here, for the same reason ci-check.yml
+      # sets it. It makes "already published" mean the manifest as of the push
+      # base, rather than the working tree. On the ordinary path - merging a
+      # PR - the merge brings the version bump AND the manifest entry that
+      # ci-apply.yml already stamped onto the PR branch, so the working tree
+      # here ALREADY lists the version being pushed. Comparing against that
+      # would make the script raise "Version cant be lower or equal" on every
+      # single merge. Compared against the base, the version is correctly seen
+      # as new, the entry is found already stamped with a matching md5sum, and
+      # the step is a no-op.
+      #
+      # This step FAILING is a feature: it means a plugin's bytes changed with
+      # no version bump, which would leave every existing install unable to
+      # verify its download.
+      - name: Apply Plugin Metadata (writes null version placeholders)
+        run: |
+          set -euo pipefail
+          BASE="$(cat "${RUNNER_TEMP}/base_sha.txt")"
+          # --diff-filter=d drops deletions; the script opens each path it is
+          # handed, so a removed plugin would crash it.
+          git diff --name-only --diff-filter=d "$BASE" HEAD > "${RUNNER_TEMP}/changed_files.txt"
+          cat "${RUNNER_TEMP}/changed_files.txt"
+          PLUGMAN_BASE_REF="$BASE" \
+            python test/auto_apply_plugin_metadata.py "$(cat "${RUNNER_TEMP}/changed_files.txt")"
+
+      - name: Commit Plugin Metadata
+        uses: stefanzweifel/git-auto-commit-action@v7
+        with:
+          commit_message: "[ci] apply-plugin-metadata"
+
       - name: Apply Version Metadata
         run: |
           python test/auto_apply_version_metadata.py $(git log --pretty=format:'%h' -n 1)

README.md

@@ -87,7 +87,14 @@ There are three different ways the plugin manager can be installed:
   the category directory you feel is the most relevant to the type of plugin you're submitting, [here](plugins).
 - You also need a `plugman` dict with the plugin metadata in the plugin (see the [example](https://github.com/bombsquad-community/plugin-manager?tab=readme-ov-file#example) below).
    - The name of the plugin must be in snake_case and matching the file name.
-   - Must have the plugin_name, description, external_url, authors and version keys.
+   - Must have the description, external_url, authors and version keys.
+   - `plugin_name` is optional and defaults to the file name without `.py`. Spell it out only if you
+     want it documented in the source; it still has to match the file name.
+   - Values may be module-level constants defined *above* the dict, so a plugin that already keeps
+     a `__version__` or `__author__` around does not have to repeat itself:
+     `version=__version__` and `authors=[{"name": __author__[0], ...}]` both work. The plugin is
+     parsed, never imported, so only plain literals and indexing into them are understood;
+     expressions like `__file__.split("/")[-1]` are rejected.
 - Plugin manager will also show and execute the settings icon if your `ba.Plugin` class has methods `has_settings_ui` and `show_settings_ui`; check out the [colorscheme](https://github.com/bombsquad-community/plugin-manager/blob/eb163cf86014b2a057c4a048dcfa3d5b540b7fe1/plugins/utilities/colorscheme.py#L448-L452) plugin for an example.
 
 #### Example:
@@ -97,15 +104,18 @@ Let's say you wanna submit this new utility-type plugin named as `sample_plugin.
 # ba_meta require api 9
 import babase
 
+__version__ = "1.0.0"
+__author__ = ["Loup", "brostos"]
+
 plugman = dict(
-    plugin_name="sample_plugin",
+    plugin_name="sample_plugin",  # optional, defaults to the file name
     description="A test plugin for demonstration purposes blah blah.",
     external_url="https://www.youtube.com/watch?v=dQw4w9WgXcQ",
     authors=[
-        {"name": "Loup", "email": "loupg450@gmail.com", "discord": "loupgarou_"},
-        {"name": "brostos", "email": "", "discord": "brostos"}
+        {"name": __author__[0], "email": "loupg450@gmail.com", "discord": "loupgarou_"},
+        {"name": __author__[1], "email": "", "discord": "brostos"}
     ],
-    version="1.0.0",
+    version=__version__,
 )
 
 # ba_meta export babase.Plugin
@@ -127,7 +137,7 @@ guide) once you open a pull request.
 ### Updating a Plugin
 
 - Make a [pull request](../../compare) with whatever changes you'd like to make to an existing plugin, and add a new
-  version number in your plugin in the plugman dict.
+  version number in your plugin in the plugman dict, or in the `__version__` it reads from.
 
 #### Example
 
@@ -138,16 +148,26 @@ diff --git a/plugins/utilities/sample_plugin.py b/plugins/utilities/sample_plugi
 index ebb7dcc..da2b312 100644
 --- a/plugins/utilities/sample_plugin.py
 +++ b/plugins/utilities/sample_plugin.py
-@@ -9,7 +9,7 @@
-         {"name": "Loup", "email": "loupg450@gmail.com", "discord": "loupgarou_"},
-         {"name": "brostos", "email": "", "discord": "brostos"}
+@@ -1,16 +1,16 @@
+ # ba_meta require api 9
+ import babase
+
+-__version__ = "1.0.0"
++__version__ = "1.1.0"
+ __author__ = ["Loup", "brostos"]
+
+ plugman = dict(
+     plugin_name="sample_plugin",  # optional, defaults to the file name
+     description="A test plugin for demonstration purposes blah blah.",
+     external_url="https://www.youtube.com/watch?v=dQw4w9WgXcQ",
+     authors=[
+         {"name": __author__[0], "email": "loupg450@gmail.com", "discord": "loupgarou_"},
+         {"name": __author__[1], "email": "", "discord": "brostos"}
      ],
--    version="1.0.0",
-+    version="1.1.0",
+     version=__version__,
  )
-
- # ba_meta export babase.Plugin
-@@ -21,4 +21,4 @@
+@@ -23,5 +23,5 @@
+     def has_settings_ui(self):
          return True
 
      def show_settings_ui(self, source_widget):