diff --git a/CHANGELOG.md b/CHANGELOG.md index 6503d6e..09372b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,9 @@ For each entry, **Upgrade impact** classifies the change against the [three-buck read-only, in a browser — never the end-to-end suite. It records what it saw in the pull request's *Preview* line, and brings any difference from local to the developer. - **Not the QC:** the developer's check on the preview still decides ready. + - **No nudges:** an agent doesn't ask or remind the developer to review, ready, or merge a pull + request, doesn't offer to, and doesn't request reviewers (`AGENTS.md`, `git-workflow.md`, + `/open-pr`'s report). - **Where:** `AGENTS.md` § Delivery rules and § Quick reference (a *Preview* line), `/open-pr` (a step 7, and the body's *Preview* line), and the testing and git-workflow rules. With `github`, the pull request template gains *Local check* and *Preview* lines. diff --git a/docs/decisions/0039-check-locally-then-on-the-preview.md b/docs/decisions/0039-check-locally-then-on-the-preview.md index e5a2314..463578d 100644 --- a/docs/decisions/0039-check-locally-then-on-the-preview.md +++ b/docs/decisions/0039-check-locally-then-on-the-preview.md @@ -34,6 +34,10 @@ there cost a deploy round-trip. The preview is shared, and it can reach real ser step off. - **Evidence, not the QC.** The developer's check on the preview still decides ready, and the agent never marks a pull request ready (0005). +- **No nudges.** An agent doesn't ask or remind the developer to review, ready, or merge a pull + request, doesn't offer to, and doesn't request reviewers. In the adopting project, agents ended + task after task with "review it, mark it ready", or "mark it ready yourself, or ask me to". + When those happen is the developer's. ## Consequences diff --git a/evals/static/check-skills.sh b/evals/static/check-skills.sh index f388236..7366413 100755 --- a/evals/static/check-skills.sh +++ b/evals/static/check-skills.sh @@ -724,6 +724,8 @@ check_practices() { file_contains "$AGENTS_MD" 'Check locally first, then on the preview' || missing+=("AGENTS.md: the agent checks locally first, then on the preview (0039)") file_contains "$AGENTS_MD" '^- Preview: ' || missing+=("AGENTS.md: Quick reference says where previews deploy (0039)") file_contains "$SKILLS_DIR/open-pr/SKILL.md" 'Check the preview' || missing+=("/open-pr: checks the preview once it deploys, read-only (0039)") + file_contains "$SKILLS_DIR/open-pr/SKILL.md" "Don't ask or remind the developer" || missing+=("/open-pr: the report doesn't nudge the developer to review, ready, or merge (0039)") + file_contains "$SKELETON/.claude/rules/git-workflow.md" '^- \*\*Reviewing, readying, and merging are the developer' || missing+=("git-workflow rule: no nudges to review, ready, or merge (0039)") file_contains "$SKILLS_DIR/open-pr/SKILL.md" 'the \*\*local check\*\*' || missing+=("/open-pr: stops without the developer's local check (0022)") file_contains "$HOOKS_DIR/guard-git.sh" 'check_pr_create' || missing+=("guard-git.sh: a pull request opens only as a draft (0022)") file_contains "$SKELETON/.claude/rules/git-workflow.md" 'Open after the local check' || missing+=("git-workflow rule: a pull request opens after the local check (0022)") diff --git a/plugins/adf/skills/open-pr/SKILL.md b/plugins/adf/skills/open-pr/SKILL.md index c02f1e1..a630e83 100644 --- a/plugins/adf/skills/open-pr/SKILL.md +++ b/plugins/adf/skills/open-pr/SKILL.md @@ -111,14 +111,16 @@ Use the repository host's CLI: `gh` for GitHub, `glab` (merge requests) for GitL a write the requester can see: show the exact text and post it only on the developer's yes. 9. **Report:** the pull request URL, that it is a **draft**, what you verified — locally, and on the - preview once it deployed — what you could not, and what the developer should QC before marking - it ready (`gh pr ready`). If, after their QC, they - ask you to promote it, run `gh pr ready` then — never before, and never on your own initiative. + preview once it deployed — and what you could not. Don't ask or remind the developer to review it, + mark it ready, or merge it, and don't offer to: when those happen is theirs. Never request + reviewers. If, after their QC, they ask you to promote it, run `gh pr ready` then — never before, + and never on your own initiative. ## Rationalizations (do not accept these) | Agent says... | Why it's wrong | |---|---| +| "I'll end with 'review it and mark it ready when you're happy'" | The developer knows their process. A reminder at the end of every task is noise, and an offer to promote is a question they didn't ask. Report what you verified; ready, review, and merge are theirs. | | "CI is green, so I'll mark it ready" | CI is a signal, not the gate. Ready means a person exercised the change; green tests aren't that. | | "I'll open it ready so the reviewer saves a click" | A ready pull request asks for attention now. Reviewers would spend it on a first QC pass the author should have done. | | "Reviewers can read the diff — a short description is fine" | A reviewer shouldn't reconstruct intent. Agent pull requests whose description doesn't match the diff are a known failure. | @@ -141,6 +143,7 @@ Use the repository host's CLI: `gh` for GitHub, `glab` (merge requests) for GitL - [ ] The push followed the developer's local-check approval — or, with nothing to run, a passing full gate and a clean `/adf:review` — or the developer's ask - [ ] The developer approved the change after testing it by hand on the local environment — or it has nothing to run - [ ] Opened as a **draft**; not marked ready +- [ ] The report doesn't ask, remind, or offer to review, ready, or merge it, and no reviewer was requested - [ ] The change was checked locally before the push, and on the preview once it deployed — read-only — or the body says why not - [ ] The body names the spec folder and links the tracker task - [ ] "Verified / not verified" matches `tasks.md` § Gate results diff --git a/skeleton/.claude/rules/git-workflow.md b/skeleton/.claude/rules/git-workflow.md index e26a370..ae62108 100644 --- a/skeleton/.claude/rules/git-workflow.md +++ b/skeleton/.claude/rules/git-workflow.md @@ -75,11 +75,13 @@ protected branch. developer tests it by hand on the local environment and approves it before the pull request opens (`AGENTS.md` § Delivery rules). - **Open as a draft.** Promote to ready only after a human has exercised the change (preview, - manual QC). + manual QC). An agent never marks a pull request ready on its own initiative — it hasn't seen the + result; it runs `gh pr ready` only when the developer, having QC'd the change, asks it to. +- **Reviewing, readying, and merging are the developer's, on their schedule.** An agent doesn't ask + or remind them to do any of it, doesn't offer to, and doesn't request reviewers or a review. - **Check it on the preview too.** Once the draft's preview deploys, the agent walks the changed flow there, read-only, after its local check, and adds what it saw to the description - (`AGENTS.md` § Delivery rules). An agent never marks a pull request ready on its own initiative — it hasn't seen the - result; it runs `gh pr ready` only when the developer, having QC'd the change, asks it to. + (`AGENTS.md` § Delivery rules). - **Title:** a commit-style subject under 70 characters. - **Body:** the spec folder path and the tracker task link; what changed and why; how to verify; what the author verified and what they could not; screenshots for UI changes. Follow the diff --git a/skeleton/AGENTS.md b/skeleton/AGENTS.md index 34b37c0..68d7911 100644 --- a/skeleton/AGENTS.md +++ b/skeleton/AGENTS.md @@ -93,7 +93,7 @@ tracker task (WHAT — the requester's channel) → specs/NNN-/ (record of - **Commits:** one per green task; prefixes and phase order in `.claude/rules/git-workflow.md`. Don't amend or rewrite pushed history unless asked. - **Local check before the pull request:** when a change alters something a person can see or use, start it on the local environment (§ Quick reference), give the developer the local URL and what to try — the acceptance criteria, or the fast lane's "done when" — and wait. The developer tests it by hand and approves it — and that approval opens the **draft** pull request: push and run `/open-pr` without waiting to be asked. A fix found there goes in first, and the check repeats. Docs-only and CI-only work has nothing to run: the draft opens once the full gate and `/review` pass. Answer-only work has no pull request unless the task asks for one. - **Check locally first, then on the preview.** Before the local check, and before any push, run the change on this checkout's own environment and walk the changed flow yourself. Then, when the project has previews and the change alters something a person sees or uses, walk the same flow on the draft's preview once it deploys — read-only, in a browser, never the end-to-end suite — and add what you saw to the pull request's *Verified*. It's evidence, not the QC: the developer's check on the preview still decides ready. -- **Pull requests:** open as **drafts**, after the local check; name the spec folder (or the light `CR N`) and link the tracker task; state the lane and why, what you verified, and what you could not. Never mark a pull request ready on your own — a human QCs it (preview, manual check) and promotes it, or asks you to once they have. Ready means "a person has exercised this". +- **Pull requests:** open as **drafts**, after the local check; name the spec folder (or the light `CR N`) and link the tracker task; state the lane and why, what you verified, and what you could not. Never mark a pull request ready on your own — a human QCs it (preview, manual check) and promotes it, or asks you to once they have. Ready means "a person has exercised this". Don't ask or remind the developer to review, ready, or merge it, don't offer to, and don't request reviewers: when that happens is theirs. - **Parallel sessions:** when several agent sessions work at once, each gets its own git worktree; never edit in a checkout another session is using. ## Boundaries & antipatterns