refactor(recipe): enhance read-only shell handling and robustness

- Tighten trigger conditions for read-only shell steps to specific phrases
- Add robust JSON extraction with escaped quote handling
- Reorder user confirmation before whitelist check in execution flow
- Relocate FileChange struct and clean up comments
- Update recipe markdown for step title consistency

.grokkit/recipes/result-refactor.md

@@ -39,7 +39,7 @@ Refactors all error handling in the target package to use the new Result[T] patt
 
 ## Execution Steps
 
-### Step 1: Explore project structure
+### Step 1: Read-Only Shell: Explore project structure
 **Objective:** Get a clear view of the current project layout.  
 **Instructions:** Use safe read-only shell commands to show the directory tree and key files.
 **Expected output:** A tree view and relevant file contents.

internal/recipe/runner.go

@@ -49,10 +49,9 @@ func (r *Runner) Run() error {
 			r.handleApplyStep(refactorJSONs)
 			continue
 
-		// NEW: Read-only shell commands (ls, cat, tree, git status, etc.)
-		case strings.Contains(titleLower, "read-only") || strings.Contains(titleLower, "inspect") ||
-			strings.Contains(titleLower, "explore") || strings.Contains(titleLower, "shell") ||
-			strings.Contains(titleLower, "show") || strings.Contains(titleLower, "list"):
+		// Explicit trigger for read-only shell commands
+		case strings.Contains(titleLower, "read-only shell") ||
+			strings.Contains(titleLower, "shell read-only"):
 			r.executeReadOnlyShell(step, previousResults)
 			continue
 
@@ -91,7 +90,7 @@ Execute this step now. Respond ONLY with the expected output format — no expla
 	return nil
 }
 
-// resolveWorkDir expands ~ and makes absolute (keeps your boundary logic)
+// resolveWorkDir expands ~ and makes absolute
 func (r *Runner) resolveWorkDir() string {
 	root := "."
 	if v, ok := r.Recipe.ResolvedParams["package_path"]; ok {
@@ -111,7 +110,7 @@ func (r *Runner) resolveWorkDir() string {
 	return abs
 }
 
-// discoverFiles — uses the resolved workDir (your current signature)
+// discoverFiles — uses resolved workDir
 func (r *Runner) discoverFiles(workDir string) []string {
 	var files []string
 
@@ -191,11 +190,7 @@ Original file:
 	}
 }
 
-type FileChange struct {
-	File    string `json:"file"`
-	Content string `json:"content"`
-}
-
+// handleApplyStep stays as you have it (or your latest version)
 func (r *Runner) handleApplyStep(refactorJSONs []string) {
 	if len(refactorJSONs) == 0 {
 		fmt.Println("   ⚠️  No refactored files to apply — skipping.")
@@ -231,6 +226,11 @@ func (r *Runner) handleApplyStep(refactorJSONs []string) {
 	fmt.Println("   Review it, then run with dry_run=false to apply.")
 }
 
+type FileChange struct {
+	File    string `json:"file"`
+	Content string `json:"content"`
+}
+
 func createUnifiedPatch(changes []FileChange, patchPath string) error {
 	f, err := os.Create(patchPath)
 	if err != nil {
@@ -280,8 +280,8 @@ Return ONLY a JSON array of read-only commands. Example:
     "args": ["-la"]
   },
   {
-    "command": "cat",
-    "args": ["README.md"]
+    "command": "tree",
+    "args": ["."]
   }
 ]
 
@@ -299,12 +299,7 @@ Only use safe read-only commands from the allowed list.`,
 	response := r.Client.Stream(messages, r.Model)
 	fmt.Println()
 
-	type ShellCommand struct {
-		Command string   `json:"command"`
-		Args    []string `json:"args"`
-	}
-
-	var cmds []ShellCommand
+	// Robust JSON extraction
 	start := strings.Index(response, "[")
 	end := strings.LastIndex(response, "]") + 1
 	if start == -1 {
@@ -312,7 +307,16 @@ Only use safe read-only commands from the allowed list.`,
 		return
 	}
 
-	if err := json.Unmarshal([]byte(response[start:end]), &cmds); err != nil {
+	jsonStr := response[start:end]
+	jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"") // fix escaped quotes
+
+	type ShellCommand struct {
+		Command string   `json:"command"`
+		Args    []string `json:"args"`
+	}
+
+	var cmds []ShellCommand
+	if err := json.Unmarshal([]byte(jsonStr), &cmds); err != nil {
 		fmt.Printf("   ⚠️  Could not parse commands: %v\n", err)
 		return
 	}
@@ -323,7 +327,19 @@ Only use safe read-only commands from the allowed list.`,
 			fullCmd += " " + strings.Join(cmd.Args, " ")
 		}
 
-		// Safety: must be in whitelist
+		fmt.Printf("   Grok wants to run: %s\n   Allow this command? [y/N] ", fullCmd)
+
+		var answer string
+		_, err := fmt.Scanln(&answer)
+		if err != nil {
+			return
+		}
+		if !strings.HasPrefix(strings.ToLower(answer), "y") {
+			fmt.Println("   ❌ Cancelled by user.")
+			continue
+		}
+
+		// Whitelist check
 		allowed := false
 		for _, allowedCmd := range r.Recipe.AllowedShellCommands {
 			if strings.HasPrefix(cmd.Command, allowedCmd) {
@@ -336,25 +352,16 @@ Only use safe read-only commands from the allowed list.`,
 			continue
 		}
 
-		// User confirmation
-		fmt.Printf("   Grok wants to run: %s\n   Allow this command? [y/N] ", fullCmd)
-		var answer string
-		_, err := fmt.Scanln(&answer)
+		// Run with strict cwd
+		execCmd := exec.Command(cmd.Command, cmd.Args...)
+		execCmd.Dir = r.resolveWorkDir()
+		output, err := execCmd.CombinedOutput()
+
 		if err != nil {
-			return
-		}
-		if strings.HasPrefix(strings.ToLower(answer), "y") {
-			execCmd := exec.Command(cmd.Command, cmd.Args...)
-			execCmd.Dir = r.resolveWorkDir()
-			output, err := execCmd.CombinedOutput()
-			if err != nil {
-				fmt.Printf("   ❌ Failed: %v\n%s\n", err, string(output))
-			} else {
-				fmt.Printf("   ✅ Success\n%s\n", string(output))
-				previousResults = append(previousResults, fmt.Sprintf("Command output:\n%s", string(output)))
-			}
+			fmt.Printf("   ❌ Failed: %v\n%s\n", err, string(output))
 		} else {
-			fmt.Println("   ❌ Cancelled by user.")
+			fmt.Printf("   ✅ Success\n%s\n", string(output))
+			previousResults = append(previousResults, fmt.Sprintf("Command output:\n%s", string(output)))
 		}
 	}
 }