Refactor Holon solve/workflow to explicit goal routing (#745)

* refactor solve/workflow to explicit goal routing

* fix review feedback for goal routing

Author: jolestar

.github/workflows/holon-solve.yml

@@ -26,6 +26,8 @@ jobs:
       comment_id: ${{ github.event.comment.id || 0 }}
       review_id: ${{ github.event.review.id || 0 }}
       auto_review: ${{ fromJSON(vars.HOLON_AUTO_REVIEW || 'false') }}
+      # Optional: override goal for all runs.
+      # goal: 'Review this PR and publish findings.'
 
       # Repo-local workflow uses the current branch Holon implementation.
       log_level: 'debug'

.github/workflows/holon-trigger.yml

@@ -50,11 +50,10 @@ inputs:
     description: 'Assistant output mode for CI visibility (none, stream)'
     required: false
     default: 'none'
-  skill:
-    description: 'Deprecated. Skill selection is now driven by workflow goal hints; this input is ignored.'
+  goal:
+    description: 'Explicit execution goal passed to holon solve --goal. Empty = solve defaults by target type and PR review signals.'
     required: false
     default: ''
-    deprecationMessage: 'Deprecated: skill is no longer passed to holon solve. Use workflow goal hints (workflow.json trigger.goal_hint) to influence behavior.'
   role:
     description: 'Role to assume (e.g. developer, reviewer)'
     required: false
@@ -337,6 +336,9 @@ runs:
         if [ -n "${{ inputs.state_dir }}" ]; then
           SOLVE_ARGS+=(--state-dir "${{ inputs.state_dir }}")
         fi
+        if [ -n "${{ inputs.goal }}" ]; then
+          SOLVE_ARGS+=(--goal "${{ inputs.goal }}")
+        fi
 
         # Always add output directory (either user-specified or temp)
         SOLVE_ARGS+=(--output "$OUT_DIR")

action.yml

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/google/go-github/v68/github"
 	"github.com/holon-run/holon/pkg/config"
 	"github.com/holon-run/holon/pkg/git"
 	pkggithub "github.com/holon-run/holon/pkg/github"
@@ -34,6 +35,7 @@ var (
 	solveImageAutoDetect bool
 	solveSkillPaths      []string // Skill paths (--skill flag, repeatable)
 	solveSkillsList      string   // Comma-separated skills (--skills flag)
+	solveGoal            string
 
 	solveLogLevel              string
 	solveAssistantOutput       string
@@ -58,7 +60,7 @@ This is a high-level command that orchestrates the full workflow:
 1. Prepare workspace using smart workspace preparation
 2. Collect context from the GitHub Issue or PR
 3. Run Holon with the collected context
-4. Publish results (create PR for issues, or push/fix for PRs)
+	4. Publish results (create PR for issues, or review/fix for PRs)
 
 Skill-First Mode:
   Holon solve uses skill-first IO mode, which delegates context
@@ -90,7 +92,7 @@ Examples:
   # Solve an issue (creates/updates a PR) - uses skill-first mode
   holon solve https://github.com/holon-run/holon/issues/123
 
-  # Solve a PR (fixes review comments) - uses skill-first mode
+  # Solve a PR (defaults to review; switches to fix when review signals exist)
   holon solve https://github.com/holon-run/holon/pull/456
 
   # Short form
@@ -140,14 +142,17 @@ Examples:
 // solvePRCmd is the explicit "solve pr" subcommand
 var solvePRCmd = &cobra.Command{
 	Use:   "pr <ref>",
-	Short: "Solve a GitHub PR (fixes review comments)",
-	Long: `Solve a GitHub PR by collecting context, running Holon, and fixing review comments.
+	Short: "Solve a GitHub PR (review or fix)",
+	Long: `Solve a GitHub PR by collecting context, running Holon, and publishing review/fix results.
 
 The workflow:
 1. Collect PR context (diff, review threads, checks)
-2. Run Holon in "pr-fix" mode
+2. Resolve goal:
+   - default review when no review signals exist
+   - default fix when review feedback exists
+   - explicit --goal overrides defaults
 3. If diff.patch exists: apply/push to PR branch
-4. Publish replies based on pr-fix.json
+4. Publish review findings or fix replies to GitHub
 
 Examples:
   holon solve pr https://github.com/holon-run/holon/pull/123
@@ -515,31 +520,6 @@ func runSolve(ctx context.Context, refStr, explicitType string) error {
 		return fmt.Errorf("failed to create context directory: %w", err)
 	}
 
-	// Read workflow.json for trigger metadata (if exists)
-	// This is populated by the holon-solve workflow when triggered by comments
-	// We read it once and pass the data to buildGoal to avoid duplicate reads
-	type workflowMetadata struct {
-		TriggerCommentID int64
-		TriggerGoalHint  string
-	}
-	var workflowMeta workflowMetadata
-	workflowPath := filepath.Join(inputDir, "workflow.json")
-	if workflowData, err := os.ReadFile(workflowPath); err == nil {
-		var workflow struct {
-			Trigger struct {
-				CommentID int64  `json:"comment_id"`
-				GoalHint  string `json:"goal_hint"`
-			} `json:"trigger"`
-		}
-		if err := json.Unmarshal(workflowData, &workflow); err == nil {
-			workflowMeta.TriggerCommentID = workflow.Trigger.CommentID
-			workflowMeta.TriggerGoalHint = workflow.Trigger.GoalHint
-			if workflowMeta.TriggerCommentID > 0 {
-				fmt.Printf("Found trigger comment ID: %d\n", workflowMeta.TriggerCommentID)
-			}
-		}
-	}
-
 	// Resolve skills early to determine skill configuration
 	// In skill mode, the skill (agent) is responsible for context collection and publishing
 	// Parse CLI skills
@@ -571,15 +551,14 @@ func runSolve(ctx context.Context, refStr, explicitType string) error {
 		allSkills = configSkills
 	}
 
-	// Skill-first is the default: always use skill mode
-	// If no skills are configured, add a default skill bundle for issues only.
-	// For PRs, leave skills empty so PR-specific defaults can be applied later.
+	// Skill-first is the default: always use skill mode.
+	// If no skills are configured, add a default skill bundle for issues.
+	// For PRs, leave skills empty and rely on the goal for review/fix behavior.
 	useSkillMode := true // Always skill-first now
 	if len(allSkills) == 0 {
 		if refType == "pr" {
 			// In PR solves, do not force an issue-solve default skill.
-			// Leaving skills empty allows buildGoal's PR defaults to select appropriate skills.
-			fmt.Printf("Config: skills = [] (source: skill-first mode for PR; deferring to PR-specific defaults)\n")
+			fmt.Printf("Config: skills = [] (source: skill-first mode for PR)\n")
 		} else {
 			// Add default skill bundle for skill-first mode on issues
 			// This ensures skill-first IO is used by default
@@ -650,13 +629,16 @@ func runSolve(ctx context.Context, refStr, explicitType string) error {
 		return fmt.Errorf("failed to resolve assistant output: %w", err)
 	}
 
-	// Determine goal from the reference (skill mode already determined earlier)
-	// Pass useSkillMode to generate appropriate goal
-	selectedSkill := ""
-	if len(allSkills) > 0 {
-		selectedSkill = strings.TrimSpace(allSkills[0])
+	goal := strings.TrimSpace(solveGoal)
+	if goal == "" {
+		defaultGoal, err := buildDefaultSolveGoal(ctx, solveRef, refType, token)
+		if err != nil {
+			return fmt.Errorf("failed to build default goal: %w", err)
+		}
+		goal = defaultGoal
+	} else {
+		fmt.Println("Using explicit solve goal from --goal")
 	}
-	goal := buildGoal(inputDir, solveRef, refType, workflowMeta.TriggerGoalHint, selectedSkill)
 
 	// Resolve output directory with precedence: CLI flag > temp dir
 	// For solve command, we use a temp directory by default to avoid polluting the workspace
@@ -862,34 +844,84 @@ func getGitHubToken() (string, error) {
 	return token, nil
 }
 
-// buildGoal builds a goal description from the reference
-// triggerGoalHint is the optional goal hint from free-form triggers (e.g., "@holonbot fix this bug")
-func buildGoal(inputDir string, ref *pkggithub.SolveRef, refType string, triggerGoalHint string, selectedSkill string) string {
-	baseGoal := ""
+func buildDefaultSolveGoal(ctx context.Context, ref *pkggithub.SolveRef, refType, token string) (string, error) {
 	if refType == "pr" {
-		switch selectedSkill {
-		case "github-review":
-			baseGoal = fmt.Sprintf("Use the github-review skill to review the PR %s. The skill will guide you through: (1) Collecting PR context, (2) Analyzing code changes for issues, and (3) Publishing structured review findings to GitHub.", ref.URL())
-		case "github-pr-fix", "":
-			baseGoal = fmt.Sprintf("Use the github-pr-fix skill to fix the PR %s. The skill will guide you through: (1) Analyzing review comments, (2) Implementing fixes, (3) Publishing replies to GitHub.", ref.URL())
-		default:
-			baseGoal = fmt.Sprintf("Use the %s skill to process the PR %s according to the skill instructions, and publish the expected results back to GitHub.", selectedSkill, ref.URL())
-		}
-	} else {
-		switch selectedSkill {
-		case "github-issue-solve", "":
-			baseGoal = fmt.Sprintf("Use the github-issue-solve skill to solve the GitHub issue %s end-to-end. Success requires all of the following: (1) Collect GitHub context, (2) Implement the solution, (3) Ensure ${GITHUB_OUTPUT_DIR}/manifest.json has status='completed' and outcome='success', and (4) After publish completes, ensure ${GITHUB_OUTPUT_DIR}/manifest.json contains pr_number and pr_url for the created PR. The runtime validates success based on the manifest contract (status/outcome), with summary.md treated as optional human-readable output.", ref.URL())
-		default:
-			baseGoal = fmt.Sprintf("Use the %s skill to solve the GitHub issue %s end-to-end following the skill instructions and publish the expected results to GitHub. Ensure ${GITHUB_OUTPUT_DIR}/manifest.json has status='completed' and outcome='success' for successful execution.", selectedSkill, ref.URL())
+		intent, err := determinePRDefaultIntent(ctx, ref, token)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "Warning: failed to inspect PR review signals, defaulting to review goal: %v\n", err)
+			intent = "review"
 		}
+
+		return buildPRGoal(ref.URL(), intent), nil
+	}
+
+	return buildIssueGoal(ref.URL()), nil
+}
+
+func buildPRGoal(prURL, intent string) string {
+	if intent == "fix" {
+		return fmt.Sprintf("Fix the PR %s by addressing outstanding review feedback and requested changes. Implement necessary code changes and publish replies to GitHub.", prURL)
+	}
+	return fmt.Sprintf("Review the PR %s. Analyze code changes for correctness, security, and maintainability, then publish structured review findings to GitHub.", prURL)
+}
+
+func buildIssueGoal(issueURL string) string {
+	return fmt.Sprintf("Solve the GitHub issue %s end-to-end. Success requires all of the following: (1) Collect GitHub context, (2) Implement the solution, (3) Ensure ${GITHUB_OUTPUT_DIR}/manifest.json has status='completed' and outcome='success', and (4) After publish completes, ensure ${GITHUB_OUTPUT_DIR}/manifest.json contains pr_number and pr_url for the created PR. The runtime validates success based on the manifest contract (status/outcome), with summary.md treated as optional human-readable output.", issueURL)
+}
+
+func determinePRDefaultIntent(ctx context.Context, ref *pkggithub.SolveRef, token string) (string, error) {
+	client := pkggithub.NewClient(token)
+
+	threads, err := client.FetchReviewThreads(ctx, ref.Owner, ref.Repo, ref.Number, true)
+	if err != nil {
+		return "review", fmt.Errorf("failed to fetch PR review threads: %w", err)
+	}
+
+	latestState, err := fetchLatestPRReviewState(ctx, client, ref.Owner, ref.Repo, ref.Number)
+	if err != nil {
+		return "review", err
 	}
 
-	// Append goal hint from free-form triggers if provided
-	if triggerGoalHint != "" {
-		return fmt.Sprintf("%s\n\nUser intent: %s", baseGoal, triggerGoalHint)
+	return inferPRIntentFromSignals(len(threads), latestState), nil
+}
+
+func inferPRIntentFromSignals(unresolvedThreadCount int, latestReviewState string) string {
+	if unresolvedThreadCount > 0 {
+		return "fix"
+	}
+	if strings.EqualFold(strings.TrimSpace(latestReviewState), "CHANGES_REQUESTED") {
+		return "fix"
+	}
+	return "review"
+}
+
+func fetchLatestPRReviewState(ctx context.Context, client *pkggithub.Client, owner, repo string, prNumber int) (string, error) {
+	opts := &github.ListOptions{PerPage: 100}
+	latestState := ""
+
+	for {
+		reviews, resp, err := client.GitHubClient().PullRequests.ListReviews(ctx, owner, repo, prNumber, opts)
+		if err != nil {
+			return "", fmt.Errorf("failed to fetch PR reviews: %w", err)
+		}
+
+		for _, review := range reviews {
+			if review == nil {
+				continue
+			}
+			state := strings.ToUpper(strings.TrimSpace(review.GetState()))
+			if state != "" {
+				latestState = state
+			}
+		}
+
+		if resp.NextPage == 0 {
+			break
+		}
+		opts.Page = resp.NextPage
 	}
 
-	return baseGoal
+	return latestState, nil
 }
 
 // publishResults publishes the holon execution results
@@ -1050,6 +1082,7 @@ func init() {
 	solveCmd.Flags().StringVar(&solveRepo, "repo", "", "Default repository in owner/repo format (for numeric references)")
 	solveCmd.Flags().StringVar(&solveBase, "base", "main", "Base branch for PR creation (issue mode only)")
 	solveCmd.Flags().StringVarP(&solveOutDir, "output", "O", "", "Output directory (default: creates temp dir to avoid polluting workspace)")
+	solveCmd.Flags().StringVarP(&solveGoal, "goal", "g", "", "Explicit goal description (when set, skips default issue/PR goal generation)")
 	_ = solveCmd.Flags().MarkDeprecated("out", "use --output instead")
 	solveCmd.Flags().StringVarP(&solveOutDir, "out", "o", "", "Deprecated: use --output")
 	solveCmd.Flags().StringVarP(&solveContext, "context", "c", "", "Additional context directory (deprecated)")

cmd/holon/solve.go

@@ -10,15 +10,14 @@ import (
 	pkggithub "github.com/holon-run/holon/pkg/github"
 )
 
-func TestBuildGoal_SkillModePRReviewUsesGithubReviewSkill(t *testing.T) {
-	ref := &pkggithub.SolveRef{Owner: "holon-run", Repo: "holon", Number: 564}
-	goal := buildGoal("", ref, "pr", "", "github-review")
+func TestBuildPRGoal_ReviewIntent(t *testing.T) {
+	goal := buildPRGoal("https://github.com/holon-run/holon/pull/564", "review")
 
-	if !strings.Contains(goal, "Use the github-review skill") {
-		t.Fatalf("expected github-review goal, got: %s", goal)
+	if !strings.Contains(goal, "Review the PR") {
+		t.Fatalf("expected review goal, got: %s", goal)
 	}
-	if strings.Contains(goal, "github-pr-fix") {
-		t.Fatalf("expected goal to avoid github-pr-fix for review skill, got: %s", goal)
+	if strings.Contains(strings.ToLower(goal), "fix the pr") {
+		t.Fatalf("expected review goal to avoid fix wording, got: %s", goal)
 	}
 }
 
@@ -93,22 +92,17 @@ func TestInferRefTypeFromURL(t *testing.T) {
 	}
 }
 
-func TestBuildGoal_SkillModePRFixUsesGithubPrFixSkill(t *testing.T) {
-	ref := &pkggithub.SolveRef{Owner: "holon-run", Repo: "holon", Number: 564}
-	goal := buildGoal("", ref, "pr", "", "github-pr-fix")
+func TestBuildPRGoal_FixIntent(t *testing.T) {
+	goal := buildPRGoal("https://github.com/holon-run/holon/pull/564", "fix")
 
-	if !strings.Contains(goal, "Use the github-pr-fix skill") {
-		t.Fatalf("expected github-pr-fix goal, got: %s", goal)
+	if !strings.Contains(strings.ToLower(goal), "fix the pr") {
+		t.Fatalf("expected fix goal, got: %s", goal)
 	}
 }
 
-func TestBuildGoal_SkillModeIssueUsesGithubIssueSolveSkill(t *testing.T) {
-	ref := &pkggithub.SolveRef{Owner: "holon-run", Repo: "holon", Number: 527}
-	goal := buildGoal("", ref, "issue", "", "github-issue-solve")
+func TestBuildIssueGoal(t *testing.T) {
+	goal := buildIssueGoal("https://github.com/holon-run/holon/issues/527")
 
-	if !strings.Contains(goal, "Use the github-issue-solve skill") {
-		t.Fatalf("expected github-issue-solve goal, got: %s", goal)
-	}
 	// Verify it uses generic manifest contract, not publish-intent.json
 	if strings.Contains(goal, "publish-intent.json") {
 		t.Fatalf("goal should not mention publish-intent.json, got: %s", goal)
@@ -119,6 +113,49 @@ func TestBuildGoal_SkillModeIssueUsesGithubIssueSolveSkill(t *testing.T) {
 	}
 }
 
+func TestInferPRIntentFromSignals(t *testing.T) {
+	tests := []struct {
+		name                  string
+		unresolvedThreadCount int
+		latestReviewState     string
+		want                  string
+	}{
+		{
+			name:                  "unresolved threads force fix",
+			unresolvedThreadCount: 2,
+			latestReviewState:     "",
+			want:                  "fix",
+		},
+		{
+			name:                  "changes requested forces fix",
+			unresolvedThreadCount: 0,
+			latestReviewState:     "CHANGES_REQUESTED",
+			want:                  "fix",
+		},
+		{
+			name:                  "changes requested is case insensitive",
+			unresolvedThreadCount: 0,
+			latestReviewState:     "changes_requested",
+			want:                  "fix",
+		},
+		{
+			name:                  "no review signals defaults to review",
+			unresolvedThreadCount: 0,
+			latestReviewState:     "COMMENTED",
+			want:                  "review",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := inferPRIntentFromSignals(tt.unresolvedThreadCount, tt.latestReviewState)
+			if got != tt.want {
+				t.Fatalf("inferPRIntentFromSignals() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestPrepareWorkspaceForSolve_WithWorkspace_UsesDirectWorkspace(t *testing.T) {
 	userWorkspace := t.TempDir()
 	ref := &pkggithub.SolveRef{Owner: "holon-run", Repo: "holon", Number: 627}

cmd/holon/solve_test.go

@@ -28,6 +28,8 @@ jobs:
       assignee_login: ${{ github.event.assignee.login || '' }}
       comment_id: ${{ github.event.comment.id || 0 }}
       review_id: ${{ github.event.review.id || 0 }}
+      # Optional: override goal for all runs.
+      # goal: 'Review this PR and publish findings.'
       # Optional: Specify custom runner labels.
       # Plain string for single label (e.g., 'self-hosted' or 'ubuntu-24.04-large'):
       # runs_on: 'self-hosted'

cmd/holon/templates/workflow.yml

@@ -27,6 +27,8 @@ jobs:
       # Optional: enable auto-review for every PR when true (default false)
       # Configure repo variable HOLON_AUTO_REVIEW=true to enable.
       auto_review: ${{ fromJSON(vars.HOLON_AUTO_REVIEW || 'false') }}
+      # Optional: override goal for all runs. Empty = Holon defaults by target type.
+      # goal: 'Review this PR and publish findings.'
       # Optional: Specify custom runner labels.
       # Plain string for single label (e.g., 'self-hosted' or 'ubuntu-24.04-large'):
       # runs_on: 'self-hosted'