Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions .github/workflows/merge-train.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Comment on lines +116 to +118

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Consider handling failures from gh api to avoid silently misclassifying commits.

If gh api exits non-zero (e.g., network/auth/rate limiting issues), verified may be unset and [ "${verified}" != "true" ] will treat the commit as unsigned even though verification never actually ran. Please check the gh api exit code and either fail the job (e.g., fail_and_unlabel with a clear "verification could not be performed" message) or otherwise distinguish API failures from verified == false so transient issues don’t show up as signing problems.

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/<key>.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..."
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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/<key>.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.
Expand Down
Loading