fix: plan mode review gate and exec guard quoted-path false positive

- Interview guide now instructs LLM to transition to "review" instead
  of "executing", ensuring user approval via /plan start is required.
- Guard logic extended to handle interviewing→review transitions with
  validation and plan display.
- exec safety guard now uses quote-aware tokenizer so that quoted
  arguments like "/review skip-git-repo-check" are not mistaken for
  absolute file paths.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

pkg/agent/loop.go

@@ -833,10 +833,10 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
 
 	// 5a. Auto-advance plan phases after LLM iteration
 	postStatus := agent.ContextBuilder.GetPlanStatus()
-	if agent.ContextBuilder.HasActivePlan() && postStatus == "executing" {
+	if agent.ContextBuilder.HasActivePlan() && (postStatus == "executing" || postStatus == "review") {
-		// Intercept: if AI changed status to executing without user approval
+		// Intercept: if AI changed status to executing or review without user approval
-		// (from interviewing or review), validate and set to "review".
+		// (from interviewing or review), validate and hold at "review".
-		if preStatus == "interviewing" || preStatus == "review" {
+		if preStatus == "interviewing" || (preStatus == "review" && postStatus == "executing") {
 			if err := agent.ContextBuilder.ValidatePlanStructure(); err != nil {
 				_ = agent.ContextBuilder.SetPlanStatus("interviewing")
 				logger.WarnCF("agent", "Reverted plan to interviewing: "+err.Error(),
@@ -856,7 +856,7 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
 					})
 				}
 			}
-		} else if agent.ContextBuilder.GetTotalPhases() == 0 {
+		} else if postStatus == "executing" && agent.ContextBuilder.GetTotalPhases() == 0 {
 			// Safeguard: executing but no phases (shouldn't happen, but be safe).
 			_ = agent.ContextBuilder.SetPlanStatus("interviewing")
 			logger.WarnCF("agent", "Reverted plan to interviewing: no phases defined",

pkg/agent/memory.go

@@ -489,7 +489,7 @@ func (ms *MemoryStore) GetInterviewContext() string {
 	sb.WriteString("- When you have enough information, use edit_file to add ## Phase, ## Commands, and ## Context sections BELOW the header block.\n")
 	sb.WriteString("- Each step MUST use checkbox syntax: `- [ ] description`. The system parses checkboxes to track progress.\n")
 	sb.WriteString("- Organize into 2-5 phases with 3-5 steps each.\n")
-	sb.WriteString("- After writing Phases, change `> Status: interviewing` to `> Status: executing` via edit_file.\n")
+	sb.WriteString("- After writing Phases, change `> Status: interviewing` to `> Status: review` via edit_file. The user must approve with /plan start before execution begins.\n")
 	sb.WriteString("\n### Target Format (MANDATORY — system parses this exact structure)\n")
 	sb.WriteString("\n")
 	sb.WriteString("# Active Plan\n")

pkg/tools/shell.go

@@ -291,11 +291,11 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
 		}
 
 		// Token-based absolute path detection.
-		// Uses strings.Fields instead of regex to avoid false positives
+		// Uses shellTokenize to respect quoted strings (e.g., "/review ..."
-		// from slashes in relative paths (e.g., "tests/cold/file.py").
+		// is a single argument, not a file path).
 		// Flags like -I/usr/local/include are naturally skipped because
 		// filepath.IsAbs returns false for tokens starting with "-".
-		for _, token := range strings.Fields(cmd) {
+		for _, token := range shellTokenize(cmd) {
 			token = strings.Trim(token, "\"'")
 
 			if !filepath.IsAbs(token) {
@@ -321,6 +321,42 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
 	return ""
 }
 
+// shellTokenize splits a command string into tokens while respecting
+// single and double quotes.  Quoted substrings are returned as a single
+// token (with the quotes still attached so the caller can trim them).
+// This prevents false positives where a quoted argument like
+// "/review skip-git-repo-check" would be split into "/review" and
+// "skip-git-repo-check" by strings.Fields.
+func shellTokenize(s string) []string {
+	var tokens []string
+	var cur strings.Builder
+	var quote byte // 0 = none, '\'' or '"'
+	for i := 0; i < len(s); i++ {
+		ch := s[i]
+		switch {
+		case quote != 0:
+			cur.WriteByte(ch)
+			if ch == quote {
+				quote = 0
+			}
+		case ch == '\'' || ch == '"':
+			cur.WriteByte(ch)
+			quote = ch
+		case ch == ' ' || ch == '\t':
+			if cur.Len() > 0 {
+				tokens = append(tokens, cur.String())
+				cur.Reset()
+			}
+		default:
+			cur.WriteByte(ch)
+		}
+	}
+	if cur.Len() > 0 {
+		tokens = append(tokens, cur.String())
+	}
+	return tokens
+}
+
 // isExecutable checks if a path points to an executable file.
 // On Unix, checks the execute permission bits.
 // On Windows, checks for known executable extensions.

pkg/tools/shell_test.go

@@ -486,3 +486,22 @@ func TestGuardCommand_CdWithAbsoluteWorkspacePath(t *testing.T) {
 		t.Errorf("cd to workspace subdir should be allowed: %q → %s", cmd, result)
 	}
 }
+
+func TestGuardCommand_QuotedSlashArgNotBlocked(t *testing.T) {
+	workspace := t.TempDir()
+	tool := NewExecTool(workspace, true)
+
+	// A quoted argument starting with "/" is not a file path — it's a
+	// command argument that happens to contain a slash.
+	cmds := []string{
+		`codex exec --yolo "/review skip-git-repo-check"`,
+		`echo '/hello world'`,
+		`grep "/etc/passwd" file.txt`,
+	}
+	for _, cmd := range cmds {
+		result := tool.guardCommand(cmd, workspace)
+		if result != "" {
+			t.Errorf("Quoted argument should not be blocked: %q → %s", cmd, result)
+		}
+	}
+}