fix: eliminate 3-level Skill tool nesting that breaks control flow (335b-c846) (merge worktree-20260325-170522)

Author: JoeOakhartNava

.test-index

@@ -145,3 +145,5 @@ tests/hooks/test-session-safety.sh: tests/scripts/test-v2-clean-guard.sh
 tests/scripts/test-validate-issues.sh: tests/scripts/test-v2-clean-guard.sh
 tests/scripts/test-read-config.sh: tests/scripts/test-v2-clean-guard.sh
 tests/scripts/test-flat-config-e2e.sh: tests/scripts/test-v2-clean-guard.sh
+plugins/dso/skills/implementation-plan/SKILL.md: tests/skills/test-skill-nesting-depth.sh
+plugins/dso/skills/design-wireframe/SKILL.md: tests/skills/test-skill-nesting-depth.sh

plugins/dso/skills/design-wireframe/SKILL.md

@@ -724,22 +724,27 @@ Create `designs/<uuid>/manifest.md` combining:
 
 ## Phase 5: Design Review (/dso:design-wireframe)
 
-This phase uses `/dso:review-protocol` at Stage 2 (skipping Stage 1 because Phase 4's
+This phase uses `REVIEW-PROTOCOL-WORKFLOW.md` at Stage 2 (skipping Stage 1 because Phase 4's
 artifact consistency checkpoint already serves as the mental pre-review).
 
+> **NESTING PROHIBITION**: Do NOT invoke `/dso:review-protocol` via the Skill tool here.
+> This skill may already be at nesting level 2+ (e.g., `sprint → preplanning → design-wireframe`).
+> A Skill tool call to `/dso:review-protocol` would create 3+ levels, which fails to return control.
+> Always read and execute the workflow inline.
+
 ### Step 16: Submit for committee review (/dso:design-wireframe)
 
 Read [docs/review-criteria.md](docs/review-criteria.md) for reviewer prompt
 files and domain-specific review context.
 
-Invoke `/dso:review-protocol` with:
+Read and execute `${CLAUDE_PLUGIN_ROOT}/docs/workflows/REVIEW-PROTOCOL-WORKFLOW.md` inline with:
 
 - **subject**: "Design Wireframe: {story title}"
 - **artifact**: The design artifacts (manifest, spatial layout JSON, SVG wireframe description, design tokens) plus story context and epic context (if applicable)
 - **pass_threshold**: 4
 - **start_stage**: 2 (Phase 4 artifact validation serves as Stage 1)
 - **max_revision_cycles**: 3
-- **perspectives**: Defined by the four reviewer prompt files. For each reviewer, read its prompt file and map to the `/dso:review-protocol` perspective format:
+- **perspectives**: Defined by the four reviewer prompt files. For each reviewer, read its prompt file and map to the `REVIEW-PROTOCOL-WORKFLOW.md` perspective format:
 
 **Reviewer 1 — Product Management** (from [docs/reviewers/product-manager.md](docs/reviewers/product-manager.md)):
 - Dimensions: `story_alignment`, `user_value`, `scope_appropriateness`, `consistency`, `epic_coherence`
@@ -761,7 +766,7 @@ Invoke `/dso:review-protocol` with:
 
 Because design reviews genuinely benefit from diverse perspectives, **always use
 multi-agent dispatch** (one sub-agent per reviewer, launched in parallel via the
-Task tool). This bypasses `/dso:review-protocol`'s default single-agent approach.
+Task tool). This bypasses `REVIEW-PROTOCOL-WORKFLOW.md`'s default single-agent approach.
 
 Since subagents cannot view images, describe the SVG wireframe's structure and
 spatial relationships in text when constructing each prompt.
@@ -780,7 +785,7 @@ The design **passes** if ALL dimension scores across ALL reviewers are 4, 5, or
 **Conflict detection**: Scan all findings for direct contradictions — pairs of
 suggestions that target the same component/artifact but pull in opposite directions
 (add vs remove, more vs less, strict vs flexible, expand vs reduce). Populate the
-`conflicts` array. Resolution follows `/dso:review-protocol`:
+`conflicts` array. Resolution follows `REVIEW-PROTOCOL-WORKFLOW.md`:
 - Critical vs minor: critical finding wins, no escalation
 - Both critical/major: escalate to user via AskUserQuestion
 - Both minor: caller chooses
@@ -795,7 +800,7 @@ record the aggregate pass/fail result.
 
 ### Step 18: Revise and re-submit (max 3 cycles) (/dso:design-wireframe)
 
-Follow `/dso:review-protocol`'s revision protocol:
+Follow `REVIEW-PROTOCOL-WORKFLOW.md`'s revision protocol:
 
 1. **Triage by severity**: Address critical findings first, then major, then minor.
 2. **Resolve conflicts first**: If `conflicts[]` is non-empty, resolve before revising.

plugins/dso/skills/implementation-plan/SKILL.md

@@ -58,9 +58,9 @@ Copy and track as you work (standalone only):
 ```
 Progress:
 - [ ] Step 1: Contextual Discovery (story loaded, context gathered, ambiguities resolved, cross-cutting detection done — layers: _, interfaces: _)
-- [ ] Step 2: Architectural Review via /dso:review-protocol (passed / skipped — no new pattern)
+- [ ] Step 2: Architectural Review via REVIEW-PROTOCOL-WORKFLOW.md inline (passed / skipped — no new pattern)
 - [ ] Step 3: Task Drafting (tasks drafted with E2E + docs coverage)
-- [ ] Step 4: Plan Review via /dso:review-protocol (all dimensions: 5, iteration: _/3)
+- [ ] Step 4: Plan Review via REVIEW-PROTOCOL-WORKFLOW.md inline (all dimensions: 5, iteration: _/3)
 - [ ] Step 5: Task Creation (tasks created, deps added, health validated)
 - [ ] Step 6: Gap Analysis (COMPLEX: opus sub-agent dispatched, findings processed; TRIVIAL: skipped)
 ```

plugins/dso/skills/sprint/prompts/impl-plan-dispatch.md

@@ -38,9 +38,9 @@ Use the `Read` tool at that path to load the skill. Then execute Steps 1-6 as de
 ### Steps to Execute
 
 - **Step 1**: Contextual Discovery — load story context, resolve ambiguities, detect cross-cutting changes (or reuse evaluator context if provided above)
-- **Step 2**: Architectural Review — invoke `/dso:review-protocol` if a new pattern is needed or cross-cutting thresholds are met; otherwise skip
+- **Step 2**: Architectural Review — read and execute `REVIEW-PROTOCOL-WORKFLOW.md` inline if a new pattern is needed or cross-cutting thresholds are met; otherwise skip
 - **Step 3**: Atomic Task Drafting — draft tasks with TDD-first, E2E coverage, and docs coverage
-- **Step 4**: Plan Review — invoke `/dso:review-protocol` with pass_threshold 5; iterate up to 3 times
+- **Step 4**: Plan Review — read and execute `REVIEW-PROTOCOL-WORKFLOW.md` inline with pass_threshold 5; iterate up to 3 times
 - **Step 5**: Task Creation — create tasks in tickets, add dependencies, validate ticket health
 - **Step 6**: Gap Analysis — dispatch opus sub-agent for COMPLEX stories; skip for TRIVIAL (uses evaluator-context classification)
 
@@ -81,7 +81,7 @@ STATUS:blocked QUESTIONS:[{"text":"What is the expected response format for the
 ### Rules
 - Do NOT: git commit, git push, .claude/scripts/dso ticket transition
 - You MAY use: .claude/scripts/dso ticket create (with --acceptance, -d flags), .claude/scripts/dso ticket link (required for Step 5 dependency wiring), direct `.tickets/<id>.md` editing for post-creation updates
-- Do NOT use the Task tool to dispatch nested sub-agents. Skill tool invocations (e.g., /dso:review-protocol) ARE permitted.
+- Do NOT use the Task tool to dispatch nested sub-agents. Do NOT invoke `/dso:review-protocol` via the Skill tool — use `REVIEW-PROTOCOL-WORKFLOW.md` inline instead (Skill nesting creates 3+ levels which fail to return control).
 - Do NOT invoke `/dso:commit`, `/dso:review`, or any slash-command other than Skill tool invocations required by the implementation-plan steps
 - Do NOT modify files outside the scope of task creation (no source code changes — this is planning only)
 - Only modify files under $(git rev-parse --show-toplevel). Do NOT write to any other path.

tests/skills/test-skill-nesting-depth.sh

@@ -0,0 +1,78 @@
+#!/usr/bin/env bash
+# Test: Skill tool nesting depth safety
+# Bug: 335b-c846 — 3-level Skill tool nesting fails to return control
+#
+# Verifies that skills invoked from within sprint (2nd level) do NOT invoke
+# other skills via the Skill tool (which would create 3+ levels).
+# They must use "read and execute ... inline" for review workflows.
+
+set -uo pipefail
+
+REPO_ROOT="$(cd "$(dirname "$0")/../.." && pwd)"
+PASS=0
+FAIL=0
+
+pass() { ((PASS++)); echo "  PASS: $1"; }
+fail() { ((FAIL++)); echo "  FAIL: $1"; }
+
+echo "=== Skill Nesting Depth Tests ==="
+
+# --- implementation-plan tests ---
+
+IMPL_PLAN="$REPO_ROOT/plugins/dso/skills/implementation-plan/SKILL.md"
+
+echo ""
+echo "-- implementation-plan checklist labels --"
+
+# Test 1: Checklist must NOT reference "via /dso:review-protocol"
+if grep -q 'via /dso:review-protocol' "$IMPL_PLAN"; then
+    fail "implementation-plan checklist references 'via /dso:review-protocol' (Skill tool invocation risk)"
+else
+    pass "implementation-plan checklist does not reference 'via /dso:review-protocol'"
+fi
+
+# Test 2: Checklist should reference the inline workflow pattern
+if grep -q 'REVIEW-PROTOCOL-WORKFLOW.md' "$IMPL_PLAN"; then
+    pass "implementation-plan references REVIEW-PROTOCOL-WORKFLOW.md"
+else
+    fail "implementation-plan does not reference REVIEW-PROTOCOL-WORKFLOW.md"
+fi
+
+# Test 3: implementation-plan must NOT contain Skill( invocations
+if grep -qE 'Skill\(' "$IMPL_PLAN"; then
+    fail "implementation-plan contains Skill() invocation — creates 3-level nesting from sprint"
+else
+    pass "implementation-plan contains no Skill() invocations"
+fi
+
+# --- design-wireframe tests ---
+
+DESIGN_WF="$REPO_ROOT/plugins/dso/skills/design-wireframe/SKILL.md"
+
+echo ""
+echo "-- design-wireframe review-protocol invocation --"
+
+# Test 4: design-wireframe must NOT say "Invoke /dso:review-protocol"
+if grep -q 'Invoke.*/dso:review-protocol' "$DESIGN_WF"; then
+    fail "design-wireframe invokes /dso:review-protocol via Skill tool (creates 3+ level nesting)"
+else
+    pass "design-wireframe does not invoke /dso:review-protocol via Skill tool"
+fi
+
+# Test 5: design-wireframe must reference REVIEW-PROTOCOL-WORKFLOW.md for its review
+if grep -q 'REVIEW-PROTOCOL-WORKFLOW.md' "$DESIGN_WF"; then
+    pass "design-wireframe references REVIEW-PROTOCOL-WORKFLOW.md (inline pattern)"
+else
+    fail "design-wireframe does not reference REVIEW-PROTOCOL-WORKFLOW.md"
+fi
+
+# Test 6: design-wireframe Phase 5 must use "read and execute ... inline" pattern
+if grep -iq 'read and execute.*REVIEW-PROTOCOL-WORKFLOW' "$DESIGN_WF"; then
+    pass "design-wireframe Phase 5 uses 'read and execute ... inline' pattern"
+else
+    fail "design-wireframe Phase 5 does not use 'read and execute ... inline' pattern"
+fi
+
+echo ""
+echo "=== Results: $PASS passed, $FAIL failed ==="
+[ "$FAIL" -eq 0 ]