Skip to content

Commit 7483bef

Browse files
committed
refactor: deduplicate setup pipeline and unify non-interactive rendering
Extract shared functions into cmd/setup.go (BuildProviderConfig, SetupAgent, BuildAppOptions, CollectAgentMetadata, DisplayDebugConfig, SetupCLIForNonInteractive) eliminating triplicated config/agent/app assembly from root.go, script.go, and the SDK. Move the event handler from cmd/script.go into internal/ui/event_handler.go as CLIEventHandler, shared by both script and --prompt modes. Fix streaming in non-interactive modes: chunks are now printed to stdout immediately as they arrive (fmt.Print) instead of being silently buffered until step completion. The --prompt path switches from RunOnce (no intermediate display) to RunOnceWithDisplay with CLIEventHandler, gaining streaming, tool call display, and spinner support that was previously exclusive to script mode.
1 parent c5b7567 commit 7483bef

File tree

5 files changed

+470
-558
lines changed

5 files changed

+470
-558
lines changed

cmd/root.go

Lines changed: 48 additions & 203 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,7 @@ import (
1313
"github.com/mark3labs/mcphost/internal/agent"
1414
"github.com/mark3labs/mcphost/internal/app"
1515
"github.com/mark3labs/mcphost/internal/config"
16-
"github.com/mark3labs/mcphost/internal/models"
1716
"github.com/mark3labs/mcphost/internal/session"
18-
"github.com/mark3labs/mcphost/internal/tools"
1917
"github.com/mark3labs/mcphost/internal/ui"
2018
"github.com/spf13/cobra"
2119
"github.com/spf13/viper"
@@ -373,33 +371,6 @@ func runNormalMode(ctx context.Context) error {
373371
log.SetFlags(log.LstdFlags | log.Lshortfile)
374372
}
375373

376-
systemPrompt, err := config.LoadSystemPrompt(viper.GetString("system-prompt"))
377-
if err != nil {
378-
return fmt.Errorf("failed to load system prompt: %v", err)
379-
}
380-
381-
// Create model configuration
382-
temperature := float32(viper.GetFloat64("temperature"))
383-
topP := float32(viper.GetFloat64("top-p"))
384-
topK := int32(viper.GetInt("top-k"))
385-
numGPU := int32(viper.GetInt("num-gpu-layers"))
386-
mainGPU := int32(viper.GetInt("main-gpu"))
387-
388-
modelConfig := &models.ProviderConfig{
389-
ModelString: viper.GetString("model"),
390-
SystemPrompt: systemPrompt,
391-
ProviderAPIKey: viper.GetString("provider-api-key"),
392-
ProviderURL: viper.GetString("provider-url"),
393-
MaxTokens: viper.GetInt("max-tokens"),
394-
Temperature: &temperature,
395-
TopP: &topP,
396-
TopK: &topK,
397-
StopSequences: viper.GetStringSlice("stop-sequences"),
398-
NumGPU: &numGPU,
399-
MainGPU: &mainGPU,
400-
TLSSkipVerify: viper.GetBool("tls-skip-verify"),
401-
}
402-
403374
// Create spinner function for agent creation
404375
var spinnerFunc agent.SpinnerFunc
405376
if !quietFlag {
@@ -408,170 +379,43 @@ func runNormalMode(ctx context.Context) error {
408379
if tempErr == nil {
409380
return tempCli.ShowSpinner(fn)
410381
}
411-
// Fallback without spinner
412382
return fn()
413383
}
414384
}
415385

416-
// Create the agent using the factory
417-
// Use a buffered debug logger to capture messages during initialization
418-
var bufferedLogger *tools.BufferedDebugLogger
419-
var debugLogger tools.DebugLogger
420-
if viper.GetBool("debug") {
421-
bufferedLogger = tools.NewBufferedDebugLogger(true)
422-
debugLogger = bufferedLogger
423-
}
424-
425-
mcpAgent, err := agent.CreateAgent(ctx, &agent.AgentCreationOptions{
426-
ModelConfig: modelConfig,
427-
MCPConfig: mcpConfig,
428-
SystemPrompt: systemPrompt,
429-
MaxSteps: viper.GetInt("max-steps"),
430-
StreamingEnabled: viper.GetBool("stream"),
431-
ShowSpinner: true,
432-
Quiet: quietFlag,
433-
SpinnerFunc: spinnerFunc,
434-
DebugLogger: debugLogger,
386+
// Create agent using shared setup (builds ProviderConfig from viper internally).
387+
agentResult, err := SetupAgent(ctx, AgentSetupOptions{
388+
MCPConfig: mcpConfig,
389+
ShowSpinner: true,
390+
SpinnerFunc: spinnerFunc,
391+
UseBufferedLogger: true,
435392
})
436393
if err != nil {
437-
return fmt.Errorf("failed to create agent: %v", err)
394+
return err
438395
}
396+
mcpAgent := agentResult.Agent
439397
defer func() { _ = mcpAgent.Close() }()
440398

441-
// Initialize hook executor if hooks are configured
442-
// Get model name for display
443-
modelString := viper.GetString("model")
444-
parsedProvider, modelName, _ := models.ParseModelString(modelString)
445-
if modelName == "" {
446-
modelName = "Unknown"
447-
}
399+
// Collect model/server/tool metadata for display and app options.
400+
parsedProvider, modelName, serverNames, toolNames := CollectAgentMetadata(mcpAgent, mcpConfig)
448401

449-
// Create CLI for non-interactive mode only. SetupCLI is the factory for the
450-
// non-interactive (quiet and non-quiet) path; interactive mode uses the full
451-
// Bubble Tea TUI (AppModel) which handles its own rendering.
452-
// cli is nil in interactive mode and in quiet non-interactive mode.
402+
// Create CLI for non-interactive mode only.
453403
var cli *ui.CLI
454404
if promptFlag != "" {
455-
agentAdapter := &agentUIAdapter{agent: mcpAgent}
456-
cli, err = ui.SetupCLI(&ui.CLISetupOptions{
457-
Agent: agentAdapter,
458-
ModelString: modelString,
459-
Debug: viper.GetBool("debug"),
460-
Compact: viper.GetBool("compact"),
461-
Quiet: quietFlag,
462-
ShowDebug: false, // Handled separately below
463-
ProviderAPIKey: viper.GetString("provider-api-key"),
464-
})
405+
cli, err = SetupCLIForNonInteractive(mcpAgent)
465406
if err != nil {
466407
return fmt.Errorf("failed to setup CLI: %v", err)
467408
}
468409

469410
// Display buffered debug messages if any (non-interactive path only).
470-
if bufferedLogger != nil && cli != nil {
471-
msgs := bufferedLogger.GetMessages()
411+
if agentResult.BufferedLogger != nil && cli != nil {
412+
msgs := agentResult.BufferedLogger.GetMessages()
472413
if len(msgs) > 0 {
473-
combinedMessage := strings.Join(msgs, "\n ")
474-
cli.DisplayDebugMessage(combinedMessage)
475-
}
476-
}
477-
478-
// Display debug configuration if debug mode is enabled (non-interactive path only).
479-
if !quietFlag && cli != nil && viper.GetBool("debug") {
480-
debugConfig := map[string]any{
481-
"model": viper.GetString("model"),
482-
"max-steps": viper.GetInt("max-steps"),
483-
"max-tokens": viper.GetInt("max-tokens"),
484-
"temperature": viper.GetFloat64("temperature"),
485-
"top-p": viper.GetFloat64("top-p"),
486-
"top-k": viper.GetInt("top-k"),
487-
"provider-url": viper.GetString("provider-url"),
488-
"system-prompt": viper.GetString("system-prompt"),
489-
}
490-
491-
// Add TLS skip verify if enabled
492-
if viper.GetBool("tls-skip-verify") {
493-
debugConfig["tls-skip-verify"] = true
494-
}
495-
496-
// Add Ollama-specific parameters if using Ollama
497-
if parsedProvider == "ollama" {
498-
debugConfig["num-gpu-layers"] = viper.GetInt("num-gpu-layers")
499-
debugConfig["main-gpu"] = viper.GetInt("main-gpu")
500-
}
501-
502-
// Only include non-empty stop sequences
503-
stopSequences := viper.GetStringSlice("stop-sequences")
504-
if len(stopSequences) > 0 {
505-
debugConfig["stop-sequences"] = stopSequences
506-
}
507-
508-
// Only include API keys if they're set (but don't show the actual values for security)
509-
if viper.GetString("provider-api-key") != "" {
510-
debugConfig["provider-api-key"] = "[SET]"
511-
}
512-
513-
// Add MCP server configuration for debugging
514-
if len(mcpConfig.MCPServers) > 0 {
515-
mcpServers := make(map[string]any)
516-
loadedServers := mcpAgent.GetLoadedServerNames()
517-
loadedServerSet := make(map[string]bool)
518-
for _, name := range loadedServers {
519-
loadedServerSet[name] = true
520-
}
521-
522-
for name, server := range mcpConfig.MCPServers {
523-
serverInfo := map[string]any{
524-
"type": server.Type,
525-
"status": "failed", // Default to failed
526-
}
527-
528-
// Mark as loaded if it's in the loaded servers list
529-
if loadedServerSet[name] {
530-
serverInfo["status"] = "loaded"
531-
}
532-
533-
if len(server.Command) > 0 {
534-
serverInfo["command"] = server.Command
535-
}
536-
if len(server.Environment) > 0 {
537-
// Mask sensitive environment variables
538-
maskedEnv := make(map[string]string)
539-
for k, v := range server.Environment {
540-
if strings.Contains(strings.ToLower(k), "token") ||
541-
strings.Contains(strings.ToLower(k), "key") ||
542-
strings.Contains(strings.ToLower(k), "secret") {
543-
maskedEnv[k] = "[MASKED]"
544-
} else {
545-
maskedEnv[k] = v
546-
}
547-
}
548-
serverInfo["environment"] = maskedEnv
549-
}
550-
if server.URL != "" {
551-
serverInfo["url"] = server.URL
552-
}
553-
if server.Name != "" {
554-
serverInfo["name"] = server.Name
555-
}
556-
mcpServers[name] = serverInfo
557-
}
558-
debugConfig["mcpServers"] = mcpServers
414+
cli.DisplayDebugMessage(strings.Join(msgs, "\n "))
559415
}
560-
cli.DisplayDebugConfig(debugConfig)
561416
}
562-
}
563-
564-
// Prepare data for slash commands
565-
var serverNames []string
566-
for name := range mcpConfig.MCPServers {
567-
serverNames = append(serverNames, name)
568-
}
569417

570-
tools := mcpAgent.GetTools()
571-
var toolNames []string
572-
for _, tool := range tools {
573-
info := tool.Info()
574-
toolNames = append(toolNames, info.Name)
418+
DisplayDebugConfig(cli, mcpAgent, mcpConfig, parsedProvider)
575419
}
576420

577421
// Main interaction logic
@@ -701,18 +545,8 @@ func runNormalMode(ctx context.Context) error {
701545
}
702546

703547
// Create the app.App instance now that session messages are loaded.
704-
appOpts := app.Options{
705-
Agent: mcpAgent,
706-
SessionManager: sessionManager,
707-
MCPConfig: mcpConfig,
708-
ModelName: modelName,
709-
ServerNames: serverNames,
710-
ToolNames: toolNames,
711-
StreamingEnabled: viper.GetBool("stream"),
712-
Quiet: quietFlag,
713-
Debug: viper.GetBool("debug"),
714-
CompactMode: viper.GetBool("compact"),
715-
}
548+
appOpts := BuildAppOptions(mcpAgent, mcpConfig, modelName, serverNames, toolNames)
549+
appOpts.SessionManager = sessionManager
716550

717551
// Create a usage tracker that is shared between the app layer (for recording
718552
// usage after each step) and the TUI (for /usage display). For non-interactive
@@ -723,7 +557,7 @@ func runNormalMode(ctx context.Context) error {
723557
usageTracker = cli.GetUsageTracker()
724558
} else {
725559
// Interactive mode: create a tracker using the same logic as SetupCLI.
726-
usageTracker = ui.CreateUsageTracker(modelString, viper.GetString("provider-api-key"))
560+
usageTracker = ui.CreateUsageTracker(viper.GetString("model"), viper.GetString("provider-api-key"))
727561
}
728562
if usageTracker != nil {
729563
appOpts.UsageTracker = usageTracker
@@ -734,7 +568,7 @@ func runNormalMode(ctx context.Context) error {
734568

735569
// Check if running in non-interactive mode
736570
if promptFlag != "" {
737-
return runNonInteractiveModeApp(ctx, appInstance, promptFlag, quietFlag, noExitFlag, modelName, parsedProvider, mcpAgent.GetLoadingMessage(), serverNames, toolNames, usageTracker)
571+
return runNonInteractiveModeApp(ctx, appInstance, cli, promptFlag, quietFlag, noExitFlag, modelName, parsedProvider, mcpAgent.GetLoadingMessage(), serverNames, toolNames, usageTracker)
738572
}
739573

740574
// Quiet mode is not allowed in interactive mode
@@ -748,24 +582,35 @@ func runNormalMode(ctx context.Context) error {
748582
// runNonInteractiveModeApp executes a single prompt via the app layer and exits,
749583
// or transitions to the interactive BubbleTea TUI when --no-exit is set.
750584
//
751-
// RunOnce does not create a tea.Program, so there is no spinner or tool-call
752-
// display; only the final response text is written to os.Stdout. This
753-
// satisfies both the normal and --quiet non-interactive cases (quiet simply
754-
// means "no intermediate output", which RunOnce already guarantees).
585+
// In quiet mode, RunOnce is used (no intermediate output, final response only).
586+
// Otherwise, RunOnceWithDisplay streams tool calls and responses through the
587+
// shared CLIEventHandler — giving --prompt mode the same rich output as script
588+
// mode. This eliminates the previous split where --prompt silently swallowed
589+
// all intermediate events.
755590
//
756-
// When --no-exit is set, after RunOnce completes the interactive BubbleTea TUI
757-
// is started so the user can continue the conversation.
758-
func runNonInteractiveModeApp(ctx context.Context, appInstance *app.App, prompt string, quiet, noExit bool, modelName, providerName, loadingMessage string, serverNames, toolNames []string, usageTracker *ui.UsageTracker) error {
759-
if err := appInstance.RunOnce(ctx, prompt); err != nil {
760-
return err
761-
}
762-
763-
// Display token usage after the response (unless quiet mode suppresses output).
764-
// The app layer has already updated the tracker inside RunOnce.
765-
if !quiet && usageTracker != nil {
766-
usageInfo := usageTracker.RenderUsageInfo()
767-
if usageInfo != "" {
768-
fmt.Println(usageInfo)
591+
// When --no-exit is set, after the prompt completes the interactive BubbleTea
592+
// TUI is started so the user can continue the conversation.
593+
func runNonInteractiveModeApp(ctx context.Context, appInstance *app.App, cli *ui.CLI, prompt string, quiet, noExit bool, modelName, providerName, loadingMessage string, serverNames, toolNames []string, usageTracker *ui.UsageTracker) error {
594+
if quiet {
595+
// Quiet mode: no intermediate display, just print final response.
596+
if err := appInstance.RunOnce(ctx, prompt); err != nil {
597+
return err
598+
}
599+
} else if cli != nil {
600+
// Display user message before running the agent.
601+
cli.DisplayUserMessage(prompt)
602+
603+
// Route events through the shared CLI event handler.
604+
eventHandler := ui.NewCLIEventHandler(cli, modelName)
605+
err := appInstance.RunOnceWithDisplay(ctx, prompt, eventHandler.Handle)
606+
eventHandler.Cleanup()
607+
if err != nil {
608+
return err
609+
}
610+
} else {
611+
// No CLI available (shouldn't happen in non-quiet mode, but be safe).
612+
if err := appInstance.RunOnce(ctx, prompt); err != nil {
613+
return err
769614
}
770615
}
771616

0 commit comments

Comments
 (0)