diff --git a/README.md b/README.md index bf3c73f..a00c377 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ Drop a new `skills//SKILL.md` (+ optional `scripts/`, `references/`, `asse |-------|------| | `work` | tracker issue → worktree → PR → hands off to `land` | | `yolo` | quick ship; optional `land` handoff | -| `land` | drive an open PR to green + merged (canonical CI/review loop) | +| `land` | drive an open PR to green + ready-to-merge; user clicks merge (canonical CI/review loop) | | `blitz` | drive a whole milestone to done | | `nightshift` | hours-long unattended build; architect delegating to subagents, backs off before the 5h limit | | `linear-common` | shared config/setup/worktree conventions + local verification budget (dependency of work/yolo/blitz/nightshift) | diff --git a/skills/land/SKILL.md b/skills/land/SKILL.md index aeb08a8..0314094 100644 --- a/skills/land/SKILL.md +++ b/skills/land/SKILL.md @@ -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 `, 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=/` from origin, `$GITEA_TOKEN` in env (`source ~/.env.claude` if missing). Never put the token in a URL. -- **Tracking issue `REF`** (optional): parse `Closes ` / `Closes #` from the PR body. Used only to mark the issue done after merge; skip silently if absent. +- **Tracking issue `REF`** (optional): parse `Closes ` / `Closes #` 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 `/pr--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 (`` = comment id, `` = thread): **Stop** (`TaskStop` the Monitor) when the terminal condition holds: CI green, approved, no pending review requests (`gh pr view --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 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 --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 --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 ` 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 `. +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 # ready to merge" + the PR URL. + +`Closes ` 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. diff --git a/skills/work/SKILL.md b/skills/work/SKILL.md index 05cfdb9..899808c 100644 --- a/skills/work/SKILL.md +++ b/skills/work/SKILL.md @@ -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 `** (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 `** (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 `, 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. --- diff --git a/skills/yolo/SKILL.md b/skills/yolo/SKILL.md index e30c049..c5581c0 100644 --- a/skills/yolo/SKILL.md +++ b/skills/yolo/SKILL.md @@ -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 `** (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 `** (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.