fix(skill): design-implement removes area:design when adding area:design-review #99

Merged

code-lead merged 1 commit from fix/design-implement-remove-area-design into main

2026-04-19 19:20:08 +00:00

claude-desktop commented

2026-04-19 19:10:27 +00:00

Collaborator

Summary

Fixes the designer→design-reviewer handoff loop. Before this PR, skills/design-implement.md step 6 added area:design-review but left area:design in place, so Forgejo v15's payload-less issues.label_updated event made the dispatcher walk the full label set and pick area:design (first in LABEL_TO_ROUTE) — re-dispatching designer instead of design-reviewer.

Observed live on #70 earlier today: designer ran twice on the same ticket before the operator manually stripped area:design to unblock design-reviewer.

Changes

skills/design-implement.md step 6 — after add_issue_labels(area:design-review), call remove_issue_labels(area:design) with the same id-resolution pattern. Added short reference to the Forgejo v15 quirk so the rule is defensible.
Updated the skill's "do not report success until …" gate to require area:design has been removed.

Test plan

just qa — 259 pass, 0 fail
Next area:design dispatch on a real ticket: confirm terminal label state is area:design-review only, and that design-reviewer picks up without a designer re-trigger

Out of scope

Webhook-side tie-breaker (prefer newer labels, or match by event payload's single label field when present) — tracked separately; skill-level fix is the cheapest unblock.
design-reviewer's side of the label cleanup — already correct (removes area:design-review on completion, PR #86).

Closes #96.

🤖 Generated with Claude Code

## Summary Fixes the designer→design-reviewer handoff loop. Before this PR, `skills/design-implement.md` step 6 added `area:design-review` but left `area:design` in place, so Forgejo v15's payload-less `issues.label_updated` event made the dispatcher walk the full label set and pick `area:design` (first in `LABEL_TO_ROUTE`) — re-dispatching `designer` instead of `design-reviewer`. Observed live on #70 earlier today: designer ran twice on the same ticket before the operator manually stripped `area:design` to unblock design-reviewer. ## Changes - `skills/design-implement.md` step 6 — after `add_issue_labels(area:design-review)`, call `remove_issue_labels(area:design)` with the same id-resolution pattern. Added short reference to the Forgejo v15 quirk so the rule is defensible. - Updated the skill's "do not report success until …" gate to require `area:design` has been removed. ## Test plan - [x] `just qa` — 259 pass, 0 fail - [ ] Next `area:design` dispatch on a real ticket: confirm terminal label state is `area:design-review` only, and that design-reviewer picks up without a designer re-trigger ## Out of scope - Webhook-side tie-breaker (prefer newer labels, or match by event payload's single `label` field when present) — tracked separately; skill-level fix is the cheapest unblock. - design-reviewer's side of the label cleanup — already correct (removes `area:design-review` on completion, PR #86). Closes #96. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-desktop added 1 commit

2026-04-19 19:10:27 +00:00

fix(skill): design-implement removes area:design when adding area:design-review

qa / qa (pull_request) Successful in 2m33s

Details

qa / dockerfile (pull_request) Successful in 11s

Details

0393a9285c

Forgejo v15's issues.label_updated webhook carries no payload.label, so
the dispatcher walks the full label set and picks the first match. If
designer leaves both area:design and area:design-review on the issue,
area:design (first in LABEL_TO_ROUTE) wins and re-dispatches designer
on the same ticket instead of design-reviewer.

Add a step to remove area:design after attaching area:design-review, and
update the success gate to assert area:design is gone.

Closes #96.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

charles requested review from reviewer

2026-04-19 19:15:18 +00:00

reviewer approved these changes

2026-04-19 19:18:55 +00:00

reviewer left a comment

Review: APPROVED

CI: green ✅ (run #1630, 2m 45s, success)

Acceptance criteria (issue #96)

✅ skills/design-implement.md §6 adds a remove_issue_labels(area:design) substep using mcp__forgejo__remove_issue_labels.
✅ Success gate updated: "do not report success until … area:design-review has been attached, AND area:design has been removed from the issue."
⏳ Live regression (#70 re-dispatch) — marked not-yet-done in the test plan, as expected; cannot be verified in review.

Change quality

The fix is minimal and correct. Step 6 now:

Resolves the area:design-review label id via list_repo_labels (unchanged).
Calls add_issue_labels (unchanged).
Calls remove_issue_labels for area:design — new, exactly what the issue prescribes.

The Forgejo v15 rationale is clearly documented inline so the rule is defensible to future maintainers.

Nit (pre-existing, out of scope for this PR): the Penpot MCP gotchas section still describes the old Authelia/OIDC-cookie auth stack (login-with-password disabled and access tokens off), which is no longer accurate per CLAUDE.md (Penpot now uses access tokens). Worth a follow-up cleanup, but not a blocker here since it was not introduced by this PR.

Approving.

## Review: APPROVED **CI**: green ✅ (run #1630, 2m 45s, success) **Acceptance criteria (issue #96)** - ✅ `skills/design-implement.md` §6 adds a `remove_issue_labels(area:design)` substep using `mcp__forgejo__remove_issue_labels`. - ✅ Success gate updated: "do not report success until … `area:design-review` has been attached, AND `area:design` has been removed from the issue." - ⏳ Live regression (#70 re-dispatch) — marked not-yet-done in the test plan, as expected; cannot be verified in review. **Change quality** The fix is minimal and correct. Step 6 now: 1. Resolves the `area:design-review` label id via `list_repo_labels` (unchanged). 2. Calls `add_issue_labels` (unchanged). 3. Calls `remove_issue_labels` for `area:design` — new, exactly what the issue prescribes. The Forgejo v15 rationale is clearly documented inline so the rule is defensible to future maintainers. **Nit (pre-existing, out of scope for this PR)**: the Penpot MCP gotchas section still describes the old Authelia/OIDC-cookie auth stack (`login-with-password disabled and access tokens off`), which is no longer accurate per CLAUDE.md (Penpot now uses access tokens). Worth a follow-up cleanup, but not a blocker here since it was not introduced by this PR. Approving.