From f7900b76e18283396d5e53b93e44f6c52efadfbc Mon Sep 17 00:00:00 2001 From: dj-oyu <68707227+dj-oyu@users.noreply.github.com> Date: Sat, 21 Feb 2026 01:12:35 +0900 Subject: [PATCH] fix: route /plan through LLM queue for AI interview flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously /plan was handled entirely on the fast path, so the LLM never received the message and never started the interview. Now handlePlanCommand returns (_, false) for new plans, and the new expandPlanCommand writes the interview seed to MEMORY.md and rewrites the message content before it enters the LLM queue — matching the existing expandSkillCommand pattern. Co-Authored-By: Claude Opus 4.6 --- pkg/agent/loop.go | 126 ++++++++++++++++++++++++++++++----------- pkg/agent/loop_test.go | 44 ++++++++------ 2 files changed, 120 insertions(+), 50 deletions(-) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 684ee4f92..1b245e147 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -347,10 +347,16 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) } // Expand /skill command: inject SKILL.md content into message, then continue to LLM - var skillCompact string + var expansionCompact string if expanded, compact, ok := al.expandSkillCommand(msg); ok { msg.Content = expanded - skillCompact = compact + expansionCompact = compact + } + + // Expand /plan : write interview seed, rewrite for LLM interview + if expanded, compact, ok := al.expandPlanCommand(msg); ok { + msg.Content = expanded + expansionCompact = compact } // Check for commands @@ -391,7 +397,7 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) Channel: msg.Channel, ChatID: msg.ChatID, UserMessage: msg.Content, - HistoryMessage: skillCompact, + HistoryMessage: expansionCompact, DefaultResponse: "I've completed processing but have no response to give.", EnableSummary: true, SendResponse: false, @@ -1285,7 +1291,8 @@ func (al *AgentLoop) handleCommand(ctx context.Context, msg bus.InboundMessage) return al.handleSkillsCommand(), true case "/plan": - return al.handlePlanCommand(args), true + resp, handled := al.handlePlanCommand(args) + return resp, handled } return "", false @@ -1397,96 +1404,147 @@ func (al *AgentLoop) handleSkillsCommand() string { return sb.String() } -// handlePlanCommand handles /plan subcommands. -func (al *AgentLoop) handlePlanCommand(args []string) string { +// handlePlanCommand handles /plan subcommands that can be resolved instantly. +// Returns (response, handled). For "/plan " (new plan), it returns +// ("", false) so the message falls through to the LLM queue, where +// expandPlanCommand writes the seed and rewrites the content. +func (al *AgentLoop) handlePlanCommand(args []string) (string, bool) { agent := al.registry.GetDefaultAgent() if agent == nil { - return "No agent configured." + return "No agent configured.", true } if len(args) == 0 { // /plan — show current plan - return agent.ContextBuilder.FormatPlanDisplay() + return agent.ContextBuilder.FormatPlanDisplay(), true } sub := args[0] switch sub { case "clear": if !agent.ContextBuilder.HasActivePlan() { - return "No active plan to clear." + return "No active plan to clear.", true } if err := agent.ContextBuilder.ClearMemory(); err != nil { - return fmt.Sprintf("Error clearing plan: %v", err) + return fmt.Sprintf("Error clearing plan: %v", err), true } - return "Plan cleared." + return "Plan cleared.", true case "done": if !agent.ContextBuilder.HasActivePlan() { - return "No active plan." + return "No active plan.", true } if len(args) < 2 { - return "Usage: /plan done " + return "Usage: /plan done ", true } stepNum, err := strconv.Atoi(args[1]) if err != nil || stepNum < 1 { - return "Step number must be a positive integer." + return "Step number must be a positive integer.", true } phase := agent.ContextBuilder.GetCurrentPhase() if err := agent.ContextBuilder.MarkStep(phase, stepNum); err != nil { - return fmt.Sprintf("Error: %v", err) + return fmt.Sprintf("Error: %v", err), true } - return fmt.Sprintf("Marked step %d in phase %d as done.", stepNum, phase) + return fmt.Sprintf("Marked step %d in phase %d as done.", stepNum, phase), true case "add": if !agent.ContextBuilder.HasActivePlan() { - return "No active plan." + return "No active plan.", true } if len(args) < 2 { - return "Usage: /plan add " + return "Usage: /plan add ", true } desc := strings.Join(args[1:], " ") phase := agent.ContextBuilder.GetCurrentPhase() if err := agent.ContextBuilder.AddStep(phase, desc); err != nil { - return fmt.Sprintf("Error: %v", err) + return fmt.Sprintf("Error: %v", err), true } - return fmt.Sprintf("Added step to phase %d: %s", phase, desc) + return fmt.Sprintf("Added step to phase %d: %s", phase, desc), true case "start": if !agent.ContextBuilder.HasActivePlan() { - return "No active plan." + return "No active plan.", true } if agent.ContextBuilder.GetPlanStatus() != "interviewing" { - return "Plan is already executing." + return "Plan is already executing.", true } if err := agent.ContextBuilder.SetPlanStatus("executing"); err != nil { - return fmt.Sprintf("Error: %v", err) + return fmt.Sprintf("Error: %v", err), true } - return "Plan status changed to executing." + return "Plan status changed to executing.", true case "next": if !agent.ContextBuilder.HasActivePlan() { - return "No active plan." + return "No active plan.", true } if err := agent.ContextBuilder.AdvancePhase(); err != nil { - return fmt.Sprintf("Error: %v", err) + return fmt.Sprintf("Error: %v", err), true } phase := agent.ContextBuilder.GetCurrentPhase() - return fmt.Sprintf("Advanced to phase %d.", phase) + return fmt.Sprintf("Advanced to phase %d.", phase), true default: // /plan — start new plan + // Block if a plan is already active (fast-path error). if agent.ContextBuilder.HasActivePlan() { - return "A plan is already active. Use /plan clear first." + return "A plan is already active. Use /plan clear first.", true } - task := strings.Join(args, " ") - seed := BuildInterviewSeed(task) - if err := agent.ContextBuilder.WriteMemory(seed); err != nil { - return fmt.Sprintf("Error creating plan: %v", err) - } - return fmt.Sprintf("Plan started: %s\nStatus: interviewing\n\nI'll ask you some questions to build a detailed plan.", task) + // Not handled here — let the message flow to the LLM queue. + // expandPlanCommand will write the seed and rewrite the content. + return "", false } } +// expandPlanCommand detects "/plan " (new plan start) and: +// - writes the interview seed to MEMORY.md +// - rewrites the message content for the LLM +// - returns a compact form for session history +// +// This follows the same pattern as expandSkillCommand: the message is +// rewritten before reaching the LLM, so the AI sees the task description +// while the system prompt contains the interview guide. +func (al *AgentLoop) expandPlanCommand(msg bus.InboundMessage) (expanded string, compact string, ok bool) { + content := strings.TrimSpace(msg.Content) + if !strings.HasPrefix(content, "/plan ") { + return "", "", false + } + + task := strings.TrimSpace(content[6:]) // len("/plan ") == 6 + if task == "" { + return "", "", false + } + + // Known subcommands are handled by handlePlanCommand (fast path). + firstWord := strings.Fields(task)[0] + switch firstWord { + case "clear", "done", "add", "start", "next": + return "", "", false + } + + agent := al.registry.GetDefaultAgent() + if agent == nil { + return "", "", false + } + + // If a plan is already active, don't expand — handleCommand will + // catch it and return the error on the fast path. + if agent.ContextBuilder.HasActivePlan() { + return "", "", false + } + + // Write the interview seed + seed := BuildInterviewSeed(task) + if err := agent.ContextBuilder.WriteMemory(seed); err != nil { + return "", "", false + } + + // Expanded: the task description goes to LLM. + // The system prompt already contains the interview guide. + expanded = task + compact = fmt.Sprintf("[Plan: %s]", utils.Truncate(task, 80)) + return expanded, compact, true +} + // extractPeer extracts the routing peer from inbound message metadata. func extractPeer(msg bus.InboundMessage) *routing.RoutePeer { peerKind := msg.Metadata["peer_kind"] diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index fc6c15c78..6bffdec03 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -917,21 +917,30 @@ func TestPlanCommand_StartNewPlan(t *testing.T) { al, cleanup := newTestAgentLoop(t) defer cleanup() - response, handled := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan Set up monitoring"}) - if !handled { - t.Fatal("expected /plan to be handled") + // /plan should NOT be handled by handleCommand — it falls through + // to the LLM queue via expandPlanCommand. + _, handled := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan Set up monitoring"}) + if handled { + t.Fatal("expected /plan NOT to be handled (should fall through to LLM)") } - if !strings.Contains(response, "Plan started") { - t.Errorf("expected 'Plan started', got %q", response) + + // expandPlanCommand writes the seed and rewrites the message + msg := bus.InboundMessage{Content: "/plan Set up monitoring"} + expanded, compact, ok := al.expandPlanCommand(msg) + if !ok { + t.Fatal("expected expandPlanCommand to succeed") } - if !strings.Contains(response, "Set up monitoring") { - t.Errorf("expected task in response, got %q", response) + if expanded != "Set up monitoring" { + t.Errorf("expected expanded = 'Set up monitoring', got %q", expanded) + } + if !strings.Contains(compact, "Set up monitoring") { + t.Errorf("expected compact to contain task, got %q", compact) } // Verify plan was created agent := al.registry.GetDefaultAgent() if !agent.ContextBuilder.HasActivePlan() { - t.Error("expected active plan after /plan start") + t.Error("expected active plan after expandPlanCommand") } if status := agent.ContextBuilder.GetPlanStatus(); status != "interviewing" { t.Errorf("expected 'interviewing', got %q", status) @@ -942,11 +951,14 @@ func TestPlanCommand_StartBlockedByExisting(t *testing.T) { al, cleanup := newTestAgentLoop(t) defer cleanup() - // Start first plan - al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan First task"}) + // Start first plan via expandPlanCommand + al.expandPlanCommand(bus.InboundMessage{Content: "/plan First task"}) - // Try to start another - response, _ := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan Second task"}) + // Try to start another — handleCommand should block it on the fast path + response, handled := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan Second task"}) + if !handled { + t.Fatal("expected second /plan to be handled (blocked)") + } if !strings.Contains(response, "already active") { t.Errorf("expected 'already active', got %q", response) } @@ -957,7 +969,7 @@ func TestPlanCommand_Clear(t *testing.T) { defer cleanup() // Start plan then clear - al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan Test task"}) + al.expandPlanCommand(bus.InboundMessage{Content: "/plan Test task"}) response, _ := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan clear"}) if !strings.Contains(response, "Plan cleared") { t.Errorf("expected 'Plan cleared', got %q", response) @@ -984,7 +996,7 @@ func TestPlanCommand_Start(t *testing.T) { defer cleanup() // Create interviewing plan - al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan Test task"}) + al.expandPlanCommand(bus.InboundMessage{Content: "/plan Test task"}) // Transition to executing response, _ := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan start"}) @@ -1003,7 +1015,7 @@ func TestPlanCommand_StartAlreadyExecuting(t *testing.T) { defer cleanup() // Create interviewing plan then start - al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan Test task"}) + al.expandPlanCommand(bus.InboundMessage{Content: "/plan Test task"}) al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan start"}) // Try start again @@ -1044,7 +1056,7 @@ func TestPlanCommand_DoneInvalidStep(t *testing.T) { al, cleanup := newTestAgentLoop(t) defer cleanup() - al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan Test task"}) + al.expandPlanCommand(bus.InboundMessage{Content: "/plan Test task"}) response, _ := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan done abc"}) if !strings.Contains(response, "positive integer") { t.Errorf("expected step validation error, got %q", response)