feat(land): never merge, user clicks
Auto-merge burned a client PR meant for manual merge. Human gate needed on public/client repos; one click on private is cheap, so rule is universal. Land now stops at green + approved + updated branch, notifies, and leaves the merge button alone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLkYHLzszmzq5fMGpFAkHh
This commit is contained in:
naps62
+18
-19
@@ -1,6 +1,6 @@
|
||||
---
|
||||
name: land
|
||||
description: "Drive an existing PR to merge: wait for CI + reviews, fix failures, resolve every comment, push, iterate until green + approved, then merge. Forge-agnostic (GitHub or Gitea)."
|
||||
description: "Drive an existing PR to ready-to-merge: wait for CI + reviews, fix failures, resolve every comment, push, iterate until green + approved, then hand the merge click to the user. Never merges. Forge-agnostic (GitHub or Gitea)."
|
||||
user-invocable: true
|
||||
args:
|
||||
- name: target
|
||||
@@ -8,9 +8,11 @@ args:
|
||||
required: false
|
||||
---
|
||||
|
||||
# Land - Drive a PR to green + merged
|
||||
# Land - Drive a PR to green + ready-to-merge
|
||||
|
||||
Takes an **already-open PR** and shepherds it to done: green CI, all review threads resolved, approved, merged, tracking issue closed. Spends **zero model tokens idling** — waits by arming background watchers that wake on real events, never by polling on a timer.
|
||||
Takes an **already-open PR** and shepherds it to the merge button: green CI, all review threads resolved, approved, branch up to date. Spends **zero model tokens idling** — waits by arming background watchers that wake on real events, never by polling on a timer.
|
||||
|
||||
**Never merge.** The final merge click is always the user's — on every repo, every forge. Public repos with other contributors and client repos need a human gate, and a single click on private repos is cheap. No `gh pr merge`, no merge API call, no `--auto`.
|
||||
|
||||
This is the canonical review/CI-iteration loop. `/work` opens a PR then hands off here; `/yolo` can hand off here when a PR flow is wanted. It also stands alone: `/land 47`, `/land <url>`, or `/land` on a branch that already has a PR.
|
||||
|
||||
@@ -32,9 +34,9 @@ This is the canonical review/CI-iteration loop. `/work` opens a PR then hands of
|
||||
**Derive** (used throughout):
|
||||
- github: `OWNER`/`REPO` from origin. Set with `gh`.
|
||||
- gitea: `BASE=$remoteBaseUrl` (or the origin host), `REPO=<owner>/<repo>` from origin, `$GITEA_TOKEN` in env (`source ~/.env.claude` if missing). Never put the token in a URL.
|
||||
- **Tracking issue `REF`** (optional): parse `Closes <REF>` / `Closes #<n>` from the PR body. Used only to mark the issue done after merge; skip silently if absent.
|
||||
- **Tracking issue `REF`** (optional): parse `Closes <REF>` / `Closes #<n>` from the PR body. Used only for the Linear follow-up note in close-out; skip silently if absent.
|
||||
|
||||
Then run the variant for your forge below. Both share these **terminal conditions** (all must hold before merge):
|
||||
Then run the variant for your forge below. Both share these **terminal conditions** (all must hold before handing off):
|
||||
- CI checks pass
|
||||
- All review threads resolved
|
||||
- Approved, no pending review requests
|
||||
@@ -44,7 +46,7 @@ Then run the variant for your forge below. Both share these **terminal condition
|
||||
- **Baseline once.** Right after resolving `N`, snapshot existing review-comment IDs to `<git-dir>/pr-<N>-seen`. Every later pass processes only IDs not in that file — handled feedback is never re-read. Guard it so a re-entry after a wake never truncates + reseeds (that would reprocess everything).
|
||||
- **Wake on events, not a clock.** CI is minutes; human review is hours. Block a background watcher (Bash `run_in_background` for one-shot "CI done"; `Monitor persistent` for the whole review window) and stay idle until something actually happens. Handle exactly what the watcher reports, then re-arm.
|
||||
- **Stall guard.** A watcher must never hang forever on a stuck pipeline. Bound every CI wait: if no check appears within ~3 min of a push, or a run sits in-progress past a sane ceiling (default ~20 min, or the repo's known CI duration ×2), **stop waiting and surface it to the user** — don't keep idling. Silence is not success.
|
||||
- **Fix everything.** Every unresolved thread gets an action — a code fix or a reply. Bot reviewers (crit, CodeRabbit, Copilot, etc.) count. Don't merge over an unaddressed thread.
|
||||
- **Fix everything.** Every unresolved thread gets an action — a code fix or a reply. Bot reviewers (crit, CodeRabbit, Copilot, etc.) count. Don't declare ready over an unaddressed thread.
|
||||
|
||||
---
|
||||
|
||||
@@ -94,13 +96,12 @@ On each `NEW COMMENT` event (`<cid>` = comment id, `<tid>` = thread):
|
||||
|
||||
**Stop** (`TaskStop` the Monitor) when the terminal condition holds: CI green, approved, no pending review requests (`gh pr view <N> --json reviewDecision,reviewRequests,reviews`), all threads resolved.
|
||||
|
||||
**Merge.** A long review loop moves the base, so update first, let CI re-run, then merge:
|
||||
**Ready.** A long review loop moves the base, so update the branch and let CI re-run — the user's click should be the only step left:
|
||||
```bash
|
||||
gh pr update-branch <N> 2>/dev/null || true # rebase/merge default into the PR branch if behind
|
||||
# if it updated, the CI watcher re-arms on the new head; wait for green again before merging
|
||||
gh pr merge <N> --squash --delete-branch
|
||||
# if it updated, the CI watcher re-arms on the new head; wait for green again
|
||||
```
|
||||
Use `--auto` if branch protection requires it: `gh pr merge <N> --squash --auto --delete-branch`.
|
||||
Then go to step 3 (close out). Do **not** run `gh pr merge` in any form.
|
||||
|
||||
## Gitea variant
|
||||
|
||||
@@ -152,14 +153,13 @@ On each `NEW` event:
|
||||
```
|
||||
- Gitea has no per-thread resolve — signal addressed by replying with a short confirmation and pushing the fix.
|
||||
|
||||
**After approval + green CI** (`TaskStop` the review Monitor first): if the base moved, update the branch, wait for CI green again, then merge.
|
||||
**After approval + green CI** (`TaskStop` the review Monitor first): if the base moved, update the branch and wait for CI green again — the user's click should be the only step left.
|
||||
```bash
|
||||
curl -sS -X POST -H "Authorization: token $GITEA_TOKEN" \
|
||||
"$BASE/api/v1/repos/$REPO/pulls/$N/update" >/dev/null 2>&1 || true
|
||||
# if it updated, re-arm the CI watcher and wait for green before merging, then:
|
||||
curl -sS -X POST -H "Authorization: token $GITEA_TOKEN" -H "Content-Type: application/json" \
|
||||
"$BASE/api/v1/repos/$REPO/pulls/$N/merge" -d '{"Do":"merge"}'
|
||||
# if it updated, re-arm the CI watcher and wait for green
|
||||
```
|
||||
Then go to step 3 (close out). Do **not** call the merge API.
|
||||
|
||||
Never embed `$GITEA_TOKEN` in URLs or commit messages — `Authorization` header only.
|
||||
|
||||
@@ -167,9 +167,8 @@ Never embed `$GITEA_TOKEN` in URLs or commit messages — `Authorization` header
|
||||
|
||||
## 3. Close out
|
||||
|
||||
Merging with `Closes <REF>` in the body closes the tracking issue automatically. If a `REF` was found and the issue is still open after merge, close it:
|
||||
- **linear**: move the issue to "Done" (via the configured Linear MCP).
|
||||
- **gitea**: `curl -sS -X PATCH -H "Authorization: token $GITEA_TOKEN" -H "Content-Type: application/json" "$BASE/api/v1/repos/$REPO/issues/$N" -d '{"state":"closed"}'`.
|
||||
- **github**: `gh issue close <n>`.
|
||||
Report to the user: PR ready to merge (link it), CI green, approved, all threads resolved, and a one-line summary of what feedback was addressed. The merge — and the branch delete + tracking-issue close that follow it — is theirs.
|
||||
|
||||
Report to the user: PR merged, branch deleted, issue closed, and a one-line summary of what feedback was addressed.
|
||||
The review window is often hours; the user may be away when the PR goes green. Push a notification so the one click can happen from their phone: `mcp__ha-mcp__ha_call_service` with `domain: "notify"`, service `mobile_app_pixel_7_naps` (or `blitz.notifyService` from config), message "PR #<N> ready to merge" + the PR URL.
|
||||
|
||||
`Closes <REF>` in the PR body closes the tracking issue automatically on merge (GitHub/Gitea). Only Linear needs follow-up: if config points at Linear, tell the user the issue must be moved to Done after they merge, or move it yourself if you're still around post-merge.
|
||||
|
||||
@@ -72,11 +72,11 @@ Advisory, not a hard gate: for a trivial diff (typo, one-liner, config bump) ski
|
||||
|
||||
### 5. Hand off to `/land`
|
||||
|
||||
The PR is open — now drive it to merged. **Invoke `/land <N>`** (the `land` skill). It owns the whole review/CI iteration loop: waits for CI + reviews without idling, fixes failures, resolves every comment (including bot reviewers), pushes, re-arms, and once green + approved it updates the branch, merges, and closes the tracking issue.
|
||||
The PR is open — now drive it to ready-to-merge. **Invoke `/land <N>`** (the `land` skill). It owns the whole review/CI iteration loop: waits for CI + reviews without idling, fixes failures, resolves every comment (including bot reviewers), pushes, re-arms, and once green + approved it updates the branch and hands the merge click to the user — it never merges.
|
||||
|
||||
Do not re-implement that loop here — `/land` is the single source of truth for it, and it reads the same `remoteHost` / tracker config. `/land` derives the tracking issue from the PR body's `Closes <REF>`, so no extra hand-off state is needed.
|
||||
|
||||
The success bar `/land` enforces (all must hold before it merges): CI green, all review threads resolved, approved with no pending review requests.
|
||||
The success bar `/land` enforces (all must hold before it declares ready): CI green, all review threads resolved, approved with no pending review requests.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -57,4 +57,4 @@ Follow the implementation guidelines from COMMON.md. Move fast — this is yolo
|
||||
|
||||
This stays true to yolo: fire-and-forget, model idle, surfaces only a broken build.
|
||||
|
||||
**If a PR does exist and you want it driven to green + merged** (CI waited on, review comments resolved, iterated until done) — don't hand-roll it here. Hand off to **`/land <N>`** (the `land` skill), the same loop `/work` uses. That's the escape hatch when a "yolo" task turns out to need real review follow-through.
|
||||
**If a PR does exist and you want it driven to green + ready-to-merge** (CI waited on, review comments resolved, iterated until done; the merge click stays with the user) — don't hand-roll it here. Hand off to **`/land <N>`** (the `land` skill), the same loop `/work` uses. That's the escape hatch when a "yolo" task turns out to need real review follow-through.
|
||||
|
||||
Reference in New Issue
Block a user