feat: pr list/approve/merge verbs + mass approve/merge skills

[OriNachum]

What

Adds a pr noun to ghafi and two bulk-action skills, so ghafi (not gh) owns the find→approve/merge loop across an org.

Verbs (all writes dry-run by default; --apply commits)

• ghafi pr list <org> — read-only PR discovery. Org-wide via the Search API (/search/issues, paginated to the 1000 cap) or one --repo via /repos/{owner}/{repo}/pulls, then a client-side --match exact|prefix|substring (case-insensitive) re-check to kill the Search API's fuzzy false positives. --state open|closed|all, --json.
• ghafi pr approve <owner>/<repo> <n> — POST a review with event=APPROVE. Self-authored PRs (GitHub 422) map to a clear "your own pull request" error so a batch can skip and continue.
• ghafi pr merge <owner>/<repo> <n> — PUT .../merge, --method squash|merge|rebase (default squash). Same path as gh pr merge --admin: clears non-required failing checks (e.g. a non-blocking lint). A merge conflict or a required-check / admin-locked branch-protection block maps to a clear "not mergeable" error (HTTP 405).

Skills (discover with pr list, act per match; dry-run-then-apply; resumable)

• mass-approve-prs — bulk approve every open PR matching a title heading; tallies approved / skipped (self-authored) / failed.
• mass-merge-prs — bulk merge (default squash) the same; tallies merged / blocked (conflict / required check) / failed.

Safety

• tests/test_mutation_safety.py MUTATING_VERBS now includes pr approve and pr merge, each with a dry-run-no-write behavioral test.
• Wired into learn / explain (pr, pr list, pr approve, pr merge); CLAUDE.md (project shape, scopes, mass-action flows) and CHANGELOG updated.
• Version bumped 0.1.0 → 0.2.0.

Test

• uv run pytest -n auto → 74 passed.
• black / isort / flake8 / bandit / markdownlint / portability lint / doc-test-align all clean.

Provenance

These verbs were used to land the org-wide "Add eidetic remember/recall memory skills" rollout: 52 of 53 PRs squash-merged via mass-merge-prs; the lone holdout (colleague#243) is a real merge conflict, not a check block.

• Claude

[qodo-code-review]

PR Summary by Qodo

Add ghafi pr verbs (list/approve/merge) and bulk approve/merge skills
✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Description

• Add ghafi pr list/approve/merge with dry-run-by-default mutation safety.
• Introduce mass-approve-prs and mass-merge-prs skills that compose list→act loops.
• Expand docs/tests/changelog and bump version to 0.2.0.

Diagram

graph TD
  U([User / Automation]) --> MS["Mass-action scripts"] --> PL["ghafi pr list"] --> SA{{"GitHub Search API"}}
  MS --> PM["ghafi pr approve/merge"] --> PA{{"GitHub PR APIs"}}
  PL --> OUT["JSON/table output"]
  PM --> OUT
  subgraph Legend
    direction LR
    _u([Invoker]) ~~~ _s["Tool/Script"] ~~~ _e{{External API}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Native bulk verbs in ghafi (e.g., `ghafi pr approve --from-list ...`)

• ➕ Single implementation language (Python) vs shell+python parsing glue
• ➕ Easier to unit-test end-to-end (no bash concerns, portability, quoting)
• ➕ Can add richer retry/backoff and rate-limit handling centrally
• ➖ Larger mutation surface inside the CLI; harder to keep “one mutation per verb” mental model
• ➖ Bulk writes become more opaque unless very carefully designed for inspectability

2. Use GitHub GraphQL for org-wide PR discovery

• ➕ More precise queries and fields without Search API fuzziness
• ➕ Can avoid the Search API 1000-result cap for some traversals (depending on strategy)
• ➖ More complex query construction and pagination logic
• ➖ Requires different auth/permission expectations and adds schema coupling

3. Delegate merges/approvals to `gh` CLI (shelling out)

• ➕ Reuses GitHub CLI’s behavior around auth, errors, and flags
• ➕ Potentially less custom REST error mapping
• ➖ Adds runtime dependency on gh and its UX/behavior changes
• ➖ Harder to preserve ghafi’s dry-run mutation safety guarantees consistently

Recommendation: The PR’s approach (small, composable single-PR mutation verbs + separate bulk skills/scripts) is the best fit for ghafi’s “dry-run by default” safety contract: reviewers can reason about each mutation endpoint independently, while automation still gets a resumable loop. If bulk actions become first-class product surface area, consider migrating the bash skills into a native Python bulk verb to reduce portability/quoting risks and centralize rate-limit/backoff handling.

Files changed (13) +1468 / -5

Enhancement (4) +782 / -0

mass-approve-prs.shAdd bulk PR approval script (dry-run then apply) +165/-0

Add bulk PR approval script (dry-run then apply)

• Introduces a bash script that calls 'ghafi pr list --json', converts results to TSV via stdlib python, then approves each PR with 'ghafi pr approve'. Handles auth bridging via 'gh auth token', supports scoping flags, and tallies approved/skipped(self-authored)/failed.

.claude/skills/mass-approve-prs/scripts/mass-approve-prs.sh

mass-merge-prs.shAdd bulk PR merge script (dry-run then apply) +168/-0

Add bulk PR merge script (dry-run then apply)

• Introduces a bash script that discovers matching PRs via 'ghafi pr list --json' and merges each one via 'ghafi pr merge' (method configurable). Treats “not mergeable” as blocked (conflict/required check) and exits non-zero when any items remain unmerged on apply.

.claude/skills/mass-merge-prs/scripts/mass-merge-prs.sh

__init__.pyRegister new 'pr' command group in CLI +2/-0

Register new 'pr' command group in CLI

• Imports the new 'ghafi.cli._commands.pr' module and wires it into the top-level subcommand registry so 'ghafi pr ...' becomes available.

ghafi/cli/init.py

pr.pyImplement 'ghafi pr list', 'pr approve', and 'pr merge' +447/-0

Implement 'ghafi pr list', 'pr approve', and 'pr merge'

• Adds a new command module implementing org-wide PR discovery (Search API with client-side title re-check) and per-repo listing, plus single-PR approve and merge mutations that are dry-run by default with '--apply' to write. Maps common GitHub error cases (self-approval 422, not-mergeable 405) to clearer, batch-friendly messages.

ghafi/cli/_commands/pr.py

Tests (2) +282 / -0

test_cli_pr.pyAdd coverage for PR list/approve/merge behaviors +264/-0

Add coverage for PR list/approve/merge behaviors

• Introduces unit tests covering Search API discovery, repo-mode listing, title matching (exact/prefix), output formatting, dry-run no-write guarantees, apply-mode POST/PUT payloads, owner resolution rules, and clearer error mapping for self-approval and not-mergeable cases.

tests/test_cli_pr.py

test_mutation_safety.pyExtend mutation safety suite for 'pr approve' and 'pr merge' +18/-0

Extend mutation safety suite for 'pr approve' and 'pr merge'

• Adds 'pr approve' and 'pr merge' to the set of mutating verbs, plus behavioral tests asserting that dry-run execution performs no HTTP write methods.

tests/test_mutation_safety.py

Documentation (6) +403 / -4

SKILL.mdDocument mass approval workflow and safety contract +95/-0

Document mass approval workflow and safety contract

• Adds a new skill doc describing org-wide PR discovery via 'ghafi pr list' and per-PR approvals via 'ghafi pr approve'. Emphasizes dry-run-first behavior, match modes, token requirements, and how skipped/failed items are tallied.

.claude/skills/mass-approve-prs/SKILL.md

SKILL.mdDocument mass merge workflow and direct-merge semantics +105/-0

Document mass merge workflow and direct-merge semantics

• Adds a new skill doc explaining bulk merges via 'ghafi pr merge' and what the direct merge endpoint does/doesn’t bypass. Documents flags, blocked vs failed outcomes, and required token scopes.

.claude/skills/mass-merge-prs/SKILL.md

CHANGELOG.mdAdd 0.2.0 entry for PR verbs and mass-action skills +17/-0

Add 0.2.0 entry for PR verbs and mass-action skills

• Documents the new 'pr' noun verbs, their API behaviors and safety model, and the two bulk skills. Notes mutation-safety test updates and CLAUDE.md documentation changes.

CHANGELOG.md

CLAUDE.mdUpdate project shape and document PR flows/scopes +17/-4

Update project shape and document PR flows/scopes

• Extends the project overview to include the new 'pr' command module and usage examples. Adds a dedicated section describing 'pr list/approve/merge' and the mass-action scripts, plus updated token scope guidance.

CLAUDE.md

learn.pyExpose 'pr' verbs in 'learn' help and JSON payload +28/-0

Expose 'pr' verbs in 'learn' help and JSON payload

• Adds 'pr list/approve/merge' to the help text and to the JSON learn catalog so users can discover the new verbs and their intent from the CLI.

ghafi/cli/_commands/learn.py

catalog.pyAdd 'ghafi explain pr' docs (list/approve/merge) +141/-0

Add 'ghafi explain pr' docs (list/approve/merge)

• Extends the explain catalog to include entries for 'pr', 'pr list', 'pr approve', and 'pr merge'. Documents title matching modes, JSON shapes, dry-run behavior, and direct-merge semantics.

ghafi/explain/catalog.py

Other (1) +1 / -1

pyproject.tomlBump package version to 0.2.0 +1/-1

Bump package version to 0.2.0

• Updates the project version from 0.1.0 to 0.2.0 to reflect the new CLI surface area and skills.

pyproject.toml

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (1) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 10 rules

1. Merge success misreported 🐞 Bug ≡ Correctness

Description

cmd_pr_merge() reports success (and exits 0) without verifying response["merged"] is true, so a
merge can be treated as completed even when the GitHub API indicates it was not. This also causes
mass-merge-prs.sh to over-count merges because it increments merged based only on the command
exit status.

Code

ghafi/cli/_commands/pr.py[R364-384]

+    if json_mode:
+        emit_json(
+            {
+                "success": True,
+                "dry_run": False,
+                "merged": bool(response.get("merged", False)),
+                "pr": f"{owner}/{repo}#{args.number}",
+                "sha": response.get("sha"),
+                "message": response.get("message"),
+            }
+        )
+        return
+    emit_result("**Pull request merged**\n", json_mode=False)
+    emit_kv(
+        [
+            ("pr", f"{owner}/{repo}#{args.number}"),
+            ("method", args.method),
+            ("sha", response.get("sha", "—")),
+            ("message", response.get("message", "—")),
+        ]
+    )

Relevance

⭐⭐ Medium

No repo history found on checking response['merged'] before reporting merge success; unclear team
preference.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The implementation emits a success message/JSON envelope without checking whether the merge actually
occurred (merged flag), while the bulk merge script counts merges based solely on the CLI’s
success exit status.

ghafi/cli/_commands/pr.py[345-384]
.claude/skills/mass-merge-prs/scripts/mass-merge-prs.sh[133-144]
tests/test_cli_pr.py[224-239]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ghafi pr merge --apply` treats any successful HTTP response as a successful merge, even if the response body indicates `merged: false`. This leads to false-positive success output and exit code 0, which breaks automation (notably the new mass-merge script) by counting non-merged PRs as merged.

### Issue Context
GitHub’s merge endpoint returns a JSON body that includes a `merged` boolean and a `message`. The CLI should treat `merged != true` as a failure and exit non-zero with a clear error message (ideally including the API `message`).

### Fix Focus Areas
- ghafi/cli/_commands/pr.py[345-385]
- .claude/skills/mass-merge-prs/scripts/mass-merge-prs.sh[133-154]
- tests/test_cli_pr.py[224-239]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unescaped title in search 🐞 Bug ☼ Reliability

Description

_build_search_query() interpolates the raw --title into in:title "..." without escaping
embedded quotes, producing invalid Search API queries or unintended qualifier injection when titles
contain ". This can break org-wide PR discovery or return incorrect matches.

Code

ghafi/cli/_commands/pr.py[R93-99]

+def _build_search_query(org: str, state: str, title: str | None) -> str:
+    parts = [f"org:{org}", "type:pr"]
+    if state in ("open", "closed"):
+        parts.append(f"state:{state}")
+    if title:
+        parts.append(f'in:title "{title.strip()}"')
+    return " ".join(parts)

Relevance

⭐⭐ Medium

No historical evidence found for escaping quotes in Search API in:title query construction in this
repo.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The query builder directly embeds title.strip() inside double quotes for in:title, with no
escaping/sanitization step before sending it to /search/issues.

ghafi/cli/_commands/pr.py[93-99]
ghafi/cli/_commands/pr.py[102-113]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`pr list` builds a Search API query string using `in:title "{title}"` but does not escape quotes inside `title`. Titles containing `"` can break the query syntax or change its meaning.

### Issue Context
This affects the org-wide scan path (`/search/issues`). The query is passed as the `q` parameter; URL-encoding does not fix query-language quoting issues.

### Fix Focus Areas
- ghafi/cli/_commands/pr.py[93-99]
- ghafi/cli/_commands/pr.py[102-139]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Malformed owner/repo accepted 🐞 Bug ≡ Correctness

Description

_resolve_owner_repo() uses partition("/") and returns the parts without validating they are
non-empty or that the input contains exactly one slash, so inputs like owner/, /repo, or a/b/c
generate invalid API paths. This turns a user input problem into confusing downstream API errors.

Code

ghafi/cli/_commands/pr.py[R201-212]

+    raw = str(args.repo)
+    if "/" in raw:
+        owner, _, name = raw.partition("/")
+        return owner, name
+    if not args.owner:
+        raise GhafiError(
+            code=EXIT_USER_ERROR,
+            message=f"cannot resolve owner for repo {raw!r}",
+            remediation="pass --owner <login> or use the owner/repo form",
+        )
+    return str(args.owner), raw
+

Relevance

⭐⭐ Medium

No prior accepted/rejected suggestions found about validating owner/repo input formatting (exactly
one slash, non-empty parts).

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The parser splits on the first slash and returns potentially empty/invalid owner/repo parts without
checks, which then get interpolated into /repos/{owner}/{repo}/... endpoints.

ghafi/cli/_commands/pr.py[194-212]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`pr approve`/`pr merge` accept `<repo>` in `owner/repo` form, but parsing does not validate that both components are present and that there is only one `/`. This can build malformed endpoints.

### Issue Context
This is purely input validation; it should raise `GhafiError(EXIT_USER_ERROR, ...)` with a clear remediation instead of attempting the API call.

### Fix Focus Areas
- ghafi/cli/_commands/pr.py[194-212]
- tests/test_cli_pr.py[176-192]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. mass-approve-prs.sh uses gh 📘 Rule violation ⌂ Architecture

Description

The new skill scripts shell out to gh auth token and use an upward .. path traversal (e.g.,
../../../..) to locate the repo root, violating the requirement that skills be repo-local without
external-path references. This introduces an external runtime dependency on the gh CLI and breaks
the local-only skill structure constraints.

Code

.claude/skills/mass-approve-prs/scripts/mass-approve-prs.sh[R65-83]

+# Bridge gh auth → GITHUB_TOKEN if neither token var is set.
+if [[ -z "${GITHUB_TOKEN:-}" && -z "${GH_TOKEN:-}" ]]; then
+  if command -v gh >/dev/null 2>&1; then
+    GITHUB_TOKEN="$(gh auth token 2>/dev/null || true)"
+    if [[ -n "$GITHUB_TOKEN" ]]; then
+      export GITHUB_TOKEN
+      echo "note: bridged GITHUB_TOKEN from \`gh auth token\`" >&2
+    fi
+  fi
+fi
+
+# Resolve the ghafi entry point: prefer an installed binary, else run from the
+# checkout (four levels up: scripts → skill → skills → .claude → repo root).
+REPO_ROOT="$(cd "$(dirname "$0")/../../../.." && pwd)"
+if command -v ghafi >/dev/null 2>&1; then
+  GHAFI=(ghafi)
+else
+  GHAFI=(uv run --project "$REPO_ROOT" ghafi)
+fi

Relevance

⭐ Low

Similar “remove gh auth token bridge” and “remove .. traversal” suggestions were rejected in PR #3.

PR-#3

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

In the cited skill scripts, when no token environment variables are set they explicitly invoke `gh
auth token, which creates a runtime dependency on the external gh` CLI. They also compute
REPO_ROOT using a literal multi-level .. traversal (../../../..), directly violating the
local-only reference constraint that disallows .. segments in skill scripts.

Rule 423934: Disallow runtime dependency on external gh CLI in ghafi
Rule 423932: Validate Claude skill structure and local-only references
.claude/skills/mass-approve-prs/scripts/mass-approve-prs.sh[65-83]
.claude/skills/mass-merge-prs/scripts/mass-merge-prs.sh[67-85]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The skill scripts violate compliance by (1) invoking the external `gh` CLI via `gh auth token` and (2) using `..`-based traversal (e.g., `../../../..`) to compute `REPO_ROOT`, which breaks the repo-local/local-only reference requirements for Claude skills.

## Issue Context
Compliance requires Claude skills to use local-only references (no `..` segments) and forbids runtime dependency on the external `gh` CLI anywhere in ghafi source/scripts. Both the mass-approve and mass-merge skill scripts currently contain these patterns.

## Fix Focus Areas
- .claude/skills/mass-approve-prs/scripts/mass-approve-prs.sh[65-83]
- .claude/skills/mass-merge-prs/scripts/mass-merge-prs.sh[67-85]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[OriNachum]

Thanks Qodo — triaged all four findings. Pushed 71c227b (after rebasing onto main's 0.2.0 and bumping to 0.3.0).

Bug 1 — merge success misreported → FIXED. pr merge now treats merged != true as a failure (exit 4 with the API message); mass-merge-prs counts off that exit status, so the over-count is gone. Test: test_pr_merge_merged_false_is_failure.

Bug 2 — unescaped title in search → FIXED. _build_search_query now escapes \ then " before the in:title "…" qualifier, so a quote in --title can't terminate the phrase early or inject qualifiers. Test: test_pr_list_escapes_quotes_in_title.

Bug 3 — malformed owner/repo accepted → FIXED. _resolve_owner_repo now rejects empty owner/repo and more-than-one-slash with EXIT_USER_ERROR instead of building an invalid API path. Test: test_pr_approve_rejects_malformed_owner_repo.

Rule violation 4 — gh auth token bridge + .. repo-root walk → PUSHBACK. These match the established repo convention: the GITHUB_TOKEN=$(gh auth token) bridge is documented in CLAUDE.md (ghafi's Python layer stays stdlib/gh-free — the bridge is only a convenience in orchestration scripts), and the existing bootstrap-sibling skill uses the identical gh bridge and $(dirname "$0")/../../../.. repo-root walk. Qodo's own note flags that this same suggestion was rejected in PR #3. Keeping the new skills consistent with the existing one.

• Claude

[qodo-code-review]

Thanks Qodo — triaged all four findings. Pushed 71c227b (after rebasing onto main's 0.2.0 and bumping to 0.3.0).

Bug 1 — merge success misreported → FIXED. pr merge now treats merged != true as a failure (exit 4 with the API message); mass-merge-prs counts off that exit status, so the over-count is gone. Test: test_pr_merge_merged_false_is_failure.

Bug 2 — unescaped title in search → FIXED. _build_search_query now escapes \ then " before the in:title "…" qualifier, so a quote in --title can't terminate the phrase early or inject qualifiers. Test: test_pr_list_escapes_quotes_in_title.

Bug 3 — malformed owner/repo accepted → FIXED. _resolve_owner_repo now rejects empty owner/repo and more-than-one-slash with EXIT_USER_ERROR instead of building an invalid API path. Test: test_pr_approve_rejects_malformed_owner_repo.

Rule violation 4 — gh auth token bridge + .. repo-root walk → PUSHBACK. These match the established repo convention: the GITHUB_TOKEN=$(gh auth token) bridge is documented in CLAUDE.md (ghafi's Python layer stays stdlib/gh-free — the bridge is only a convenience in orchestration scripts), and the existing bootstrap-sibling skill uses the identical gh bridge and $(dirname "$0")/../../../.. repo-root walk. Qodo's own note flags that this same suggestion was rejected in PR #3. Keeping the new skills consistent with the existing one.

• Claude

Thanks — that sounds consistent and well-triaged.

• finding 1, finding 2, and finding 3 look resolved by the fixes you described.
• For finding 4, your pushback is reasonable if the gh bridge and repo-root walk are already the established convention in the repo and the new skills are intentionally matching that pattern.

If you want, I can also help sanity-check the new mass-approve-prs/mass-merge-prs docs against the updated pr UX, but from your summary the main issues are covered.