Skip to content

fix(cli): apply imports before rendering structured output #1729

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Dumbris merged 3 commits into from
Oct 22, 2026
Merged

fix(cli): apply imports before rendering structured output #1729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a2b940f
Fix upstream import effects in structured output
Dumbris Oct 22, 2026
446d29e
Clarify invalid import previews and save failures
Dumbris Oct 22, 2026
dc8c9b7
Skip permission-based import test where unsupported
Dumbris Oct 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 80 additions & 26 deletions cmd/mcpproxy/upstream_cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2113,14 +2113,45 @@ func runUpstreamImport(_ *cobra.Command, args []string) error {
return outputError(output.NewStructuredError(output.ErrCodeOperationFailed, err.Error()), output.ErrCodeOperationFailed)
}

// Output results based on format
// Apply once, before choosing a renderer. The parser's Imported slice is a
// plan; only successful writes may be reported as imported after execution.
planned := len(result.Imported)
applied := 0
var applyErr error
if !upstreamImportDryRun && planned > 0 {
var successful []*configimport.ImportedServer
var failed []configimport.FailedServer
successful, failed, applyErr = applyImportedServers(result.Imported, globalConfig)
result.Imported = successful
result.Failed = append(result.Failed, failed...)
result.Summary.Imported = len(successful)
result.Summary.Failed = len(result.Failed)
applied = len(successful)
}
outcome := importOutcome{planned: planned, applied: applied, dryRun: upstreamImportDryRun}

// Render the completed outcome even when some writes failed. Automation gets
// the per-server failures on stdout and a nonzero exit status.
outputFormat := ResolveOutputFormat()

var outputErr error
if outputFormat == "json" || outputFormat == "yaml" {
return outputImportResultStructured(result, outputFormat)
outputErr = outputImportResultStructured(result, outcome)
} else {
outputErr = outputImportResultTable(result, outcome, upstreamImportNoQuarantine)
}

return outputImportResultTable(result, upstreamImportDryRun, upstreamImportNoQuarantine, globalConfig)
if outputErr != nil {
return outputErr
}
if applyErr != nil {
return applyErr
}
if result.Summary.Failed > 0 {
if outcome.dryRun {
return fmt.Errorf("validation failed for %d of %d source servers; no changes made", result.Summary.Failed, result.Summary.Total)
}
return fmt.Errorf("failed to import %d of %d servers", result.Summary.Failed, result.Summary.Total)
}
return nil
}

// importSelfListenAddrs returns the addresses the local mcpproxy answers on,
Expand Down Expand Up @@ -2164,13 +2195,28 @@ func parseImportFormat(format string) configimport.ConfigFormat {
}
}

// outputImportResultStructured outputs the import result in JSON/YAML format
func outputImportResultStructured(result *configimport.ImportResult, format string) error {
type importOutcome struct {
planned int
applied int
dryRun bool
}

func importOutputSummary(result *configimport.ImportResult, outcome importOutcome) map[string]int {
return map[string]int{
"total": result.Summary.Total, "imported": result.Summary.Imported,
"planned": outcome.planned, "applied": outcome.applied,
"skipped": result.Summary.Skipped, "failed": result.Summary.Failed,
}
}

// outputImportResultStructured outputs the import result in JSON/YAML format.
func outputImportResultStructured(result *configimport.ImportResult, outcome importOutcome) error {
// Build output structure
output := map[string]interface{}{
"format": result.Format,
"format_name": result.FormatDisplayName,
"summary": result.Summary,
"summary": importOutputSummary(result, outcome),
"dry_run": outcome.dryRun,
"imported": buildImportedServersOutput(result.Imported),
"skipped": result.Skipped,
"failed": result.Failed,
Expand Down Expand Up @@ -2241,9 +2287,9 @@ func buildImportedServersOutput(imported []*configimport.ImportedServer) []map[s
}

// outputImportResultTable outputs the import result in table format
func outputImportResultTable(result *configimport.ImportResult, dryRun bool, noQuarantine bool, globalConfig *config.Config) error {
func outputImportResultTable(result *configimport.ImportResult, outcome importOutcome, noQuarantine bool) error {
// Header
if dryRun {
if outcome.dryRun {
fmt.Println("🔍 DRY RUN - No changes will be made")
fmt.Println()
}
Expand All @@ -2254,22 +2300,23 @@ func outputImportResultTable(result *configimport.ImportResult, dryRun bool, noQ
// Summary
fmt.Printf("Summary:\n")
fmt.Printf(" Total servers found: %d\n", result.Summary.Total)
fmt.Printf(" To import: %d\n", result.Summary.Imported)
fmt.Printf(" Planned: %d\n", outcome.planned)
fmt.Printf(" Applied: %d\n", outcome.applied)
fmt.Printf(" Skipped: %d\n", result.Summary.Skipped)
fmt.Printf(" Failed: %d\n", result.Summary.Failed)
fmt.Println()

// Show imported servers
if len(result.Imported) > 0 {
if dryRun {
if outcome.dryRun {
fmt.Println("Servers to import:")
} else {
fmt.Println("Imported servers:")
}

for _, s := range result.Imported {
statusIcon := "✅"
if dryRun {
if outcome.dryRun {
statusIcon = "📋"
}
fmt.Printf(" %s %s (%s)\n", statusIcon, s.Server.Name, s.Server.Protocol)
Expand Down Expand Up @@ -2325,12 +2372,7 @@ func outputImportResultTable(result *configimport.ImportResult, dryRun bool, noQ
fmt.Println()
}

// If not dry run and we have servers to import, actually add them
if !dryRun && len(result.Imported) > 0 {
err := applyImportedServers(result.Imported, globalConfig)
if err != nil {
return err
}
if outcome.applied > 0 {
if noQuarantine {
fmt.Println("✅ Servers imported without quarantine. They are ready to use.")
} else {
Expand All @@ -2342,7 +2384,7 @@ func outputImportResultTable(result *configimport.ImportResult, dryRun bool, noQ
}

// applyImportedServers adds the imported servers to the configuration
func applyImportedServers(imported []*configimport.ImportedServer, globalConfig *config.Config) error {
func applyImportedServers(imported []*configimport.ImportedServer, globalConfig *config.Config) ([]*configimport.ImportedServer, []configimport.FailedServer, error) {
// Create context
ctx := reqcontext.WithMetadata(context.Background(), reqcontext.SourceCLI)
ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
Expand All @@ -2354,11 +2396,20 @@ func applyImportedServers(imported []*configimport.ImportedServer, globalConfig
}

// Direct config file mode
return applyImportedServersConfigMode(imported, globalConfig)
if err := applyImportedServersConfigMode(imported, globalConfig); err != nil {
failed := make([]configimport.FailedServer, 0, len(imported))
for _, s := range imported {
failed = append(failed, configimport.FailedServer{Name: s.Server.Name, Error: "apply_failed", Details: err.Error()})
}
return nil, failed, err
}
return imported, nil, nil
}

// applyImportedServersDaemonMode adds servers via the daemon
func applyImportedServersDaemonMode(ctx context.Context, client *cliclient.Client, imported []*configimport.ImportedServer) error {
func applyImportedServersDaemonMode(ctx context.Context, client *cliclient.Client, imported []*configimport.ImportedServer) ([]*configimport.ImportedServer, []configimport.FailedServer, error) {
var applied []*configimport.ImportedServer
var failed []configimport.FailedServer
for _, s := range imported {
req := &cliclient.AddServerRequest{
Name: s.Server.Name,
Expand All @@ -2377,12 +2428,15 @@ func applyImportedServersDaemonMode(ctx context.Context, client *cliclient.Clien

_, err := client.AddServer(ctx, req)
if err != nil {
// Log error but continue with other servers
fmt.Fprintf(os.Stderr, " ❌ Failed to add '%s': %v\n", s.Server.Name, err)
failed = append(failed, configimport.FailedServer{Name: s.Server.Name, Error: "apply_failed", Details: err.Error()})
continue
}
applied = append(applied, s)
}

return nil
if len(failed) > 0 {
return applied, failed, fmt.Errorf("failed to import %d of %d servers", len(failed), len(imported))
}
return applied, nil, nil
}

// runUpstreamInspect handles the 'upstream inspect' command (Spec 032)
Expand Down
Loading
Loading