From ec9ddb09a7e4ea79f23f0440705360dd183ac02f Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Thu, 2 Jul 2026 04:16:07 -0400 Subject: [PATCH] docs: closed-not-merged PR reconciliation rules (#51) Documents and enforces rules for closed-not-merged PR reconciliation, direct-master-push prevention, and issue label cleanup. Rules added: - Explicit definitions for Merged, Landed, Closed-not-merged, and Reconciled. - A PR is done only when Gitea reports it merged or reconciliation proves content is present on master. - Direct push to master is forbidden except as a documented recovery exception. - PRs closed but not merged trigger the reconciliation process. - Branch and worktree cleanup is forbidden until merge or reconciliation is confirmed. - Final reports require PR metadata and Git content verification. Closes #51. --- docs/llm-workflow-runbooks.md | 15 ++++-- skills/llm-project-workflow/SKILL.md | 46 ++++++++++++++----- .../templates/merge-pr.md | 5 +- .../reconcile-closed-not-merged-pr.md | 24 ++++++++++ .../templates/recover-bad-state.md | 3 +- 5 files changed, 73 insertions(+), 20 deletions(-) create mode 100644 skills/llm-project-workflow/templates/reconcile-closed-not-merged-pr.md diff --git a/docs/llm-workflow-runbooks.md b/docs/llm-workflow-runbooks.md index b7ff2f4..709f310 100644 --- a/docs/llm-workflow-runbooks.md +++ b/docs/llm-workflow-runbooks.md @@ -299,14 +299,19 @@ touching anything. - **Prompt:** `Use any eligible merger profile to merge PR #N if checks pass and it is mergeable. Confirm with "MERGE PR N". Do not force-merge.` -### Close the issue after merge +### Close the issue after merge / Reconciliation - **Profile:** issue-manager or merger. - **Steps:** verify remote `master` actually contains the merge; close the - issue (or rely on a `Closes #N` keyword); release `status:in-progress`; - clean up merged branches. -- **Prompt:** `After confirming master contains the merge of PR #N, close issue - #M and delete the merged branch.` + issue; release `status:in-progress` (if it cannot be removed, report why). +- **If closed but not merged (`merged=false`):** Stop normal flow. Do not delete worktrees. Compare PR content to remote `master`. + - **fully landed:** comment it landed, remove `status:in-progress`, clean up. + - **partially landed:** reopen issue, create corrective PR for missing pieces. + - **not landed:** reopen issue/PR, do not clean up. +- **Direct push to master:** is forbidden except as a documented recovery exception. Final reports must include why, commits, PR metadata, and repaired labels. +- **Final reports:** must include both PR metadata (state, merged flag, merge commit) and Git content (remote master hash, expected content present). +- **Prompt (normal):** `After confirming master contains the merge of PR #N, close issue #M and delete the merged branch.` +- **Prompt (reconcile):** `Reconcile closed-not-merged PR #N by verifying if its content landed on master.` ### Stop on blocker diff --git a/skills/llm-project-workflow/SKILL.md b/skills/llm-project-workflow/SKILL.md index 5f5ca61..15ce70c 100644 --- a/skills/llm-project-workflow/SKILL.md +++ b/skills/llm-project-workflow/SKILL.md @@ -19,6 +19,14 @@ identity, and cleaned up only after a real merge. --- + +## Definitions + +- **Merged**: Gitea PR metadata says `merged=true`. +- **Landed**: Equivalent content is present on remote `master`, but PR metadata may not say merged. +- **Closed-not-merged**: PR state is closed and `merged=false`. +- **Reconciled**: A human/LLM verified whether closed-not-merged content landed, partially landed, or was lost, and repaired issue/label/tracker state. + ## A. Issue-first rule **No repository change without a tracking issue.** This includes creating, @@ -133,6 +141,14 @@ Worktree folder = branch with `/` replaced by `-` 10. Push the branch. 11. Open a PR to `master`. 12. **If you are the author, stop before review/merge.** +13. **Normal issue work must not directly push to `master`.** PR content should be merged through the forge PR merge mechanism. +14. Direct push to `master` is allowed only as a documented recovery exception. If used, the final report must include: + - why the PR merge path could not be used + - exact commits pushed + - PR metadata state + - issue labels/state repaired + - whether the PR is closed-not-merged + ## F. Review workflow @@ -148,13 +164,15 @@ Worktree folder = branch with `/` replaced by `-` Only an eligible (non-author) reviewer merges. After a real merge: -1. Confirm remote `master` actually contains the merge commit. -2. Close/release the issue; remove `status:in-progress` if used. -3. Delete the remote branch. -4. Remove the local branch. -5. Remove the branch worktree folder (`scripts/worktree-clean --delete-branch `). -6. Fetch/prune. -7. Confirm the main checkout is clean and current (`0 0` vs remote). +1. Confirm remote `master` actually contains the merge commit (A PR is not done just because `master` moved. A PR is done only when: Gitea reports the PR merged or reconciliation documents equivalent content on `master`; remote `master` contains the expected content; linked issues are closed; `status:in-progress` is removed). +2. Close/release the issue. +3. Whenever an issue is closed, check for `status:in-progress`: remove it, or report why it could not be removed. +4. Do not delete the remote source branch until: PR `merged=true`, or reconciliation confirms content is safely landed, or the issue owner explicitly abandons the work. +5. Remove the local branch. +6. Remove the branch worktree folder (`scripts/worktree-clean --delete-branch `). Branches/worktrees are cleaned only after the above is verified. +7. Fetch/prune. +8. Confirm the main checkout is clean and current (`0 0` vs remote). +9. Final merge/reconciliation reports must include both: PR metadata (state, merged flag, merge commit/hash) and Git content (remote master hash, expected content present or not). Never run cleanup before the merge is confirmed on remote `master`. @@ -165,7 +183,11 @@ Never run cleanup before the merge is confirmed on remote `master`. - No issue exists and one cannot be created. - Worktree state is unclear or unexpected. - Branch/PR state conflicts with the prompt (e.g. prompt says "merged" but it is not). -- A PR is closed but not merged. +- A PR is closed but not merged (closed with `merged=false`). In this case: + - stop normal review/merge + - do not delete branches/worktrees + - do not start dependent work + - run reconciliation - Local `master` is ahead of remote unexpectedly. - The authenticated user is the PR author (for review/merge). - Secrets/tokens appear in the diff. @@ -182,9 +204,10 @@ When in doubt, stop and surface the discrepancy; do not guess or work around a g the commits are preserved on a feature branch (local + remote) first, then `git reset --hard /master` to realign. Never discard commits that are not safely pushed elsewhere. -- **PR closed but not merged:** the work is not in mainline. Re-push the branch, - reopen (or open a replacement) PR, and let an eligible reviewer merge. Do not - assume "closed" means "merged" — verify remote `master` contains the commits. +- **PR closed but not merged (`merged=false`):** do not merge. Run reconciliation: compare PR content to remote `master` and decide: + - **fully landed:** comment that content is present on `master`, remove `status:in-progress`, keep/close issue as appropriate, clean up only after content equivalence is confirmed. + - **partially landed:** do not clean up, reopen issue if needed, create corrective issue/PR for missing pieces. + - **not landed:** reopen issue if needed, reopen PR or create replacement PR, do not clean up source branch/worktree. - **Branch deleted before merge:** if the commits still exist locally (a branch or reflog), re-push them and reopen the PR; otherwise recover via `git fsck --lost-found`. Preserve first, then proceed. @@ -203,6 +226,7 @@ Ready-to-copy templates live in [`templates/`](templates/): - [`review-pr.md`](templates/review-pr.md) — review a PR. - [`merge-pr.md`](templates/merge-pr.md) — merge a PR (eligible reviewer only). - [`recover-bad-state.md`](templates/recover-bad-state.md) — recover from bad state. +- [`reconcile-closed-not-merged-pr.md`](templates/reconcile-closed-not-merged-pr.md) — reconcile a closed-not-merged PR. - [`worktree-cleanup.md`](templates/worktree-cleanup.md) — clean up after merge. - [`release-tag.md`](templates/release-tag.md) — create a release tag. diff --git a/skills/llm-project-workflow/templates/merge-pr.md b/skills/llm-project-workflow/templates/merge-pr.md index f0de7dc..4ae7db6 100644 --- a/skills/llm-project-workflow/templates/merge-pr.md +++ b/skills/llm-project-workflow/templates/merge-pr.md @@ -10,6 +10,7 @@ Rules (llm-project-workflow): author → STOP. - Do not merge unless the PR is open, mergeable, and its checks/review pass. - No force-merge, no bypassing branch protections. +- If the PR is closed but `merged=false`, STOP and run reconciliation. Do not clean up. Steps: 1. Verify authenticated identity + active profile. @@ -20,9 +21,9 @@ Steps: 5. Confirm remote master now contains the merge commit. Then run the cleanup template (worktree-cleanup.md): -- close/release issue #, remove status:in-progress +- close/release issue #, remove status:in-progress (if it cannot be removed, report why) - delete remote branch, remove local branch + worktree folder - fetch/prune; confirm main checkout is clean and current (0 0). -Handoff: reviewer identity, merge result + commit, cleanup done, issue closed. +Handoff: reviewer identity, merge result + commit, cleanup done, issue closed, PR metadata state/merged flag/hash, remote master hash & Git content check. ``` diff --git a/skills/llm-project-workflow/templates/reconcile-closed-not-merged-pr.md b/skills/llm-project-workflow/templates/reconcile-closed-not-merged-pr.md new file mode 100644 index 0000000..80ec6a8 --- /dev/null +++ b/skills/llm-project-workflow/templates/reconcile-closed-not-merged-pr.md @@ -0,0 +1,24 @@ +# Reconcile Closed-Not-Merged PR Prompt + +You are reconciling PR `` in `` which is closed but `merged=false`. + +Rules: + +- Do not delete branches or worktrees before reconciliation is complete. +- Compare the PR's exact content to remote ``. +- Determine if the content is fully landed, partially landed, or not landed. + +Workflow: + +1. Verify the PR metadata says `state=closed` and `merged=false`. +2. Fetch/prune and inspect remote ``. +3. If fully landed: comment that it landed, remove `status:in-progress`, close issue, and clean up. +4. If partially landed: reopen issue if needed, create corrective PR for missing pieces, do not clean up. +5. If not landed: reopen issue/PR, do not clean up. + +Final handoff: + +- PR metadata (state, merged flag, hash) +- Git content verification (remote master hash, expected content present or not) +- reconciliation decision (fully/partially/not landed) +- issue/label state repaired diff --git a/skills/llm-project-workflow/templates/recover-bad-state.md b/skills/llm-project-workflow/templates/recover-bad-state.md index ffe5e04..343a096 100644 --- a/skills/llm-project-workflow/templates/recover-bad-state.md +++ b/skills/llm-project-workflow/templates/recover-bad-state.md @@ -22,8 +22,7 @@ Act per case: - Local master ahead of remote: confirm the extra commits live on a branch pushed to , THEN git reset --hard /master. Verify with `git branch --contains ` first. -- PR closed but not merged: re-push the branch, reopen/replace the PR, let an - eligible reviewer merge. Do not merge your own. +- PR closed but not merged (`merged=false`): stop normal flow and use reconcile-closed-not-merged-pr.md instead. - Branch deleted before merge: recover commits from a local branch/reflog (or git fsck --lost-found), re-push, reopen the PR. - Unauthorized untracked file: do not commit it; leave pre-existing artifacts.