fix(#415): /split-portfolio configures branch protection on private repo#431
Conversation
…r split-portfolio Add step 4a to /split-portfolio so that `main` on the newly-created private portfolio repo is immediately protected: 1 required approving review, no required status checks (lenient for repos without CI yet), enforce_admins off so the operator retains admin merge ability. Gracefully degrades when the operator lacks admin access — prints an actionable warning with the manual GitHub UI path and continues the migration rather than blocking. Idempotency: PUT to the branch protection API is a no-op update on re-runs. The idempotency table and the final migration report are updated to reflect the new step. Closes #415 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
atlas-apex
left a comment
There was a problem hiding this comment.
Code Review: PR #431
Commit: 6df02306c72590e61dfbc76c3e1d363bacf02c3b
Summary
Adds step 4a to /split-portfolio SKILL.md — applies branch protection on main of the newly-created private portfolio repo via gh api ... /branches/main/protection -X PUT. Lenient defaults (no required CI checks, 1 approval, enforce_admins: false), graceful 403 handling when the operator lacks admin perms, idempotent on re-runs. Closes #415 — closes the gap where the only gate on the new private repo's main was the agent-side block-main-push.sh hook (ungated for direct git push from a non-Claude terminal).
Checklist Results
- Architecture & Design: N/A (docs-only change to skill markdown)
- Code Quality: Pass
- Testing: Pass (3-scenario test plan in PR body)
- Security: Pass (defense-in-depth — actively improves posture)
- Performance: N/A
- PR Description & Glossary: Pass
- Summary Bullet Narrative: Pass (each bullet has what + why)
- Technical Decisions (AgDR): N/A (implements expected behavior from issue #415; no new decision)
- Adopter Handbooks: Pass (no handbook violations)
Issues Found
None blocking.
Suggestions
suggestion: — The JSON body for PUT /branches/main/protection is duplicated verbatim across two code blocks (the bare gh api ... <<EOF example, then the if ! gh api ... <<EOF ... fi graceful-error wrapper). A future edit could drift them apart. Consider one of:
- Show the JSON body once as a standalone snippet (or a variable assignment
PROTECTION_BODY=$(cat <<'EOF' ... EOF)), then reference it from both calls. - OR drop the bare example and keep only the production form with graceful handling — the operator reading the skill only needs the latter.
This is purely a documentation cleanliness nit; the current shape is functionally correct and the duplication isn't actively misleading.
nit: — On the success-print, ${OWNER}/${REPO_NAME} interpolates inside literal text in the SKILL.md. Confirmed elsewhere in the skill that operators are expected to copy-paste bash blocks and the variables resolve at runtime — so this is consistent with house style. Mentioning only because it's worth a final eyeball pass that all four ${OWNER} and ${REPO_NAME} references are inside bash fenced blocks (they are).
Verdict
APPROVED
The change is small, surgical, and directly closes the documented gap in #415. Branch protection is configured idempotently with sensible lenient defaults; the 403 fallback prints an actionable manual path; the migration continues either way. Idempotency table and final report are updated consistently. CI green (4/4 passing). Commit message follows handbooks/general/commit-message-quality.md — clear WHAT + WHY + trade-offs + Closes link.
🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: `6df02306c72590e61dfbc76c3e1d363bacf02c3b`
…r split-portfolio (#431) Add step 4a to /split-portfolio so that `main` on the newly-created private portfolio repo is immediately protected: 1 required approving review, no required status checks (lenient for repos without CI yet), enforce_admins off so the operator retains admin merge ability. Gracefully degrades when the operator lacks admin access — prints an actionable warning with the manual GitHub UI path and continues the migration rather than blocking. Idempotency: PUT to the branch protection API is a no-op update on re-runs. The idempotency table and the final migration report are updated to reflect the new step. Closes #415 Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
/split-portfolioSKILL.md — after the private portfolio repo is created and pushed, configures branch protection onmainviagh apiTesting
/split-portfolioon a test fork → verify branch protection appears on private repoGlossary
Closes #415
🤖 Generated with Claude Code