Skip to content

Commit 1b2cfba

Browse files
[Update] enforce mandatory PR workflow on extension commands
Adds Phase PR (commit + push + gh pr create) to both /implement-extensions and /implement-extensions-batch as a mandatory final phase, and codifies the rule in CLAUDE.md as 'Branch & PR workflow (MANDATORY)'. Every branch must reach an open PR against development; direct pushes to development or master are forbidden.
1 parent 6bf73d6 commit 1b2cfba

3 files changed

Lines changed: 129 additions & 19 deletions

File tree

.claude/commands/implement-extensions-batch.md

Lines changed: 60 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -327,11 +327,61 @@ step-11 logic from `/implement-extensions`:
327327
6. Push via `gh issue edit <num> --body-file <tmp-body-file>`.
328328
7. Re-fetch + diff to verify only the Checklist section changed.
329329

330-
### 14. Final summary
330+
### 14. Phase PR — Commit, push, open PR (MANDATORY)
331+
332+
This phase is non-skippable. Every batch run ends with a pushed branch and an open PR against `development`. See the user-memory rule `feedback_pr_mandatory.md` and the CLAUDE.md "Branch & PR workflow (MANDATORY)" section.
333+
334+
1. **Stage in-scope files EXPLICITLY** — NEVER `git add -A` / `git add .`. Stage only:
335+
- the N production files `SysML2.NET/Extend/<Foo>Extensions.cs`,
336+
- the N test fixtures `SysML2.NET.Tests/Extend/<Foo>ExtensionsTestFixture.cs`,
337+
- any sibling fixtures the regression sweep (step 11) touched.
338+
339+
```bash
340+
git add SysML2.NET/Extend/<Foo1>Extensions.cs SysML2.NET/Extend/<Foo2>Extensions.cs … \
341+
SysML2.NET.Tests/Extend/<Foo1>ExtensionsTestFixture.cs … \
342+
<touched-sibling-fixtures>
343+
```
344+
345+
`.team-notes/` is gitignored so it stays local automatically.
346+
347+
**If the working tree contains files outside the in-scope list** (e.g. instruction-file updates surfaced mid-run, scratch files), do NOT stage them in this commit. Either split them into a separate follow-up commit on the same branch (recommended) or surface to the user and pause.
348+
349+
2. **Commit** with the canonical batch message — single line, no body, no trailers, no `--no-verify`:
350+
351+
```bash
352+
git commit -m "Fix #<n1> #<n2> #<n3> …"
353+
```
354+
355+
The numbers are exactly the GitHub issue numbers handled by this batch, in the original `$ARGUMENTS` order (or, if the user expressed a preferred order in the invocation prompt, that order).
356+
357+
3. **Push** the branch to `origin`:
358+
359+
```bash
360+
git push -u origin <branch-name>
361+
```
362+
363+
NEVER `--force`. NEVER `--force-with-lease`. If the push is rejected because the branch diverged from origin (unlikely for a fresh batch branch, but possible if the user pushed manually mid-run), surface the conflict and stop.
364+
365+
4. **Open the PR** against `development`:
366+
367+
```bash
368+
gh pr create --base development --head <branch-name> \
369+
--title "Fix #<n1> #<n2> #<n3> …" \
370+
--body-file <pr-body-tmp-file>
371+
```
372+
373+
The PR body is the per-file table + branch-wide totals from step 15, without the pre-filled-commit-message block (the commit has already been made). NEVER use `--draft` unless the user explicitly asked. NEVER use `--base master`.
374+
375+
5. **Capture the PR URL** from `gh pr create`'s stdout and pass it to step 15.
376+
377+
If the current branch is `development` or `master` at this point (defensive check; should never happen for a batch run), ABORT the workflow — feature work belongs on a feature branch. Surface to the user and do not push.
378+
379+
### 15. Final summary
331380

332381
Print to the user:
333382

334-
- **Branch**: name + base ref + how to delete-if-aborting.
383+
- **Branch**: name + base ref.
384+
- **PR**: `<pr-url>` (captured from step 14.5).
335385
- **Per-file table**:
336386

337387
| File | Stubs impl. | Targeted tests | Reg. sweep impact | Reviewer | Issue |
@@ -340,28 +390,20 @@ Print to the user:
340390
| `<Foo2>Extensions.cs` ||||||
341391

342392
- **Branch-wide totals**:
343-
- Files modified (sum of production + tests + notes).
393+
- Files modified (sum of production + tests + sibling fixtures touched). `.team-notes/` are gitignored and not committed.
344394
- Full solution test count (e.g. `1082/1082`).
345395
- Unresolved reviewer findings (if any).
346396
- Spec-text-only methods flagged separately (grounded in spec prose, not OCL).
347397
- Out-of-scope blockers surfaced (e.g. "VerifyComputeX in <Sibling>TestFixture
348398
is still stub-blocked on `<UpstreamMethod>` — consider a follow-up issue").
349399

350-
- **Pre-filled commit message** (MANDATORY — append at the very end of the
351-
final-summary message in a fenced code block, ready to copy):
400+
- **Commit message used** (informational — the commit is already on the branch):
352401

353402
```
354403
Fix #<n1> #<n2> #<n3> …
355404
```
356405

357-
Where the numbers are exactly the GitHub issue numbers handled by this
358-
batch, in the original `$ARGUMENTS` order (or, if the user expressed a
359-
preferred order in the invocation prompt, that order). Nothing else —
360-
no body paragraphs, no per-file bullet list, no `Co-Authored-By` trailer,
361-
no "🤖 Generated with …" footer. The single line is the entire message.
362-
363-
- **Reminder**: nothing is auto-committed. User reviews `git diff`, decides
364-
whether to commit / push / open PR.
406+
- **PR is the review surface.** The user reviews the PR on GitHub — do not ask them to review `git diff` locally.
365407

366408
## Failure handling
367409

@@ -380,6 +422,10 @@ Print to the user:
380422
| Reviewer NEEDS FIX for one or more files | Step 12 | Re-dispatch THE implementer or THE tester with a focused brief naming the broken file(s); other files' results still reported. |
381423
| Implementer's context runs out mid-batch | Any IT/V step | Re-dispatch THE implementer with a focused brief covering only the unfinished file(s). Partial progress on disk is preserved. |
382424
| Batch partially fails after branch + assignment | Any step ≥ 6 | Keep the branch; surface in final summary; user decides whether to retry via `/implement-extensions` for the still-broken single file or revert. |
425+
| `git push` rejected (branch diverged from origin) | Phase PR (step 14.3) | Abort the push, surface to user, do NOT force-push. |
426+
| `gh pr create` fails (no GitHub auth, repo write permission missing) | Phase PR (step 14.4) | Surface the gh error, leave the branch pushed, advise the user to open the PR manually. |
427+
| Working tree contains files outside the in-scope list at Phase PR | Phase PR (step 14.1) | Refuse to stage them in the `Fix #…` commit. Either split into a separate follow-up commit on the same branch or surface to the user. |
428+
| Current branch is `development` or `master` at Phase PR | Phase PR (defensive) | ABORT the workflow — feature work belongs on a feature branch. Surface to the user. |
383429

384430
## Parallelism caps (orchestrator self-enforced)
385431

@@ -407,7 +453,7 @@ context per role.
407453
mistranslation.
408454
- The branch and the assignments persist even on partial failure. Be explicit
409455
in the final summary about which files succeeded vs which need follow-up.
410-
- Do NOT auto-commit. The user reviews `git diff` and commits manually.
456+
- **Phase PR is MANDATORY** (step 14). Commit + push + open PR at the end of every batch run. The PR is the user's review surface; do NOT ask the user to review `git diff` and commit manually. See `feedback_pr_mandatory.md` and CLAUDE.md "Branch & PR workflow (MANDATORY)".
411457
- If the user supplies a single file, route them to `/implement-extensions`
412458
with the same filename rather than creating a degenerate 1-file "batch"
413459
branch.

.claude/commands/implement-extensions.md

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -380,8 +380,8 @@ Report to the user:
380380
knows the implementation is grounded in spec prose rather than OCL.
381381
- **Issue checklist sync**: `<issue-url>``<newly-ticked>` newly ticked,
382382
`<newly-added>` newly added, `<ticked>/<total>` total (filled in after step 11).
383-
- **Pre-filled commit message** (MANDATORY — append at the very end of the
384-
final-summary message in a fenced code block, ready to copy):
383+
- **PR URL**: `<pr-url>` (captured in step 12 / Phase PR).
384+
- **Commit message used** (informational — the commit is already on the branch):
385385

386386
```
387387
Fix #<n>
@@ -391,7 +391,7 @@ Report to the user:
391391
no body paragraphs, no per-method bullet list, no `Co-Authored-By` trailer,
392392
no "🤖 Generated with …" footer. The single line is the entire message.
393393

394-
Do NOT auto-commit. The user reviews and commits.
394+
The PR is the user's review surface — do NOT ask them to review `git diff` and commit manually. See CLAUDE.md "Branch & PR workflow (MANDATORY)" and `feedback_pr_mandatory.md`.
395395

396396
### 11. Sync GitHub issue checklist
397397

@@ -452,6 +452,50 @@ unresolved findings are separately surfaced in the final-summary report. The
452452
newly ticked items, count of newly added items, and the resulting
453453
`<ticked>/<total>` ratio.
454454

455+
### 12. Phase PR — Commit, push, open PR (MANDATORY when on a non-development branch)
456+
457+
This phase is non-skippable. Every `/implement-extensions` run ends with a pushed branch and an open PR against `development`. See the user-memory rule `feedback_pr_mandatory.md` and the CLAUDE.md "Branch & PR workflow (MANDATORY)" section.
458+
459+
**Defensive guard**: check the current branch first.
460+
461+
```bash
462+
git branch --show-current
463+
```
464+
465+
If it is `development` or `master`, ABORT — surface a hard refusal to the user. Feature work belongs on a feature branch first; the user should `git switch -c <feature-branch>` then re-invoke. Do NOT push.
466+
467+
Otherwise:
468+
469+
1. **Stage in-scope files EXPLICITLY** — NEVER `git add -A` / `git add .`:
470+
```bash
471+
git add SysML2.NET/Extend/<FOO>Extensions.cs \
472+
SysML2.NET.Tests/Extend/<FOO>ExtensionsTestFixture.cs \
473+
<any-touched-sibling-fixtures>
474+
```
475+
`.team-notes/` is gitignored so it stays local automatically.
476+
477+
2. **Commit** with the canonical single-issue message:
478+
```bash
479+
git commit -m "Fix #<n>"
480+
```
481+
Single line, no body, no trailers, no `--no-verify`.
482+
483+
3. **Push** the branch:
484+
```bash
485+
git push -u origin <branch>
486+
```
487+
NEVER `--force` / `--force-with-lease`. If push is rejected because the branch diverged, surface the conflict and stop.
488+
489+
4. **Open PR** against `development`:
490+
```bash
491+
gh pr create --base development --head <branch> \
492+
--title "Fix #<n>" \
493+
--body-file <pr-body-tmp>
494+
```
495+
PR body is the per-method test result + reviewer verdict from step 10. NEVER `--base master`.
496+
497+
5. **Capture the PR URL** and feed it back into the step-10 final-summary line.
498+
455499
## Notes for the orchestrator (you, the main agent)
456500

457501
- Pick the model per role using the complexity-grading rubric in step 3.5.
@@ -482,6 +526,7 @@ unresolved findings are separately surfaced in the final-summary report. The
482526
implementation state of the file; unresolved findings are separately surfaced
483527
in the final-summary report. The `gh issue edit` push must touch ONLY the
484528
`### Checklist` section — verify with a re-fetch + diff before reporting "done".
529+
- **Phase PR (step 12) is MANDATORY.** Commit + push + open PR at the end of every run. The PR is the user's review surface; do NOT ask the user to review `git diff` and commit manually. See `feedback_pr_mandatory.md` and CLAUDE.md "Branch & PR workflow (MANDATORY)". Refuse to push if the current branch is `development` or `master`.
485530
- **Plan mode is handled by Gate 0 at the top of this file** — the orchestrator writes the proposed-execution plan to the plan file, calls `ExitPlanMode`, and proceeds on approval. The orchestrator never spawns sub-agents while plan mode is active, so the previous "degraded mode" workaround is no longer needed and has been removed.
486531
- **Gate R-A (step 5.5) is mandatory every run.** It is the only structural checkpoint between the researcher returning `spec ready` and the implementer + tester being spawned. Do not skip it even when the researcher reports zero ambiguities — the user explicitly asked for an unconditional gate so they can review per-method derivations before code is written.
487532
- **Tool-level prompts (`Bash(...)`, `Edit(...)`, `Write(...)`, `Agent(...)`) still surface per the user's `settings.json`.** Gate 0 and Gate R-A govern STRUCTURAL approval only. The user has explicitly chosen to keep handling tool-level prompts manually rather than auto-allowing them via permission rules or hooks.

CLAUDE.md

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,31 @@ Auto-generated DTOs use structured namespaces reflecting the KerML/SysML package
142142

143143
## Key Conventions
144144

145-
- Commit messages use prefix tags: `[Add]`, `[Update]`, `[Remove]`, `[Fix]`
146-
- Main branch: `master`. Development branch: `development`
145+
- Commit messages use prefix tags: `[Add]`, `[Update]`, `[Remove]`, `[Fix]` — except for issue-fixing commits produced by `/implement-extensions` and `/implement-extensions-batch`, which use the canonical short form `Fix #<n>` (single issue) or `Fix #<n1> #<n2> …` (batch) so GitHub auto-closes the issues on merge.
146+
- Main branch: `master`. Development branch: `development`. **All feature work targets `development`** via PR; `master` is downstream only.
147147
- CI: GitHub Actions (`CodeQuality.yml`) — builds, tests, and runs SonarQube analysis
148148
- License: Apache 2.0 (code), LGPL v3.0 (metamodel files)
149149
- To add a new metaclass: update the UML XMI source files, then run the code generators — do not manually create AutoGen files
150150

151+
## Branch & PR workflow (MANDATORY)
152+
153+
Every branch must be pushed and have an open PR against `development` before any task is reported "done". The PR is the user's review surface — the agent does NOT ask the user to review `git diff` manually before committing. Direct pushes to `development` or `master` are forbidden.
154+
155+
Operationally, at the end of any task that creates a branch (most commonly `/implement-extensions` and `/implement-extensions-batch`):
156+
157+
1. **Stage in-scope files explicitly**`git add <path1> <path2> …`. NEVER `git add -A` or `git add .` (sensitive files / drift can leak).
158+
2. **Commit** with the canonical short message: `Fix #<n>` for single-issue runs, `Fix #<n1> #<n2> …` for batches. Single line. No body, no `Co-Authored-By` trailer, no "🤖 Generated with …" footer, no `--no-verify`.
159+
3. **Push**: `git push -u origin <branch>`. NEVER `--force`, NEVER `--force-with-lease`.
160+
4. **Open PR**: `gh pr create --base development --head <branch> --title "Fix #<n>…" --body-file <pr-body-tmp>`. Capture the PR URL and surface it in the final summary.
161+
5. **If the current branch is `development` or `master`**: REFUSE the workflow. Surface to the user — feature work must live on a feature branch first.
162+
163+
Failure modes:
164+
- `git push` rejected (branch diverged from origin) → abort, surface, do NOT force-push.
165+
- `gh pr create` fails (no auth, no write permission) → leave the branch pushed, advise the user to open the PR manually.
166+
- Mid-task non-payload edits (e.g. instruction-file updates surfaced during a batch) → split into separate commits on the same branch so the `Fix #…` commit carries only the issue-resolving payload.
167+
168+
This policy reverses any older slash-command snippet that says "Do NOT auto-commit; the user reviews `git diff` and commits manually" — those snippets have been superseded by Phase PR.
169+
151170
## Quality rules
152171

153172
- Prefer comparing 'Count' to 0 rather than using 'Any()', both for clarity and for performance

0 commit comments

Comments
 (0)