From 547a9f7e8105dce95a9c8971b3ed0d30fe5b5fb1 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Fri, 3 Apr 2026 21:33:18 +0900 Subject: [PATCH 1/2] refactor: embed memory feedback rules into project rules and skills (#1189) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate 13 feedback rules from personal memory into shared project rules/skills so all users benefit from accumulated best practices. Changes across 5 files: A. taskMaestro SKILL.md (3 items): - Default --review-pane to disabled; Conductor Review is primary - Add Mandatory PR Rules (--add-label requirement) to assign template - Add CI gate check to Conductor Review checklist B. Core Rules (3 items): - Add Branch Discipline section (never commit to master/main) - Add Mode Transition Rules (PLAN→ACT requires explicit user input) - Strengthen Error Recovery (recoverable vs unrecoverable classification) C. Systematic Debugging Skill (3 items): - Add mandatory 6-step debugging sequence - Add Validate First rule (run validate/check before debugging) - Add Verify Mental Model rule (read official docs before assuming) D. Ship Skill (2 items): - Add security check must run across ALL workspaces - Add CI workflow self-inclusion check for paths filters E. Parallel Execution Rules (2 items): - Add implementation vs read-only task dispatch rule (taskMaestro vs subAgents) - Add monorepo path safety rule (absolute paths / git -C) Closes #1189 --- .claude/skills/taskmaestro/SKILL.md | 44 ++++++++++++------- packages/rules/.ai-rules/rules/core.md | 21 +++++++-- .../.ai-rules/rules/parallel-execution.md | 32 ++++++++++++++ packages/rules/.ai-rules/skills/ship/SKILL.md | 35 +++++++++++++++ .../skills/systematic-debugging/SKILL.md | 39 ++++++++++++++++ 5 files changed, 151 insertions(+), 20 deletions(-) diff --git a/.claude/skills/taskmaestro/SKILL.md b/.claude/skills/taskmaestro/SKILL.md index c5cdbc66..94bb6756 100644 --- a/.claude/skills/taskmaestro/SKILL.md +++ b/.claude/skills/taskmaestro/SKILL.md @@ -16,7 +16,7 @@ user-invocable: true `$ARGUMENTS`를 파싱하여 서브커맨드와 인수를 분리한다: -- `start [--repo ] [--base ] [--panes <1,2,3>] [--review-pane]` +- `start [--repo ] [--base ] [--panes <1,2,3>] [--review-pane] [--no-review-pane]` - `assign <패널번호> "<작업 지시>"` - `status` - `watch` @@ -88,9 +88,9 @@ MY_PANE=$(tmux display-message -p '#{pane_index}') 워커가 PR을 생성하고 RESULT.json에 `status: "success"`를 기록하면, 리뷰 사이클을 시작한다. `status: "approved"`가 될 때까지 반복하며, 승인 시점이 "진짜 완료"가 된다. -> **리뷰 에이전트 패널이 설정된 경우:** 지휘자가 직접 리뷰하지 않고, 리뷰 에이전트 패널에 리뷰를 위임한다. -> 아래 [Review Agent Protocol](#review-agent-protocol) 섹션을 참조한다. -> 리뷰 에이전트가 없는 경우에만 지휘자가 직접 리뷰를 수행한다 (아래 Fallback 참조). +> **기본 동작:** 지휘자가 직접 리뷰를 수행한다 (Conductor Review — primary method). +> Conductor has PLAN context; worker reviewers do not. +> `--review-pane` 옵션으로 리뷰 에이전트 패널이 설정된 경우에만 리뷰를 위임한다 (아래 [Review Agent Protocol](#review-agent-protocol) 참조). ### Trigger @@ -106,12 +106,19 @@ Watch 모드가 RESULT.json을 감지했을 때: `status: "success"` 또는 `status: "review_addressed"` 감지 시: 1. **상태 파일에서 `review_pane` 확인** -2. `review_pane`이 존재하면 → **Review Agent Protocol** 실행 (리뷰 에이전트에 위임) -3. `review_pane`이 없으면 → **Conductor Fallback Review** 실행 (지휘자 직접 리뷰) +2. `review_pane`이 없으면 (기본) → **Conductor Review** 실행 (지휘자 직접 리뷰 — primary method) +3. `review_pane`이 존재하면 → **Review Agent Protocol** 실행 (리뷰 에이전트에 위임) -### Conductor Fallback Review (리뷰 에이전트 없을 때) +### Conductor Review (Primary — default method) -리뷰 에이전트 패널이 설정되지 않은 경우, 지휘자가 직접 리뷰를 수행한다: +지휘자가 직접 리뷰를 수행한다 (리뷰 에이전트 패널이 설정되지 않은 경우, 기본 동작): + +0. **CI Gate (MUST — BLOCKING)** + ```bash + gh pr checks + ``` + ALL checks must pass before proceeding to review. + If any check fails → report to user, do NOT approve. Stop review here. 1. **PR diff 읽기** ```bash @@ -537,7 +544,7 @@ Wave 1 → Wave 2 → compact → Wave 3 → Wave 4 → compact → ... ## Subcommand: start -`/taskmaestro start [--repo ] [--base ] [--panes <1,2,3>] [--review-pane]` +`/taskmaestro start [--repo ] [--base ] [--panes <1,2,3>] [--review-pane] [--no-review-pane]` 현재 세션의 기존 패널들에 worktree + Claude Code를 세팅한다. @@ -546,7 +553,8 @@ Wave 1 → Wave 2 → compact → Wave 3 → Wave 4 → compact → ... - `--repo`: 대상 레포 경로 (기본값: 현재 작업 디렉토리) - `--base`: 베이스 브랜치 (기본값: 현재 브랜치) - `--panes`: 사용할 패널 번호 목록 (기본값: 지휘자 패널을 제외한 모든 패널) -- `--review-pane`: 리뷰 전용 패널 할당 (기본값: 활성화). 비활성화하려면 `--no-review-pane` 사용 +- `--review-pane`: 리뷰 전용 패널 할당 (기본값: 비활성화). 활성화하려면 `--review-pane` 명시 사용 + - **Rationale:** Conductor has PLAN context; worker reviewers do not. Conductor Review is the primary and default review method. ### Step 2: 사전 검증 @@ -564,12 +572,13 @@ tmux -L "$SOCKET_NAME" list-panes -t "$SESSION:$WIN_IDX" -F '#{pane_index} #{pan 지휘자 패널($MY_PANE)을 제외한 패널들이 워커 대상이 된다. `--panes`로 지정하지 않으면 모든 비활성 패널을 사용한다. -**리뷰 패널 할당 (`--review-pane`, 기본 활성화):** -사용 가능한 패널 중 마지막 패널을 리뷰 에이전트 전용으로 할당한다. -`--no-review-pane` 플래그로 비활성화할 수 있다. +**리뷰 패널 할당 (`--review-pane`, 기본 비활성화):** +`--review-pane` 플래그를 명시하면 사용 가능한 패널 중 마지막 패널을 리뷰 에이전트 전용으로 할당한다. +기본값은 비활성화이며, 지휘자가 직접 리뷰를 수행한다 (Conductor Review). +**Rationale:** Conductor has PLAN context; worker reviewers do not. ```bash -# 리뷰 패널 할당 (기본: 마지막 패널) +# 리뷰 패널 할당 (기본: 비활성화 — --review-pane 명시 시에만 활성화) if [ "$REVIEW_PANE_ENABLED" = true ]; then REVIEW_PANE=$(echo "$AVAILABLE_PANES" | tail -1) WORKER_PANES=$(echo "$AVAILABLE_PANES" | head -n -1) @@ -815,6 +824,9 @@ cat > "$WT_PATH/TASK.md" << 'TASKEOF' <작업 지시 전체 내용> --- +[MANDATORY PR RULES] +When creating PR with gh pr create, ALWAYS include --add-label with appropriate labels (feat, chore, fix, etc.). + [MANDATORY PRE-PUSH] yarn prettier --write . && yarn lint --fix && yarn type-check && yarn test — ALL must pass before push. [COMPLETION] Write RESULT.json: {"status":"success|failure|error","issue":"#NNNN","pr_number":N,"pr_url":"URL","timestamp":"ISO","cost":null,"error":null} [CONTINUOUS] DO NOT stop. Complete ALL steps. Only stop AFTER RESULT.json. @@ -1086,7 +1098,7 @@ done - **`"success"` 감지 (리뷰 사이클 시작):** 1. 상태 파일에서 `review_pane` 확인 2. `review_pane` 존재 시 → Review Agent Protocol에 따라 리뷰 에이전트에 위임 - 3. `review_pane` 없으면 → Conductor Fallback Review 실행: + 3. `review_pane` 없으면 → Conductor Review 실행: a. `gh pr diff ` 로 PR 변경사항 읽기 b. codingbuddy `generate_checklist` 도구로 체크리스트 생성 c. `gh pr review --comment --body "<리뷰>"` 로 리뷰 코멘트 작성 @@ -1109,7 +1121,7 @@ done 1. `review_cycle` 확인. 3 이상이면 사용자에게 보고하고 해당 패널 리뷰 중단 2. 상태 파일에서 `review_pane` 확인 3. `review_pane` 존재 시 → 리뷰 에이전트에 재리뷰 위임 - 4. `review_pane` 없으면 → Conductor Fallback Review 재실행: + 4. `review_pane` 없으면 → Conductor Review 재실행: a. `gh pr diff ` 재확인 b. 미해결 코멘트 확인 (`gh pr view --comments`) c. 충분히 개선 → `status: "approved"`, PR approve 코멘트 작성, 사용자에게 완료 보고 diff --git a/packages/rules/.ai-rules/rules/core.md b/packages/rules/.ai-rules/rules/core.md index 993f1c74..e296abaf 100644 --- a/packages/rules/.ai-rules/rules/core.md +++ b/packages/rules/.ai-rules/rules/core.md @@ -609,11 +609,24 @@ To preserve this implementation session for future reference: --- -### Error Recovery (ACT mode) +### Branch Discipline -- On recoverable errors (file not found, command failure, path issues), try alternatives immediately -- Do not wait for user confirmation on recoverable errors — recover immediately -- Only report to user on unrecoverable errors (auth failure, critical data loss risk) +- Always verify current branch before committing (`git status` / `git branch`) +- Never commit directly to master/main — always use feature branches +- If accidentally on master/main, create a new branch before committing + +### Mode Transition Rules + +- **PLAN → ACT:** MUST wait for explicit user "ACT" input. Never auto-proceed from PLAN to ACT. +- **During ACT:** Proceed autonomously. Only stop for errors or blockers. +- **"Wait for mode transition"** ≠ "wait during execution" — mode transitions require user input, but execution within a mode is continuous. + +### Error Recovery + +- **Recoverable errors** (label not found, file missing, command failure, path issues): auto-try alternatives immediately, continue without user confirmation +- **Unrecoverable errors** (auth failure, critical data loss risk): report to user +- Only report to user when recovery is impossible +- Never stop for confirmation on recoverable errors - Never break the workflow on error — try alternatives and continue ### Path Safety (monorepo) diff --git a/packages/rules/.ai-rules/rules/parallel-execution.md b/packages/rules/.ai-rules/rules/parallel-execution.md index 7d4b3812..56f53b6f 100644 --- a/packages/rules/.ai-rules/rules/parallel-execution.md +++ b/packages/rules/.ai-rules/rules/parallel-execution.md @@ -81,6 +81,38 @@ Order waves considering: --- +## Implementation vs Read-Only Task Dispatch + +Implementation tasks (code changes, file modifications, commits) MUST use tmux-based parallel execution (e.g., taskMaestro), NOT background sub-agents. + +| Task Type | Dispatch Method | Rationale | +|-----------|----------------|-----------| +| **Implementation** (write code, modify files, commit, create PR) | taskMaestro (tmux panes) with git worktree isolation | Full environment, git operations, pre-push checks | +| **Read-only** (research, analysis, code review, search) | Background sub-agents (Agent tool) | No file mutations, safe to run in parallel without isolation | + +**Why:** Background sub-agents lack proper git worktree isolation, cannot run pre-push checks reliably, and risk file conflicts when multiple agents write to the same workspace. taskMaestro provides each worker with an isolated worktree, proper shell environment, and full CI toolchain access. + +## Monorepo Path Safety + +In monorepo environments, always use absolute paths or `git -C ` for git commands to prevent path doubling: + +```bash +# ✅ Correct — absolute path or git -C +git -C /absolute/path/to/repo status +git -C "$REPO_ROOT" add src/file.ts + +# ❌ Wrong — relative path after cd can double +cd packages/my-lib +git add packages/my-lib/src/file.ts # Path doubled! + +# ❌ Wrong — assuming cwd after operations +git add src/file.ts # May not be in expected directory +``` + +**Why:** Relative paths in monorepo subdirectories frequently cause `git add` to target wrong files or fail silently. After any `cd` or worktree operation, verify cwd or use absolute paths. + +--- + ## Sub-Agent Parallelization Strategy When dispatching parallel sub-agents (workers), follow these guidelines: diff --git a/packages/rules/.ai-rules/skills/ship/SKILL.md b/packages/rules/.ai-rules/skills/ship/SKILL.md index a7168c66..00ec75a4 100644 --- a/packages/rules/.ai-rules/skills/ship/SKILL.md +++ b/packages/rules/.ai-rules/skills/ship/SKILL.md @@ -137,6 +137,41 @@ If `custom.ship.globalChecks` is configured, run each global check command after # executed from project root ``` +### Security check (ALL workspaces) + +Security audit must run across ALL workspaces, not just affected ones: + +```bash +# Monorepo — run for ALL workspaces (not just affected): + audit +# Or for npm: +npm audit --workspaces +``` + +**Why:** A vulnerability in an unaffected workspace still ships with the project. Security scanning must be comprehensive. + +### CI workflow self-inclusion check + +When modifying CI workflow files (e.g., `.github/workflows/*.yml`), verify the workflow's `on.push.paths` or `on.pull_request.paths` filter includes the workflow file itself: + +```yaml +# ✅ Correct — workflow file is included in its own paths filter +on: + push: + paths: + - 'src/**' + - '.github/workflows/ci.yml' # Self-included + +# ❌ Wrong — workflow changes won't trigger the workflow +on: + push: + paths: + - 'src/**' + # Missing: .github/workflows/ci.yml +``` + +**Why:** If a workflow file is not in its own paths filter, changes to the workflow itself won't trigger CI, making it impossible to validate workflow changes before merging. + If ANY check fails, stop and report the failure. Do NOT proceed to shipping. ## Step 6: Check Commit Convention diff --git a/packages/rules/.ai-rules/skills/systematic-debugging/SKILL.md b/packages/rules/.ai-rules/skills/systematic-debugging/SKILL.md index 514c6e64..52cdbabb 100644 --- a/packages/rules/.ai-rules/skills/systematic-debugging/SKILL.md +++ b/packages/rules/.ai-rules/skills/systematic-debugging/SKILL.md @@ -22,6 +22,45 @@ NO FIXES WITHOUT ROOT CAUSE INVESTIGATION FIRST If you haven't completed Phase 1, you cannot propose fixes. +## Mandatory 6-Step Debugging Sequence + +Before ANY debugging attempt, follow this sequence in order: + +1. **Validate/Check command** — If the external tool has a validate/check/lint command, run it FIRST. Always. No exceptions. +2. **Official documentation** — Verify system behavior by reading official docs. No guessing how a system works. +3. **Hypothesis formation** — Form a hypothesis with evidence from steps 1-2. +4. **Fix implementation** — Implement the fix based on your hypothesis. +5. **Local verification** — Verify the fix locally before shipping. +6. **Ship** — Only ship after local verification passes. + +### Validate First Rule + +If an external tool, service, or configuration format has a validate/check/lint command, you MUST run it BEFORE any debugging or fix attempts: + +| Tool/Format | Validate Command | +|-------------|-----------------| +| GitHub Actions | `actionlint` or validate via `act --list` | +| Docker | `docker compose config` | +| Kubernetes | `kubectl apply --dry-run=client` | +| Terraform | `terraform validate` | +| JSON | `jq .` or schema-specific validators | +| YAML | `yamllint` | +| TypeScript | `tsc --noEmit` | +| ESLint config | `eslint --print-config` | + +**Why:** Validation catches syntax and schema errors instantly. Debugging without validation wastes time on issues the tool itself can detect. + +### Verify Mental Model Rule + +Before assuming how an external system works, read the official documentation first: + +- **Never guess** API behavior, config format, or tool behavior from memory +- **Always verify** against official docs, especially for version-specific behavior +- **Check changelogs** when upgrading — behavior may have changed between versions +- If official docs are unavailable, use `--help`, man pages, or source code as reference + +**Why:** Stale mental models from past versions or similar-but-different tools cause debugging to chase phantom issues. + ## When to Use Use for ANY technical issue: From 26d43cda098bb169e42dfa4ed482d16e49e78eca Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Fri, 3 Apr 2026 21:43:38 +0900 Subject: [PATCH 2/2] fix(mcp): remove duplicate AgentStack interface and unify field names Remove the second AgentStack interface (lines 190-197) that conflicted with the first (lines 6-17). The duplicate used 'specialists' and optional 'tags', while the schema and JSON files use 'specialist_agents' and required 'tags'. Unified all references to use 'specialist_agents' consistently: - agent-stack.service.ts: isValidStack and toSummary - agent-stack.service.spec.ts: mock data - agent.handler.ts: stack resolution - agent.handler.spec.ts: mock stack data --- .../mcp-server/src/agent/agent-stack.service.spec.ts | 12 ++++++------ apps/mcp-server/src/agent/agent-stack.service.ts | 6 +++--- apps/mcp-server/src/agent/agent.types.ts | 12 ------------ .../src/mcp/handlers/agent.handler.spec.ts | 2 +- apps/mcp-server/src/mcp/handlers/agent.handler.ts | 4 ++-- 5 files changed, 12 insertions(+), 24 deletions(-) diff --git a/apps/mcp-server/src/agent/agent-stack.service.spec.ts b/apps/mcp-server/src/agent/agent-stack.service.spec.ts index 1f0c3f2b..f908fa7f 100644 --- a/apps/mcp-server/src/agent/agent-stack.service.spec.ts +++ b/apps/mcp-server/src/agent/agent-stack.service.spec.ts @@ -23,7 +23,7 @@ describe('AgentStackService', () => { description: 'API development stack', category: 'development', primary_agent: 'backend-developer', - specialists: ['security-specialist', 'test-engineer'], + specialist_agents: ['security-specialist', 'test-engineer'], tags: ['api', 'backend'], }; @@ -32,7 +32,7 @@ describe('AgentStackService', () => { description: 'Frontend review stack', category: 'review', primary_agent: 'frontend-developer', - specialists: ['accessibility-specialist', 'performance-specialist'], + specialist_agents: ['accessibility-specialist', 'performance-specialist'], tags: ['frontend', 'ui'], }; @@ -79,7 +79,7 @@ describe('AgentStackService', () => { const customStack = { ...sampleStack, description: 'Custom API stack', - specialists: ['security-specialist'], + specialist_agents: ['security-specialist'], }; mockReaddir.mockImplementation(async (dirPath: string) => { @@ -188,13 +188,13 @@ describe('AgentStackService', () => { expect(stack.name).toBe('api-development'); expect(stack.primary_agent).toBe('backend-developer'); - expect(stack.specialists).toEqual(['security-specialist', 'test-engineer']); + expect(stack.specialist_agents).toEqual(['security-specialist', 'test-engineer']); }); it('should prefer custom stack over default', async () => { const customStack = { ...sampleStack, - specialists: ['security-specialist', 'test-engineer', 'code-quality-specialist'], + specialist_agents: ['security-specialist', 'test-engineer', 'code-quality-specialist'], }; mockReadFile.mockImplementation(async (filePath: string) => { @@ -207,7 +207,7 @@ describe('AgentStackService', () => { const stack = await service.resolveStack('api-development'); - expect(stack.specialists).toHaveLength(3); + expect(stack.specialist_agents).toHaveLength(3); }); it('should throw when stack not found', async () => { diff --git a/apps/mcp-server/src/agent/agent-stack.service.ts b/apps/mcp-server/src/agent/agent-stack.service.ts index b40f5566..b7f919de 100644 --- a/apps/mcp-server/src/agent/agent-stack.service.ts +++ b/apps/mcp-server/src/agent/agent-stack.service.ts @@ -104,7 +104,7 @@ export class AgentStackService { typeof obj.description === 'string' && typeof obj.category === 'string' && typeof obj.primary_agent === 'string' && - Array.isArray(obj.specialists) + Array.isArray(obj.specialist_agents) ); } @@ -114,8 +114,8 @@ export class AgentStackService { description: stack.description, category: stack.category, primary_agent: stack.primary_agent, - specialist_count: stack.specialists.length, - tags: stack.tags ?? [], + specialist_count: stack.specialist_agents.length, + tags: stack.tags, }; } } diff --git a/apps/mcp-server/src/agent/agent.types.ts b/apps/mcp-server/src/agent/agent.types.ts index ad90c94f..ffbc7887 100644 --- a/apps/mcp-server/src/agent/agent.types.ts +++ b/apps/mcp-server/src/agent/agent.types.ts @@ -184,18 +184,6 @@ export interface InlineAgentDefinition { [key: string]: unknown; } -/** - * Agent stack definition — a preset combination of primary agent + specialists - */ -export interface AgentStack { - name: string; - description: string; - category: string; - primary_agent: string; - specialists: string[]; - tags?: string[]; -} - /** * Summary of an agent stack for listing */ diff --git a/apps/mcp-server/src/mcp/handlers/agent.handler.spec.ts b/apps/mcp-server/src/mcp/handlers/agent.handler.spec.ts index c57e0ba6..f1f04cc7 100644 --- a/apps/mcp-server/src/mcp/handlers/agent.handler.spec.ts +++ b/apps/mcp-server/src/mcp/handlers/agent.handler.spec.ts @@ -977,7 +977,7 @@ describe('AgentHandler', () => { description: 'API development stack', category: 'development', primary_agent: 'backend-developer', - specialists: ['security-specialist', 'test-engineer'], + specialist_agents: ['security-specialist', 'test-engineer'], tags: ['api', 'backend'], }; diff --git a/apps/mcp-server/src/mcp/handlers/agent.handler.ts b/apps/mcp-server/src/mcp/handlers/agent.handler.ts index 75bd134c..2d80a167 100644 --- a/apps/mcp-server/src/mcp/handlers/agent.handler.ts +++ b/apps/mcp-server/src/mcp/handlers/agent.handler.ts @@ -255,8 +255,8 @@ export class AgentHandler extends AbstractHandler { try { const stack = await this.agentStackService.resolveStack(agentStack); primaryAgent = primaryAgent ?? stack.primary_agent; - specialists = specialists?.length ? specialists : stack.specialists; - includeParallel = includeParallel || stack.specialists.length > 0; + specialists = specialists?.length ? specialists : stack.specialist_agents; + includeParallel = includeParallel || stack.specialist_agents.length > 0; } catch (error) { return createErrorResponse( `Failed to resolve agent stack: ${error instanceof Error ? error.message : 'Unknown error'}`,