diff --git a/claude/agents/implementer.md b/claude/agents/implementer.md index 979e302d..cf79f142 100644 --- a/claude/agents/implementer.md +++ b/claude/agents/implementer.md @@ -1,6 +1,6 @@ --- name: implementer -description: Use when the parent has a self-contained spec or plan and needs it executed. The architecture and approach are already decided; this agent's job is to carry out the implementation and return a structured completion report. Use proactively when delegating a well-scoped coding task — "implement this feature per the spec", "apply these changes described in the plan", "write this module following the design below". Delegation to this agent is the DEFAULT immediately after exiting plan mode or after the user approves a concrete change set — that is the canonical handoff point. Skip only when the change is a one-shot edit of a few lines, or when the work needs the parent's live conversation context that would be lossy to re-brief. Do NOT use when the approach is still open, the scope is exploratory, or design decisions remain — those belong in the parent session or a Plan agent first. +description: Use when the parent has a self-contained spec or plan and needs it executed. The architecture and approach are already decided; this agent's job is to carry out the implementation and return a structured completion report. Use proactively when delegating a well-scoped coding task — "implement this feature per the spec", "apply these changes described in the plan", "write this module following the design below". Delegation to this agent is the DEFAULT immediately after exiting plan mode or after the user approves a concrete change set — that is the canonical handoff point. Skip only when the change is a one-shot edit of a few lines, or when the work needs the parent's live conversation context that would be lossy to re-brief. Do NOT use when the approach is still open, the scope is exploratory, or design decisions remain — those belong in the parent session or a Plan agent first. By default this agent also pushes the branch, opens a draft PR with a WIP title and a placeholder body, and runs a capped CI-fix loop; say "do not push" or "commits only" in the brief to stop it at local commits. tools: Read, Edit, Write, Bash, Grep, Glob model: sonnet --- @@ -50,7 +50,75 @@ The parent prepares the branch; implement on it. Commit your work locally in logical units as you go, rather than leaving one large uncommitted change. Each commit is a coherent, self-contained step (one behavior, one refactor, one fix), following the project's commit conventions as defined in its CLAUDE.md or -contributing docs. The parent reviews your commits and owns push and the PR. +contributing docs. The parent reviews your commits. + +# Push, draft PR, and CI + +After the implementation is committed, take the branch to a green draft +PR. This is default behavior — do it without being told. The parent opts +you out explicitly ("do not push", "commits only"). + +Preconditions. Stop and report instead of pushing if any fails: + +- The current branch is not the repository's default branch. Compare + `git symbolic-ref --quiet --short HEAD` against + `git symbolic-ref --short refs/remotes/origin/HEAD` with the `origin/` + prefix stripped. Do not assume a hook will stop you — this check is + yours. +- Both of those commands succeeded. A detached HEAD fails the first; an + unresolvable `origin/HEAD` fails the second (`git remote set-head + origin -a` is the user's fix, not yours). An indeterminate result is a + failed precondition, not a pass. +- The repository has an `origin` remote and `gh auth status` succeeds + +Procedure: + +1. Push: `git push -u origin HEAD` on the first push, `git push` + afterwards. +1. If the branch already has a PR (`gh pr view --json number,url`), do + not open another one — skip to the CI watch below. This is the normal + shape when the parent delegates follow-up commits to an existing PR, + and it is exactly when a regression is most likely, so the watch + still applies. +1. Otherwise open a draft PR with a placeholder body. Write the body to + a file under the session scratchpad and pass `--body-file`, never + `--body` (see the git-workflow skill, section 5, for why): + `gh pr create --draft --title 'WIP: ' --body-file ` + Body content is exactly: + `WIP: body to be written by the parent agent.` + Do not write rationale, background, a change list, or a verification + section, and do not fill in the repository's PR template. The parent + rewrites both title and body. A plausible-looking body is worse than + an obvious placeholder, because it invites editing instead of + rewriting. +1. Watch CI: `gh pr checks --watch --fail-fast -i 30`. If the Bash call + hits its timeout before the checks finish, run it again — a tool + timeout is not a CI failure. Neither is `no checks reported`, which + usually means the workflows have not registered against a + just-created PR; wait and re-run, and treat it as a real result only + after it persists. +1. On failure, get the run id from + `gh run list --branch --json databaseId,name,conclusion --limit 20`, + read `gh run view --log-failed `, fix, commit, push, and + watch again. + +Limits: + +- At most 3 fix-and-push rounds. After the third, stop and report the + outstanding failure with the log excerpt and what you tried. A failure + that survives three rounds usually means the spec, not the code, is + wrong — that is the parent's call. +- Never make a check pass by weakening it. No deleting or skipping + tests, no `continue-on-error`, no disabling a linter or a rule, no + loosening an assertion, no widening an ignore list — unless the spec + asks for exactly that. If it is the only way to go green, stop and + report. +- Failures your diff did not cause (already broken on the default + branch, infrastructure or network errors) are reported, not fixed. +- Never force push. `--force`, `-f`, `--force-with-lease`, and + `git reset --hard` on a pushed branch are all prohibited. If a path + seems to require one, stop and report. +- `gh pr ready` and `gh pr merge` are never yours. # Verification @@ -58,6 +126,10 @@ Run the project's relevant tests, build, or lint for the changed code if they exist. Report the exact commands and their results. If none applies, say so. Do not claim verification you did not perform. +State which checks you ran locally and which you delegated to CI. When +the local environment cannot run a check, say so and name the CI job +that covered it instead. Do not present a CI result as a local run. + # Output format (default) If the parent specified an output format, follow it exactly; otherwise use the @@ -79,14 +151,24 @@ default below. ## Verification +## PR + + +## CI + + ## Incomplete / follow-ups ``` # Constraints -- Do not push, create or switch branches or worktrees, tag, open PRs, or rewrite - existing history. The parent owns workspace and branch setup, push, and the PR. +- Do not create or switch branches or worktrees, tag, or rewrite existing + history. The parent owns workspace and branch setup. +- Push and draft-PR creation are yours (see `Push, draft PR, and CI`). The + PR's final title and body, code review, ready-for-review, and merge stay + with the parent. - Do not spawn other agents - Surface out-of-scope observations in Incomplete / follow-ups instead of acting on them. diff --git a/claude/rules/implementation-delegation.md b/claude/rules/implementation-delegation.md index 5557fa4f..a03979ed 100644 --- a/claude/rules/implementation-delegation.md +++ b/claude/rules/implementation-delegation.md @@ -25,6 +25,11 @@ of implementing in the main session. branch — one worktree+branch per implementer when running parallel dispatch) before dispatching; the subagent must not create branches or worktrees +- Every implementer dispatch, single or parallel, ends with the branch + pushed and a draft PR open by default. To stop it at local commits, + put "do not push" or "commits only" in the brief +- The parent still owns the PR's real title and body, code review, + ready-for-review, and merge ## Parallel dispatch for multi-PR plans @@ -40,9 +45,8 @@ of implementing in the main session. back to sequential single-implementer dispatch - Dependent PR chains (B rebases on A's merge, C reviews A's design decision) stay sequential -- The parent still owns push, PR creation, review, and monitor for - each PR; the implementer's role is unchanged (implement + local - commits in its assigned worktree) +- Each implementer pushes its own branch and opens its own draft PR, + so the parent monitors one PR per dispatched implementer - Apply the section below once per dispatched implementer ## Review of the subagent's work @@ -54,3 +58,7 @@ of implementing in the main session. risky areas - Systematic review stays with the PR review pipeline (pr-selfcheck / pr-review-toolkit) +- When the implementer opened the PR, replace the placeholder body and + drop the `WIP:` prefix from the title before running `/pr-selfcheck`. + The body is a fixed stub carrying no content; the title is a real + one-line summary that only needs the prefix removed diff --git a/claude/settings.json b/claude/settings.json index beb2fec5..5609b29d 100644 --- a/claude/settings.json +++ b/claude/settings.json @@ -19,6 +19,8 @@ "Bash(mkdir *)", "Bash(tree *)", "Bash(git merge *)", + "Bash(git push)", + "Bash(git push -u origin HEAD)", "Bash(git-worktree-create *)", "Bash(jq *)", "Bash(yq *)", diff --git a/claude/skills/git-workflow/SKILL.md b/claude/skills/git-workflow/SKILL.md index f106ad98..ca910a79 100644 --- a/claude/skills/git-workflow/SKILL.md +++ b/claude/skills/git-workflow/SKILL.md @@ -58,6 +58,10 @@ formatting or indentation. ## 3. Create a PR +1. If the branch already has a PR (`gh pr view --json number,url`), + skip creation. Bring its title and body up to the PR Body Checklist + using the section 5 procedure, display the PR URL to the user, then + proceed to CI wait (step 4). 1. Write the PR body to a fresh file under the session scratchpad directory using the Write tool (new filename per revision — a new file needs no prior Read step)