diff --git a/docs/external-contributor-merge-flow.md b/docs/external-contributor-merge-flow.md index 7465636..79465d7 100644 --- a/docs/external-contributor-merge-flow.md +++ b/docs/external-contributor-merge-flow.md @@ -3,11 +3,12 @@ **Author:** Epimetheus **Architecture review:** Ship (owns merge.py, sync-mirror.sh) **Code review:** Ganymede (line-level, post-design-approval) -**Status:** Phase 1 sweep-only scope locked (Ship Msg 2/2). Awaiting code review. +**Status:** Phase 1 sweep-only scope locked. Phase 2 architecture decisions locked (Ship Msg 3). Awaiting Phase 1 line-level code review. ## Revision log -- **v2 (this revision):** Phase 1 simplified per Ship's Msg 2 — backfill script dropped (sweep IS the backfill), sweep placement specified explicitly as the very first action after initial fetch (ahead of branch-mirror loop AND auto-create-PR block at line ~250). Phase 2 architecture unchanged. +- **v3 (this revision):** Cleanup pass per Ship Msg 3 — merge commit message updated to locked verbose form (`"Merge external GitHub PR #{N}: {branch_slug}"`), open-questions section collapsed to "Locked Phase 2 decisions" restating the three resolved outcomes (no longer questions). +- **v2:** Phase 1 simplified per Ship's Msg 2 — backfill script dropped (sweep IS the backfill), sweep placement specified explicitly as the very first action after initial fetch (ahead of branch-mirror loop AND auto-create-PR block at line ~250). Phase 2 architecture unchanged. - **v1:** Initial draft. ## Problem statement @@ -90,9 +91,12 @@ async def _merge_no_ff_external(branch: str) -> tuple[bool, str]: Mechanics: 1. Fetch latest origin/main and origin/{branch} 2. Create scratch worktree at HEAD of origin/main - 3. git merge --no-ff origin/{branch} -m "Merge PR: {branch}" - 4. git push origin HEAD:main - 5. Cleanup worktree + 3. Derive: gh_pr_num = re.match(r"gh-pr-(\d+)/", branch).group(1) + branch_slug = branch[len(f"gh-pr-{gh_pr_num}/"):] + 4. git merge --no-ff origin/{branch} \ + -m f"Merge external GitHub PR #{gh_pr_num}: {branch_slug}" + 5. git push origin HEAD:main + 6. Cleanup worktree Conflict handling: - Entity conflicts: same auto-resolve pattern as cherry-pick @@ -108,9 +112,12 @@ async def _merge_no_ff_external(branch: str) -> tuple[bool, str]: """ ``` -The merge commit message uses the branch name (which contains the GitHub PR -number via the `gh-pr-{N}/...` convention) so post-merge processing can -correlate back to the GitHub PR if needed. +The merge commit message format `"Merge external GitHub PR #{N}: {branch_slug}"` +embeds the GitHub PR number explicitly so `git log --merges --grep "#90"` +surfaces the merge from PR number alone (no branch-name guessing). Branch slug +is the post-`gh-pr-{N}/` portion of the branch (e.g., +`contributor/arcium-confidential-computing-challenge`) — already in scope at +merge time, no claim-file read needed. ### Bug #2 fix: self-healing sweep at top of sync-mirror cycle @@ -340,29 +347,29 @@ next merge action. us contributor comments on closed-via-cherry-pick PRs (already partially solves the UX). Phase 2 adds the "merged" badge. Phase 3 is post-Phase-1 cleanup. -## Open questions for Ship +## Locked decisions -**Resolved in Msg 2:** -- ~~`.fork-pr-map` file location~~ → moot, sweep replaces map approach -- ~~Phase 1 vs Phase 2 sequencing~~ → separate deploys, sweep-only Phase 1 confirmed -- ~~One-shot backfill script~~ → dropped, sweep IS the backfill -- ~~Sweep placement~~ → confirmed: first action after initial fetch, before - branch-mirror loop AND auto-create-PR block +**Phase 1 (resolved in Msg 2):** +- `.fork-pr-map` file location → moot, sweep replaces map approach +- Phase 1 vs Phase 2 sequencing → separate deploys, sweep-only Phase 1 +- One-shot backfill script → dropped, sweep IS the backfill +- Sweep placement → first action after initial fetch, before branch-mirror loop + AND auto-create-PR block -**Still open (Phase 2 architecture):** +**Phase 2 (resolved in Msg 2 / Ship reply):** -1. **Backout flag necessary?** I'm including `EXTERNAL_PR_NO_FF_MERGE` config - flag for fast cutout. Overkill given low-traffic external-PR path? Or - prudent given Accelerate Solana pressure? +1. **Backout flag:** `EXTERNAL_PR_NO_FF_MERGE` config flag included, default + `True` after deploy. One config branch in dispatch is cheap insurance with + hackathon timing risk one week out. -2. **Test plan #5 (rebase-after-fix) scope.** Documenting the silent-ignore as - acceptable for now. Sufficient for hackathon, or do you want me to add the - structured-alert path before May 5? +2. **Rebase-after-fix scope:** silent-ignore as documented in §"Edge case". + Eval-reset semantics already handle the recovery path — no structured + alert in this scope. Re-evaluate after first 5-10 external contributors. -3. **`merge --no-ff` commit message format.** I have `"Merge PR: {branch}"`. - Better signal to derive the GitHub PR number from the branch and write - `"Merge external GitHub PR #{N}: {claim_title_or_branch_slug}"`? Trades - verbosity for searchability in `git log --merges`. +3. **Merge commit message format:** verbose with PR number — + `"Merge external GitHub PR #{N}: {branch_slug}"`. Branch slug derived from + the `gh-pr-{N}/` prefix strip; already in scope at merge time, no claim-file + read needed. Searchability via `git log --merges --grep "#90"` is the value. -Sending revised doc directly via Pentagon. Ganymede gets line-level review of -the Phase 1 code once Ship signs off on the revised design. +Ganymede gets line-level review of the Phase 1 sweep code on +`epimetheus/sync-mirror-self-heal` once Ship signs off on this revision.