feat(keygen): pmoves-keygen integration — auto-populate signing-card ml halves - #2591
Conversation
…ml halves Adds the pmoves-keygen submodule (fork of charmbracelet/keygen) and a keygen_cards wrapper that turns pending-ml signing identity cards into 5x5 cards: - generate --agent <id>: Ed25519 keypair (passphrase via env, never CLI), patches the card's ssh_fingerprint + ssh_allowed_signers_line in place with comment-preserving ruamel (width=4096 so the signers line never wraps) - ssh-auth --name <id>: fleet-access keypair with secrets-funnel *_FILE hint - audit: lists cards still missing their ml SSH half Passphrases resolve from --passphrase-env or KEYGEN_PASSPHRASE. Private keys land in pmoves/chit/keys/ (gitignored). Refuses to overwrite existing keys. Fingerprint output cross-verified byte-identical against ssh-keygen; passphrase enforcement verified. Fork PR powermoves/pmoves-keygen#1 adds the keygen-cli shim this wraps; the gitlink pins its branch tip until it lands on master, then repoint in a follow-up. Testing: pytest tests/tools/test_keygen_cards.py — 6 passed; live generate+revert round-trip on the crush card validated YAML integrity. 💘 Generated with Crush
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6bff7f7e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| if mode == "ruamel": | ||
| ml = card.setdefault("ml", {}) | ||
| ml["primary_method"] = "ssh" |
There was a problem hiding this comment.
Preserve each card's declared signing method
When generate is run for a card whose primary_method is github-app—including the GitHub App service account and label-only runner cards—this unconditional assignment silently converts its canonical verification method to SSH while retaining the old app/runner fields. Restrict this command to SSH cards or require an explicit migration mode rather than rewriting the identity method.
Useful? React with 👍 / 👎.
| if args.passphrase_env: | ||
| return os.environ.get(args.passphrase_env) or None | ||
| return os.environ.get("KEYGEN_PASSPHRASE") or None |
There was a problem hiding this comment.
Fail when the selected passphrase variable is unset
If an operator misspells or forgets the variable named by --passphrase-env, this returns None and key generation proceeds without the requested passphrase—or silently inherits an unrelated KEYGEN_PASSPHRASE already present in the copied environment. For signing private keys, an explicitly selected but empty variable should fail closed before creating any key material.
Useful? React with 👍 / 👎.
| print(f"error: key already exists at {key_path} (refusing to overwrite)", file=sys.stderr) | ||
| return 1 | ||
|
|
||
| artifacts = generate_key(key_path, args) |
There was a problem hiding this comment.
Install or validate ruamel before generating the key
In the repository's normal tool environment only PyYAML is declared, so ruamel.yaml is absent: this line creates the private key successfully, but the command later returns 2 without patching the card. Installing ruamel and retrying then hits the existing-key refusal, leaving the advertised generate-and-patch workflow unable to complete; declare the dependency or check it before consuming the key path.
Useful? React with 👍 / 👎.
| for card in data.get("cards", []): | ||
| agent = card.get("h", {}).get("agent_id", "?") | ||
| ml = card.get("ml", {}) or {} | ||
| has_ssh = bool(ml.get("ssh_fingerprint") and ml.get("ssh_allowed_signers_line")) |
There was a problem hiding this comment.
Audit the configured ML method instead of only SSH fields
This predicate reports every non-SSH card as pending even when its declared machine identity is complete—for example, runner cards are intentionally identified by ci_runner_label and do not sign commits, while GitHub App cards use github_app_installation_id. Because the audit then recommends generate, it directs operators toward unnecessary SSH keys; determine completeness from primary_method and the card role.
Useful? React with 👍 / 👎.
| [submodule "pmoves-keygen"] | ||
| path = pmoves-keygen | ||
| url = https://github.com/POWERFULMOVES/pmoves-keygen.git |
There was a problem hiding this comment.
Add the required Codex home for the new submodule
The repository-wide search finds no pmoves/docs/AGENTS/SUBMODULE_CODEX_HOMES/pmoves-keygen.md, although the referenced submodule workflow requires a matching overlay in the same change. Consequently codex-audit records this keygen/CHIT integration without its routing, companion, and validation guidance; add the required overlay alongside this stanza.
AGENTS.md reference: AGENTS.md:L181-L184
Useful? React with 👍 / 👎.
…08-18) The claim register's last SPARK-KIMI entry was 2026-05-27; this session landed eight lanes that were never registered. One CLAIM+RELEASE entry registers them with live-main merge verification for each: - Hermes unblock chain (fork sync #3 + gitlink #2511) - P7 legacy stanza fix #2549 (+ pr-monitor bot classifier fix) - PMOVES-crush visual ecosystem (#10/#11 + promotion #2571) - pmoves-keygen adoption (fork #1 OPEN, parent #2591) — closes the pending-ml signing-card gap (24/25 cards h-only) - #2515 fleet handoffs docs - Hygiene: 86-commit stack proven landed and retired; 10 worktrees + 5 stashes cleared; cipher-mcp deletion corrected-to-restore - ~100GB disk recovery - P7 migrated to canonical service (13 rooms, NATS connected) Open follow-ups listed unclaimed for the next lane owners. 💘 Generated with Crush
…08-18) The claim register's last SPARK-KIMI entry was 2026-05-27; this session landed eight lanes that were never registered. One CLAIM+RELEASE entry registers them with live-main merge verification for each: - Hermes unblock chain (fork sync #3 + gitlink #2511) - P7 legacy stanza fix #2549 (+ pr-monitor bot classifier fix) - PMOVES-crush visual ecosystem (#10/#11 + promotion #2571) - pmoves-keygen adoption (fork #1 OPEN, parent #2591) — closes the pending-ml signing-card gap (24/25 cards h-only) - #2515 fleet handoffs docs - Hygiene: 86-commit stack proven landed and retired; 10 worktrees + 5 stashes cleared; cipher-mcp deletion corrected-to-restore - ~100GB disk recovery - P7 migrated to canonical service (13 rooms, NATS connected) Open follow-ups listed unclaimed for the next lane owners. 💘 Generated with Crush
…08-18) (#2596) * docs(agents): register the SPARK-KIMI convergence wave (2026-08-10 → 08-18) The claim register's last SPARK-KIMI entry was 2026-05-27; this session landed eight lanes that were never registered. One CLAIM+RELEASE entry registers them with live-main merge verification for each: - Hermes unblock chain (fork sync #3 + gitlink #2511) - P7 legacy stanza fix #2549 (+ pr-monitor bot classifier fix) - PMOVES-crush visual ecosystem (#10/#11 + promotion #2571) - pmoves-keygen adoption (fork #1 OPEN, parent #2591) — closes the pending-ml signing-card gap (24/25 cards h-only) - #2515 fleet handoffs docs - Hygiene: 86-commit stack proven landed and retired; 10 worktrees + 5 stashes cleared; cipher-mcp deletion corrected-to-restore - ~100GB disk recovery - P7 migrated to canonical service (13 rooms, NATS connected) Open follow-ups listed unclaimed for the next lane owners. 💘 Generated with Crush * docs(agnote): SPARK-KIMI lane refresh — coordination split acknowledged, stale VSS claim released Claims the active SPARK lanes under the operator's merge/fix split (4090 merges, Z890 tools, SPARK reviews + registers), and releases the stale feature/spark-vss-submodule-wiring claim after verifying it landed via squash #2277. Generated with Crush * docs(agnote): correct the Hermes commit count and the SPARK registration gap Two review findings on the register entry, both verified against source. Hermes fork-sync #3 advanced 42 upstream commits, not 49. The promotion commit f6c7936 corrects this in its own body ("42 commits advance on the hardened branch"); 49 was the original PR title. A provenance register that preserves a superseded count disagrees with the commit it summarizes. The "last SPARK-KIMI entry was 2026-05-27" claim was false. The same register already holds a SPARK-KIMI claim on 2026-06-01, a CLAIM/RELEASE pair on 2026-07-13, and KIMI-SPARK activity through 2026-07-29. The real unregistered interval is 2026-07-29 -> 2026-08-18. Overstating it by two months would have made lane reconciliation unreliable in exactly the direction that causes duplicate work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Agent Zero <agent.zero@pmoves.ai> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…it ungated `pmoves-keygen` became a submodule in #2591, so the monorepo now pins a commit in it. The repo has NO protection of any kind: GET /repos/POWERFULMOVES/pmoves-keygen/branches/master/protection 404 "Branch not protected" GET /repos/POWERFULMOVES/pmoves-keygen/rulesets [] That is the #2522 exposure class, on a branch the parent now depends on. The `fork` profile's `non_fast_forward` rule is the one that matters here: without it, a force-push to master can strand the parent's recorded gitlink, and `deletion` prevents the branch disappearing out from under a clone. Enrolled with `profile: fork`, `branch: master`: * `fork`, not `monorepo` — the monorepo profile requires merge-gate, python-tests, hardening-validation, verify and submodule-gitlink-gate, none of which exist on that repo. Applying it would gate master on checks that can never report, deadlocking the repo. * `master`, not `main` — that is the fork's actual default branch. The ruleset conditions use ~DEFAULT_BRANCH so they resolve correctly either way, but the classic-protection workflow reads this field literally. * `required_approving_review_count` stays at the profile default of 0, so a solo operator is not locked out of their own repo. Enrollment goes from 4 repos to 5. The broader gap #2522 documented is untouched here — this fixes the one the parent just started consuming. Applying it still needs a dispatch of branch-protection-ruleset-sync.yml; this commit only makes the spec name the repo. 31 branch-protection tests pass; JSON validates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ungated (#2607) * fix(branch-protection): enroll pmoves-keygen — the monorepo consumes it ungated `pmoves-keygen` became a submodule in #2591, so the monorepo now pins a commit in it. The repo has NO protection of any kind: GET /repos/POWERFULMOVES/pmoves-keygen/branches/master/protection 404 "Branch not protected" GET /repos/POWERFULMOVES/pmoves-keygen/rulesets [] That is the #2522 exposure class, on a branch the parent now depends on. The `fork` profile's `non_fast_forward` rule is the one that matters here: without it, a force-push to master can strand the parent's recorded gitlink, and `deletion` prevents the branch disappearing out from under a clone. Enrolled with `profile: fork`, `branch: master`: * `fork`, not `monorepo` — the monorepo profile requires merge-gate, python-tests, hardening-validation, verify and submodule-gitlink-gate, none of which exist on that repo. Applying it would gate master on checks that can never report, deadlocking the repo. * `master`, not `main` — that is the fork's actual default branch. The ruleset conditions use ~DEFAULT_BRANCH so they resolve correctly either way, but the classic-protection workflow reads this field literally. * `required_approving_review_count` stays at the profile default of 0, so a solo operator is not locked out of their own repo. Enrollment goes from 4 repos to 5. The broader gap #2522 documented is untouched here — this fixes the one the parent just started consuming. Applying it still needs a dispatch of branch-protection-ruleset-sync.yml; this commit only makes the spec name the repo. 31 branch-protection tests pass; JSON validates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(branch-protection): enroll the other 20 ungated submodules — 21 of 72 were open keygen was not the only one. Swept all 72 POWERFULMOVES-owned submodules for classic protection and rulesets on the branch the parent actually tracks: reachable 72 GATED 51 UNGATED 21 <- neither a ruleset nor classic protection Every ungated repo is one the monorepo pins a gitlink into, so a force-push or a branch deletion on any of them can strand or silently change what a clone of PMOVES.AI resolves to. `non_fast_forward` and `deletion` in the fork profile are the two rules that close that. All 20 (plus keygen, already in this branch) enrolled with `profile: fork` — minimum protection, no required status checks, required_approving_review_count inherited at 0, so no repo is deadlocked and no sync/* PR is stranded (the constraint recorded in the profile description from #2490 review N5). The `branch` field is per-repo and load-bearing: 9 of these track PMOVES.AI-Edition-Hardened while their GitHub default is `main`, and the rest spread across main / master / release / develop. Verified against the existing PMOVES-hermes-agent enrollment that this is handled correctly — its applied ruleset condition reads {"include": ["refs/heads/PMOVES.AI-Edition-Hardened"], "exclude": []} so the tool resolves the spec's ~DEFAULT_BRANCH token from the `branch` field at apply time, not from the repo's actual default. Enrolling with the tracked branch therefore gates the branch the parent consumes rather than one nobody pins. Had it resolved the other way, 9 of these would have been "protected" on a branch the monorepo never reads. Enrollment goes 4 -> 25 repos. Nothing changes on GitHub until branch-protection-ruleset-sync.yml is dispatched; this commit only edits the spec. 31 branch-protection tests pass; JSON validates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(branch-protection): pinokio was gated on a branch the parent does not track Cross-checked all 25 per_repo_overrides against .gitmodules. 21 of 21 that declare a branch matched, and the 3 without one (keygen, nats-server, PMOVES.AI) match their repo default. One did not: POWERFULMOVES/PMOVES-pinokio config "main" .gitmodules "PMOVES.AI-Edition-Hardened" Pre-existing, not introduced here -- but it is precisely the gap this PR exists to close, so it is fixed with the sweep rather than left behind. Why it matters more than a typo: branch_protection.py resolves the branch as per_repo_overrides[repo].branch FIRST, then .gitmodules, then the spec default. The explicit override therefore BEAT the correct .gitmodules value. Protection was written to `main` while the parent pins a gitlink into PMOVES.AI-Edition-Hardened, so the branch a clone actually resolves had neither non_fast_forward nor deletion protection -- the same exposure the other 21 repos were enrolled to fix. Left alone deliberately: the "[ main ]" key under ruleset_overrides is a RULESET NAME, not a branch selector. Both profiles name their ruleset "[ main ]" regardless of target branch, so renaming it would have broken the override binding. Checked before touching it. Verified: load_spec OK, SpecValidator clean, 64/64 tests pass, and the resolved branch now equals the .gitmodules declaration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…merged (#2663) The gitlink sat at 36ef041 — the tip of the feat/keygen-cli BRANCH, not master. PR POWERFULMOVES/pmoves-keygen#1 merged as 1302da96dd44, so the parent now points at master head and the merged CLI is actually what a checkout gets. The repoint was queued on merge in #2591 and re-flagged in the #2655 correction. What landed in that PR beyond the original three commits, all found in review: - A blank KEYGEN_PASSPHRASE was treated as absent, writing an UNENCRYPTED private key with nothing in the output saying so — the secrets-funnel-gap shape. Output now carries encrypted=true|false and warns on set-but-empty, verified against ssh-keygen rather than trusting the flag. - key_type reported the REQUESTED type while authorized_key and fingerprint were derived from the actual key, so an existing RSA key emitted `ssh-rsa` beside `key_type=ed25519`. For a consumer populating signing identity cards that writes the wrong key type into fleet identity metadata. - The repo had no .gitignore, correct while it was a library; adding the first binary target left keygen-cli untracked in the repo root. STILL OPEN, deliberately not resolved here. The submodule tracks `master`, while submodule-branch-policy-check expects PMOVES.AI-Edition-Hardened — this is the sixth of the six mismatches in #2639, and the one #2642 does not cover. It also now carries a consequence worth stating plainly: PMOVES-specific code has merged into the fork's `master`, so master has diverged from charmbracelet/keygen and future fork-syncs are no longer fast-forward. That is contrary to the pattern #2643 established (upstream-identical default branch, PMOVES overlay on the hardened branch). Merging to master was the documented plan of record from #2591, so it was followed rather than overridden — but whether to reorganise onto a hardened branch is an owner decision, not one to take by rewriting a fork's master unilaterally. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Summary
Adopts the
pmoves-keygenfork (charmbracelet/keygen) to close the pending-ml gap in the 5×5 signing identity cards: today only 1 of 25 cards (darkxside) carries its SSH machine-loadable half — the rest areactive: truewith null key fields, whichSIGNING_IDENTITY_CARDS.mddocuments as "ML half is operator-provided when SSH/GPG/App keys land." This makes key landing mechanical instead of manual.Three wrapper modes (
python3 -m pmoves.tools.keygen_cards):generate --agent <id>— Ed25519 keypair (passphrase via--passphrase-env/KEYGEN_PASSPHRASE, never CLI args), patches the card'sssh_fingerprint+ssh_allowed_signers_linein place with comment-preserving ruamel (width=4096so the long signers line never wraps into invalid YAML)ssh-auth --name <id>— fleet-access keypair with a secrets-funnel*_FILEhintaudit— lists cards still missing their ml SSH halfPrivate keys land in
pmoves/chit/keys/(gitignored). Existing keys are never overwritten.Companion
Fork PR powermoves/pmoves-keygen#1 adds the
keygen-clishim this wraps. The gitlink pins the branch tip so the CLI resolves; follow-up repoint to master once the fork PR lands.Verification
ssh-keygen -lfcrushcard, validated YAML parses with a single-line 4-part signers entry, then revertedTesting
pytest pmoves/tests/tools/test_keygen_cards.py— 6 passed (audit classification, unknown-agent, overwrite-refusal, CLI parse, no-wrap regression guard)Node impact
git submodule update --init pmoves-keygenafter merge💘 Generated with Crush