diff --git a/.github/workflows/merge-train.yml b/.github/workflows/merge-train.yml index b6cbbd089..a943ff009 100644 --- a/.github/workflows/merge-train.yml +++ b/.github/workflows/merge-train.yml @@ -103,9 +103,25 @@ jobs: # be signed. The bot can't sign, but it isn't adding commits # here — these are the author's. Verify upfront so a quiet # branch-protection rejection downstream doesn't confuse. - unsigned=$(git log --format='%H %G?' "origin/main..origin/${HEAD_REF}" | awk '$2!="G" && $2!="U" {print $1}' || true) + # + # Use the GitHub commit-verification API rather than `git log + # %G?`. The runner is a fresh `actions/checkout` with no + # `gpg.ssh.allowedSignersFile` configured, so `%G?` returns + # `N` for every SSH-signed commit even though the signature + # itself is valid (#618). The API uses each contributor's + # registered SSH signing keys (`gh ssh-key add --type signing` + # — same set the GitHub UI uses for the green "Verified" + # badge), which is the authoritative source. + unsigned="" + while read -r sha; do + [ -n "${sha}" ] || continue + verified=$(gh api "repos/${REPO}/commits/${sha}" --jq '.commit.verification.verified') + if [ "${verified}" != "true" ]; then + unsigned="${unsigned}${sha}"$'\n' + fi + done < <(git rev-list "origin/main..origin/${HEAD_REF}") if [ -n "${unsigned}" ]; then - fail_and_unlabel "one or more commits between main and \`${HEAD_REF}\` are not GPG/SSH-signed:\n\n\`\`\`\n${unsigned}\n\`\`\`\n\nSign them locally and re-add the label. The bot cannot sign on your behalf." + fail_and_unlabel "one or more commits between main and \`${HEAD_REF}\` are not verified-signed per the GitHub API:\n\n\`\`\`\n${unsigned}\`\`\`\n\nSign them locally and re-add the label. If the commits look signed locally (\`git log --show-signature\`) but the API disagrees, the contributor's SSH signing key isn't registered with GitHub — run \`gh ssh-key add --type signing ~/.ssh/.pub\` (needs \`admin:ssh_signing_key\` scope; \`gh auth refresh -h github.com -s admin:ssh_signing_key\` if missing)." fi echo "[4/5] waiting for required checks to complete..." diff --git a/CHANGELOG.md b/CHANGELOG.md index e2b7fbf4f..223dbe5e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ installable release; see the roadmap in [README.md](README.md). ## [Unreleased] +### Fixed + +- **`merge-train` signature check now uses GitHub's verification API instead of `git log %G?`** ([#618](https://github.com/robotrocketscience/aelfrice/issues/618)). The `[3/5] signature check` step in `.github/workflows/merge-train.yml` rejected every PR labeled `ready-to-merge` since the workflow shipped (#602): `gh pr list --state merged --search "label:ready-to-merge"` returned `[]`. Root cause: the check ran `git log --format='%H %G?'` from a fresh `actions/checkout` environment with no `gpg.ssh.allowedSignersFile` configured, so `%G?` returned `N` (no signature) for every SSH-signed commit even though the signature itself was valid. Local working trees verify only when the contributor explicitly sets `gpg.ssh.allowedSignersFile`; the bot never does. Fix replaces the `%G?` check with `gh api repos/{owner}/{repo}/commits/{sha} --jq '.commit.verification.verified'` per commit between main and the PR head — the same source of truth the GitHub UI uses for the green "Verified" badge, populated from each contributor's `gh ssh-key add --type signing` registration. Failure message now points the contributor at `gh ssh-key add --type signing ~/.ssh/.pub` (with the `admin:ssh_signing_key` scope hint) for the case where commits look signed locally but the API disagrees. No other workflow logic touched; FF-check, head-SHA sanity, required-checks wait, and FF push are unchanged. + ### Added - **HRR persistence — split-format save/load on `HRRStructIndex`** ([#553](https://github.com/robotrocketscience/aelfrice/issues/553)). `HRRStructIndex.save(path)` now writes a per-store directory containing `struct.npy` (the `(N, dim)` float64 matrix, mmap-able) plus `meta.npz` (the small metadata blob: belief ids, role/id vectors, dim, seed, layout version) instead of a single bundled `.npz`. Writes are atomic via temp-file + `os.replace` so a reader process never observes a partial write. `HRRStructIndex.load(path)` auto-detects the layout: a directory with the two split-format files loads the new layout; a file path falls through to the v1.7 bundled `.npz` reader and emits a one-shot module-logger deprecation warning (legacy stores still load — this is a substrate landing, not a breaking change). New `mmap=False` keyword on `load()` requests `np.load(mmap_mode='r')` on the struct matrix; default off keeps existing callers byte-stable. The split layout is the substrate that makes persistence-default-ON viable per `docs/feature-hrr-integration.md` — `np.load(.npz, mmap_mode='r')` is silently ignored per the numpy docs, so mmap requires this format change. Sub-pieces (`HRRStructIndexCache` mmap read path, ephemeral-path auto-disable, `aelf doctor` rows, `AELFRICE_HRR_PERSIST` opt-out flag) ship as separate atomic PRs per the spec's "Coordination" section.