familienarchiv/.claude/skills/deliver-issue/SKILL.md

---
name: deliver-issue
description: Full end-to-end delivery of a Gitea issue for the Familienarchiv project — six-persona review → theme-grouped discussion walking through EVERY raised point with the user → isolated git worktree → TDD implementation → PR → review+fix loop until all personas approve (max 10 cycles). Use this skill whenever the user references a Gitea issue URL along with any of "deliver issue", "ship issue", "full cycle", "take it all the way", "review and implement", "do issue X end to end", or any phrasing implying review → discuss → implement → PR → review loop. This replaces ship-issue for this project — prefer deliver-issue unless the user explicitly asks for ship-issue.
---

# Deliver Issue — Review → Discuss → Implement → PR → Review Loop

Own the full lifecycle for a Gitea issue. Two human checkpoints, everything else autonomous. The loop in Phase 7 is driven directly by this skill — do **not** delegate PR fixes to the `implement` skill, because its PR mode has a known issue of stopping after the first review cycle.

## Input

A Gitea issue URL. Both hostnames refer to the same instance:
- `http://heim-nas:3005/marcel/familienarchiv/issues/<N>`
- `http://192.168.178.71:3005/marcel/familienarchiv/issues/<N>`

Parse: `owner = marcel`, `repo = familienarchiv`, `issue_number = <N>`.

---

## Phase 0 — Multi-Persona Review (autonomous)

Invoke the `review-issue` skill with the issue URL. It reads the issue, loads all six personas from `.claude/personas/`, and posts one comment per persona to the Gitea issue.

Wait for it to finish. Do not proceed until the six comments are posted.

**Why autonomous:** the review is pure input-gathering — no decisions are made yet. The next phase is where the human gets involved.

---

## Phase 1 — Consolidate Every Point by Theme (autonomous)

Re-read the issue and every persona comment from Phase 0 using `mcp__gitea__issue_read` (method `get_comments`).

Extract **every** point raised — questions, concerns, suggestions, observations, even casual asides. Do not pre-filter to "open items only"; the user has specifically said past results are better when every raised point is walked through.

Group points by **theme**, not by persona. A theme is a topical cluster — what the point is *about*, not who said it. Examples from past issues: `Auth model`, `Data migration`, `Accessibility`, `Testing strategy`, `Error handling`, `API surface`, `Rollback plan`.

For each theme:

1. Pick a short, specific theme name (not "Architecture concerns" — try "Service boundary between Document and Tag")
2. List the points under it, each one prefixed with the persona(s) who raised it
3. Dedupe near-identical points across personas but preserve attribution — if Felix and the tester both asked the same thing, note both

Order themes by blast radius / blocking potential:
- **First**: anything that shapes the data model, API, or irreversible architectural decisions
- **Middle**: implementation approach, testing strategy, error handling
- **Last**: polish — naming, copy, accessibility nits, follow-up ideas

Example output shape (show this to the user before starting the walk-through):

```
## Themes to Discuss — Issue #<N>

I've grouped the persona reviews into themes. We'll walk through every point.

### 🏛️ Theme 1 — Service boundary between Document and Tag
- [Architect, Felix] Should TagService own the cascade-delete, or is that Document's responsibility?
- [Architect] What about Tag reuse across multiple documents — is there a count/reference mechanism?

### 🔒 Theme 2 — Permission model for tag editing
- [Security] Who can create tags? Reuse them? Admin-only?
- [Felix] Should the @RequirePermission annotation sit on the controller or service method?

### 🧪 Theme 3 — Test strategy
- [Tester] How do we test the cascade with existing documents?
- [Tester, Security] Do we need a test for the unauthorized-user path?

### 💅 Theme 4 — UI feedback on tag operations
- [UI] Optimistic update vs. wait-for-server?
- [UI] Toast on success, or silent?

Ready to start with Theme 1?
```

Stop and wait for the user's go-ahead before proceeding.

---

## Phase 2 — Interactive Walk-Through (HUMAN CHECKPOINT)

Work through the themes **in order**, and within each theme walk through **every point**.

For each point:

1. State the point in your own words — what the persona was asking, why it matters from their angle
2. Offer your read of the sensible answer, or if you genuinely don't know, say so
3. Ask a focused, specific question — one question, not three
4. Wait for the user's response
5. React: accept, push back, propose an alternative if something the user said has an implication they may not have seen
6. When the point feels resolved, record the decision internally and move to the next point

Stay substantive. The value of this phase is the back-and-forth — don't rush through it. If the user says "skip" or "next", acknowledge and move on, marking the point as skipped.

After the last point of the last theme, show a summary:

```
## Summary of Decisions

### Theme 1 — Service boundary between Document and Tag
- TagService owns cascade-delete. Document calls TagService.detachAll(docId) on deletion.
- Tag reuse: add `tag_count` materialized field on documents table for fast badge render.

### Theme 2 — Permission model
- Admins-only for tag create. Reuse is open to all WRITE_ALL users.
- @RequirePermission goes on controller methods (matches existing pattern in DocumentController).

...
```

Then ask:

> Ready to post these resolutions to the issue as a consolidated comment?

Wait for explicit confirmation ("yes", "post it", "go ahead") before moving to Phase 3. If the user wants edits, loop back and adjust.

---

## Phase 3 — Post Consolidated Resolutions (autonomous)

Post a single comment on the issue via `mcp__gitea__issue_write` (method `add_comment`).

Format:

```markdown
# 🎯 Discussion Resolutions

After reviewing the persona feedback with the user, here are the agreed decisions:

## Theme 1 — <name>
- **Decision**: ...
- **Rationale**: ...

## Theme 2 — <name>
...

---

These resolutions now act as the authoritative design for implementation. The `implement` skill will read this comment alongside the original issue.
```

Include every resolved theme. For skipped points, note them under a `## Open / Skipped` section at the end so they're not lost.

---

## Phase 4 — Create Isolated Worktree (autonomous)

Derive a short slug from the issue title: lowercase, hyphens instead of spaces, drop punctuation, max ~40 chars. E.g. "Admin: tag overhaul for bulk operations" → `admin-tag-overhaul`.

From the project root (`/home/marcel/Desktop/familienarchiv`):

```bash
git fetch origin
git worktree add ../familienarchiv-issue-<N> -b feat/issue-<N>-<slug> origin/main
cd ../familienarchiv-issue-<N>
```

**Why a sibling worktree:** the user's main workspace stays untouched so other work can continue in parallel. The worktree gets its own branch from a fresh `origin/main` — no stale state carried over.

Report the worktree path to the user in one line before moving on. All subsequent phases run inside this worktree.

---

## Phase 5 — Implement (HUMAN CHECKPOINT — plan approval)

Invoke the `implement` skill with the issue URL.

The `implement` skill will:
1. Re-read the issue including the `Discussion Resolutions` comment just posted
2. Ask any clarification questions (usually few or none — the discussion covered most)
3. Present an implementation plan as a numbered TDD task list
4. **Pause for plan approval** — this is the second human checkpoint

**Why keep this pause** even after the full discussion: the plan is where abstract decisions meet concrete test order and file touches. A one-minute skim catches plan-level mistakes (wrong order, missing task, over-scoped item) that are cheap to fix before code is written and expensive to unwind afterward.

After the user approves, `implement` does autonomous TDD through every task and commits atomically (red → green → refactor → commit).

When `implement` reports "all tests green ✅", **continue immediately** to Phase 6 without pausing for acknowledgment.

---

## Phase 6 — Open Pull Request (autonomous)

From inside the worktree:

1. Push: `git push -u origin HEAD`
2. Fetch issue title via `mcp__gitea__issue_read` (method `get`)
3. Create PR via `mcp__gitea__pull_request_write` (method `create`):

```
owner:  marcel
repo:   familienarchiv
head:   feat/issue-<N>-<slug>
base:   main
title:  <exact issue title>
body: |
  Closes #<N>

  ## Summary
  <one paragraph summarizing what was built, referencing the Discussion Resolutions>
```

Capture the PR index from the response. Announce:

> PR #<index> opened: http://heim-nas:3005/marcel/familienarchiv/pulls/<index>

Continue immediately to Phase 7.

---

## Phase 7 — Review + Fix Loop (autonomous, max 10 cycles, owned by this skill)

Initialize `cycle = 1`. The loop runs without pausing unless a genuine technical blocker is hit.

### Step A — Run review-pr

Announce: `🔍 Review cycle <cycle>/10`

Invoke the `review-pr` skill with the PR URL. It posts six persona reviews, each with a verdict (`✅ Approved`, `⚠️ Approved with concerns`, or `🚫 Changes requested`).

Read the summary `review-pr` reports back.

- **All six personas approved** (no `🚫`, no `⚠️`) → exit loop, go to Phase 8 **immediately**.
- **Any concerns or blockers** → proceed to Step B **immediately**, no pause.

### Step B — Address Every Concern (don't delegate to implement)

If `cycle == 10`: stop, go to the cycle-limit handoff at the end of this phase.

**Do the work in this skill directly.** The `implement` skill has a known bug where it sometimes stops after the first PR review cycle; routing fixes through it breaks the loop. Apply the same TDD discipline inline:

**1. Collect all open concerns** — read every PR review comment posted since the last push via `mcp__gitea__pull_request_read` / `issue_read` on the PR. Build a flat list:
- Blockers
- Suggestions / concerns
- Unanswered questions

Tag each with the persona who raised it and a short quote so the commit + summary comment can reference them.

**2. Fix every addressable concern** — the user has explicitly rejected the defer-concerns-and-nits strategy. Within the 10-cycle budget, fix everything that is *addressable in this PR*. For each concern:

- **Red**: write a failing test that captures the required behavior (for code concerns) or a check that fails today (for config/infra concerns)
- **Green**: minimum code to pass; run the full test suite
- **Refactor**: only if there's actual duplication or naming cleanup
- **Commit**: atomic per concern, message referencing the persona and excerpt:

```
fix(scope): address <persona> — <short quote>

<optional explanation>

Co-Authored-By: Claude <noreply@anthropic.com>
```

Test commands for this project:
- Backend: `cd backend && ./mvnw test` (single class: `./mvnw test -Dtest=ClassName`)
- Frontend unit tests: `cd frontend && npm run test`
- Frontend type check: `cd frontend && npm run check`
- Full backend build: `cd backend && ./mvnw clean package -DskipTests`

**3. Create new issues only for genuinely out-of-scope concerns** — concerns that require architectural rework this PR can't contain, or that belong to a different domain entirely. Use `mcp__gitea__issue_write` (method `create`):

```
title: <short description>
body: |
  ## Background
  Raised during PR #<pr_index> review cycle <cycle>.

  ## Concern
  <persona name, quoted text>

  ## Why deferred
  <why this belongs in its own issue, not this PR>

  ## Reference
  PR: http://heim-nas:3005/marcel/familienarchiv/pulls/<pr_index>
```

The bar for "out of scope" is high — reach for it only when the concern genuinely doesn't belong in this PR. Everything else gets fixed.

**4. Push and post a summary comment** — once all fixable concerns are committed:

```bash
git push
```

Post one PR comment via `mcp__gitea__issue_write` (PRs share the comment API):

```markdown
## Review Cycle <cycle> — Changes

### Addressed
- [@developer] Magic number replaced with `MAX_RESULTS` constant — commit `<sha>`
- [@security] Added input validation for tag name length — commit `<sha>`
- ...

### Deferred to new issues
- [@architect] Redesign of permission cascade — #<new_issue_number>

Re-running review cycle <cycle+1>.
```

**5. Loop** — increment `cycle`, return to Step A. No pause, no confirmation.

### If cycle 10 is reached without full approval

Stop. Report:

```
⚠️ Reached 10 review/fix cycles — remaining open concerns:

<list per-persona concerns still open>

PR: <url>
Worktree: <path>

How would you like to proceed? Options: continue manually, merge as-is, close.
```

Let the user decide. Do not make this decision autonomously.

---

## Phase 8 — Final Report

All six personas approved. Report:

```
✅ Delivery complete — PR #<index> fully approved

Cycles: <cycle - 1> review/fix round(s)
PR:       http://heim-nas:3005/marcel/familienarchiv/pulls/<index>
Worktree: /home/marcel/Desktop/familienarchiv-issue-<N>
Branch:   feat/issue-<N>-<slug>

Ready for manual merge.
```

Do not merge the PR automatically — merge is the user's final gate.

---

## Operating Notes

- **Two human checkpoints, nothing else.** Phase 2 (walk-through) and Phase 5 (plan approval). Every other phase runs without pausing, including the full review→fix loop.
- **Genuine blockers pause the flow.** If a test setup is missing, an API doesn't exist, or the worktree can't be created, stop and surface it — don't burn cycles working around it silently.
- **Worktree isolation means other work continues.** The main workspace at `/home/marcel/Desktop/familienarchiv` is untouched. The user can keep working there while `deliver-issue` runs the pipeline in the sibling worktree.
- **Posting side effects are real.** Phase 0 posts six comments to Gitea. Phase 3 posts the resolutions comment. Phase 6 opens a PR. Each review cycle posts six review comments plus one summary comment. Don't run this skill on an issue you're still drafting.
- **If the user interrupts mid-loop**, honor it. Stop where you are and let them redirect.