fix(cli)!: address CLI and config robustness gaps (breaking: JSON-mode buy/create now requires --yes)

cmd/megaport/megaport.go

@@ -22,6 +22,13 @@ func init() {
 	// Initialize common components
 	InitializeCommon()
 
+	// Structurally tag flag-parsing failures (unknown flag, unknown shorthand
+	// flag, malformed value) as usage errors at the point cobra generates them,
+	// instead of pattern-matching the message text later in exitCodeFromError.
+	rootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error {
+		return exitcodes.NewUsageError(err)
+	})
+
 	// Apply non-WASM specific initialization
 	rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
 		defaultWarnings := applyDefaultSettings(cmd)
@@ -75,6 +82,14 @@ func init() {
 			return utils.FinishPreRunError(cmd, args, exitcodes.NewUsageError(err))
 		}
 
+		// Run cobra's own required-flags check here, ahead of when cobra would
+		// normally run it after PersistentPreRunE, so a missing required flag
+		// gets tagged as a typed usage error instead of falling back to
+		// isCobraUsageError's message-substring match.
+		if err := cmd.ValidateRequiredFlags(); err != nil {
+			return utils.FinishPreRunError(cmd, args, exitcodes.NewUsageError(err))
+		}
+
 		return nil
 	}
 
@@ -313,6 +328,11 @@ func exitCodeFromError(err error) int {
 	return exitcodes.General
 }
 
+// isCobraUsageError is a defensive fallback matching cobra's usage-error text.
+// Flag-parse and missing-required-flag errors are already typed as usage errors
+// at the source (SetFlagErrorFunc and the ValidateRequiredFlags check), so the
+// matches for those are a backstop for any that arrive untyped; "unknown command"
+// and the arg-count validators have no such hook and rely on this match.
 func isCobraUsageError(msg string) bool {
 	cobraPatterns := []string{
 		"unknown command",

cmd/megaport/megaport_test.go

@@ -351,3 +351,40 @@ func TestExitCodeFromError_CobraArgValidators(t *testing.T) {
 		})
 	}
 }
+
+// TestUnknownFlagIsTypedUsageError verifies that rootCmd.SetFlagErrorFunc
+// tags a flag-parse failure as a typed *exitcodes.CLIError at the point
+// cobra generates it, rather than relying on exitCodeFromError's message
+// substring match.
+func TestUnknownFlagIsTypedUsageError(t *testing.T) {
+	rootCmd.SetArgs([]string{"version", "--totally-bogus-flag"})
+	var execErr error
+	_ = output.CaptureOutput(func() {
+		execErr = rootCmd.Execute()
+	})
+
+	require.Error(t, execErr)
+	var cliErr *exitcodes.CLIError
+	require.True(t, errors.As(execErr, &cliErr), "expected a typed *exitcodes.CLIError, got %T: %v", execErr, execErr)
+	assert.Equal(t, exitcodes.Usage, cliErr.Code)
+}
+
+// TestMissingRequiredFlagIsTypedUsageError verifies that the proactive
+// cmd.ValidateRequiredFlags() check added to PersistentPreRunE tags a
+// missing-required-flag error as a typed *exitcodes.CLIError ahead of
+// cobra's own (redundant) ValidateRequiredFlags call.
+func TestMissingRequiredFlagIsTypedUsageError(t *testing.T) {
+	t.Setenv("MEGAPORT_CONFIG_DIR", t.TempDir())
+	defer output.ResetState()
+
+	rootCmd.SetArgs([]string{"billing-market", "set"})
+	var execErr error
+	_ = output.CaptureOutput(func() {
+		execErr = rootCmd.Execute()
+	})
+
+	require.Error(t, execErr)
+	var cliErr *exitcodes.CLIError
+	require.True(t, errors.As(execErr, &cliErr), "expected a typed *exitcodes.CLIError, got %T: %v", execErr, execErr)
+	assert.Equal(t, exitcodes.Usage, cliErr.Code)
+}

docs/index.md

@@ -1,6 +1,6 @@
 # Megaport CLI Documentation
 
-> Generated on July 8, 2026 for version v1.0.0-beta.1
+> Generated on July 13, 2026 for version v1.0.0-beta.1
 
 ## Available Commands
 

docs/megaport-cli_ix_buy.md

@@ -77,5 +77,5 @@ megaport-cli ix buy [flags]
 | `--rate-limit` |  | `0` | Rate limit in Mbps | true |
 | `--shutdown` |  | `false` | Whether the IX is initially shut down | false |
 | `--vlan` |  | `0` | VLAN ID for the IX connection | true |
-| `--yes` | `-y` | `false` | Skip confirmation prompt for purchase | false |
+| `--yes` | `-y` | `false` | Skip confirmation prompt for purchase; required when using --json or --json-file | false |
 

docs/megaport-cli_mcr_buy.md

@@ -96,5 +96,5 @@ megaport-cli mcr buy [flags]
 | `--resource-tags` |  |  | Resource tags as a JSON string (e.g. {"key1":"value1","key2":"value2"}) | false |
 | `--resource-tags-file` |  |  | Path to JSON file containing resource tags | false |
 | `--term` |  | `0` | The term of the MCR (1, 12, 24, 36, 48, or 60 months) | true |
-| `--yes` | `-y` | `false` | Skip confirmation prompt for purchase | false |
+| `--yes` | `-y` | `false` | Skip confirmation prompt for purchase; required when using --json or --json-file | false |
 

docs/megaport-cli_mve_buy.md

@@ -100,5 +100,5 @@ megaport-cli mve buy [flags]
 | `--term` |  | `0` | The term of the MVE (1, 12, 24, 36, 48, or 60 months) | true |
 | `--vendor-config` |  |  | JSON string with vendor-specific configuration (for flag mode) | true |
 | `--vnics` |  |  | JSON array of network interfaces (for flag mode) | true |
-| `--yes` | `-y` | `false` | Skip confirmation prompt for purchase | false |
+| `--yes` | `-y` | `false` | Skip confirmation prompt for purchase; required when using --json or --json-file | false |
 

docs/megaport-cli_nat-gateway_create.md

@@ -84,5 +84,5 @@ megaport-cli nat-gateway create [flags]
 | `--session-count` |  | `0` | The number of NAT sessions (optional) | false |
 | `--speed` |  | `0` | The speed of the NAT Gateway in Mbps | true |
 | `--term` |  | `0` | The contract term in months (1, 12, 24, 36, 48, or 60) | true |
-| `--yes` | `-y` | `false` | Skip the confirmation prompt for creating the NAT Gateway design (no charges are incurred until 'nat-gateway buy') | false |
+| `--yes` | `-y` | `false` | Skip the confirmation prompt for creating the NAT Gateway design (no charges are incurred until 'nat-gateway buy'); required when using --json or --json-file | false |
 

docs/megaport-cli_ports_buy-lag.md

@@ -85,5 +85,5 @@ megaport-cli ports buy-lag [flags]
 | `--resource-tags` |  |  | Resource tags as a JSON string (e.g. {"key1":"value1","key2":"value2"}) | false |
 | `--resource-tags-file` |  |  | Path to JSON file containing resource tags | false |
 | `--term` |  | `0` | The term of the port (1, 12, or 24 months) | true |
-| `--yes` | `-y` | `false` | Skip confirmation prompt for purchase | false |
+| `--yes` | `-y` | `false` | Skip confirmation prompt for purchase; required when using --json or --json-file | false |
 

docs/megaport-cli_ports_buy.md

@@ -84,5 +84,5 @@ megaport-cli ports buy [flags]
 | `--resource-tags` |  |  | Resource tags as a JSON string (e.g. {"key1":"value1","key2":"value2"}) | false |
 | `--resource-tags-file` |  |  | Path to JSON file containing resource tags | false |
 | `--term` |  | `0` | The term of the port (1, 12, 24, 36, 48, or 60 months) | true |
-| `--yes` | `-y` | `false` | Skip confirmation prompt for purchase | false |
+| `--yes` | `-y` | `false` | Skip confirmation prompt for purchase; required when using --json or --json-file | false |
 

docs/megaport-cli_vxc_buy.md

@@ -95,5 +95,5 @@ megaport-cli vxc buy [flags]
 | `--resource-tags-file` |  |  | Path to JSON file containing resource tags | false |
 | `--service-key` |  |  | Service key | false |
 | `--term` |  | `0` | Contract term in months (1, 12, 24, 36, 48, or 60) | true |
-| `--yes` | `-y` | `false` | Skip confirmation prompt for purchase | false |
+| `--yes` | `-y` | `false` | Skip confirmation prompt for purchase; required when using --json or --json-file | false |
 

internal/base/cmdbuilder/docs/megaport-cli.md

@@ -0,0 +1,3 @@
+# Embedded Sample
+
+Embedded documentation used by docs_render tests.

internal/base/cmdbuilder/docs_render.go

@@ -7,7 +7,6 @@ import (
 	"os"
 	"path"
 	"path/filepath"
-	"strings"
 
 	"github.com/charmbracelet/glamour"
 	"github.com/spf13/cobra"
@@ -16,8 +15,9 @@ import (
 // DocsDirectory is the fallback location for markdown documentation files
 var DocsDirectory = "./docs"
 
-// FindDocFile locates the markdown file for a specific command
-func FindDocFile(cmd *cobra.Command) (string, error) {
+// FindDocFile locates the markdown file for a specific command. The second
+// return value reports whether the path is a temp file the caller must remove.
+func FindDocFile(cmd *cobra.Command) (string, bool, error) {
 	cmdPath := getCommandPath(cmd)
 	docName := cmdPath + ".md"
 
@@ -30,29 +30,29 @@ func FindDocFile(cmd *cobra.Command) (string, error) {
 		// by path and the caller removes it after rendering.
 		tempFile, err := os.CreateTemp("", "megaport-docs-*.md")
 		if err != nil {
-			return "", fmt.Errorf("failed to create temporary file: %w", err)
+			return "", false, fmt.Errorf("failed to create temporary file: %w", err)
 		}
 		if _, err := tempFile.Write(content); err != nil {
 			_ = tempFile.Close()
 			_ = os.Remove(tempFile.Name())
-			return "", fmt.Errorf("failed to write to temporary file: %w", err)
+			return "", false, fmt.Errorf("failed to write to temporary file: %w", err)
 		}
 		if err := tempFile.Close(); err != nil {
 			_ = os.Remove(tempFile.Name())
-			return "", fmt.Errorf("failed to close temporary file: %w", err)
+			return "", false, fmt.Errorf("failed to close temporary file: %w", err)
 		}
-		return tempFile.Name(), nil
+		return tempFile.Name(), true, nil
 	}
 
 	// If embedded file not found, try local docs directory as fallback
 	docPath := filepath.Join(DocsDirectory, docName)
 
 	// Check if the file exists
 	if _, err := os.Stat(docPath); os.IsNotExist(err) {
-		return "", fmt.Errorf("documentation file not found for %s: %w", cmdPath, err)
+		return "", false, fmt.Errorf("documentation file not found for %s: %w", cmdPath, err)
 	}
 
-	return docPath, nil
+	return docPath, false, nil
 }
 
 // RenderDocFile reads and renders a markdown file using Glamour
@@ -83,13 +83,13 @@ func RenderDocFile(filePath string) (string, error) {
 
 // ShowDocumentation displays rendered documentation for a command
 func ShowDocumentation(cmd *cobra.Command) error {
-	docPath, err := FindDocFile(cmd)
+	docPath, isTemp, err := FindDocFile(cmd)
 	if err != nil {
 		return err
 	}
 
 	// If we created a temporary file, ensure it gets deleted
-	if strings.Contains(docPath, "megaport-docs-") {
+	if isTemp {
 		defer os.Remove(docPath)
 	}
 

internal/base/cmdbuilder/docs_render_native_test.go

@@ -12,6 +12,9 @@ import (
 	"github.com/spf13/cobra"
 )
 
+//go:embed docs/megaport-cli.md
+var embeddedTestDocsFS embed.FS
+
 // ShowDocumentation must write to the command's configured output writer, not
 // straight to os.Stdout, so callers (and tests) can capture/redirect docs output.
 func TestShowDocumentationWritesToCobraWriter(t *testing.T) {
@@ -39,3 +42,54 @@ func TestShowDocumentationWritesToCobraWriter(t *testing.T) {
 		t.Fatal("expected rendered docs on the cobra writer, got nothing")
 	}
 }
+
+// ShowDocumentation must stage embedded docs in a temp file and clean it up
+// afterwards, since embed.FS content can't be read back by path directly.
+func TestShowDocumentationEmbeddedTempFile(t *testing.T) {
+	origFS := embeddedDocsFS
+	embeddedDocsFS = embeddedTestDocsFS
+	t.Cleanup(func() { embeddedDocsFS = origFS })
+
+	root := &cobra.Command{Use: "megaport-cli"}
+	var buf bytes.Buffer
+	root.SetOut(&buf)
+
+	docPath, isTemp, err := FindDocFile(root)
+	if err != nil {
+		t.Fatalf("FindDocFile: %v", err)
+	}
+	if !isTemp {
+		t.Fatal("expected embedded doc to be staged in a temp file")
+	}
+	defer os.Remove(docPath)
+
+	if _, err := os.Stat(docPath); err != nil {
+		t.Fatalf("expected temp doc file to exist: %v", err)
+	}
+
+	if err := ShowDocumentation(root); err != nil {
+		t.Fatalf("ShowDocumentation: %v", err)
+	}
+	if buf.Len() == 0 {
+		t.Fatal("expected rendered docs on the cobra writer, got nothing")
+	}
+}
+
+func TestFindDocFileNotFound(t *testing.T) {
+	origFS := embeddedDocsFS
+	embeddedDocsFS = embed.FS{} // force the on-disk fallback
+	t.Cleanup(func() { embeddedDocsFS = origFS })
+
+	origDir := DocsDirectory
+	DocsDirectory = t.TempDir() // empty, no doc files present
+	t.Cleanup(func() { DocsDirectory = origDir })
+
+	root := &cobra.Command{Use: "megaport-cli"}
+	_, isTemp, err := FindDocFile(root)
+	if err == nil {
+		t.Fatal("expected an error for a missing doc file")
+	}
+	if isTemp {
+		t.Fatal("expected isTemp to be false when no doc file is found")
+	}
+}

internal/base/cmdbuilder/flagsets.go

@@ -81,7 +81,7 @@ func (b *CommandBuilder) WithDeferredDeleteFlags() *CommandBuilder {
 
 // WithBuyConfirmFlags adds the --yes/-y flag to skip buy confirmation prompts
 func (b *CommandBuilder) WithBuyConfirmFlags() *CommandBuilder {
-	b.WithBoolFlagP("yes", "y", false, "Skip confirmation prompt for purchase")
+	b.WithBoolFlagP("yes", "y", false, "Skip confirmation prompt for purchase; required when using --json or --json-file")
 	return b
 }
 

internal/base/output/common.go

@@ -452,6 +452,63 @@ func filterByFields(headers, jsonNames []string, indices []int, selected []strin
 	return outHeaders, outJSONNames, outIndices, nil
 }
 
+// isXMLNameStartChar reports whether r is legal as the first character of an XML
+// element local name. Colon is excluded on purpose: it is namespace-significant,
+// so a name like "a:b" would trip an undeclared-prefix error in aware parsers.
+func isXMLNameStartChar(r rune) bool {
+	return r == '_' || (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z')
+}
+
+// isXMLNameChar reports whether r is legal as a non-first character of an XML
+// element local name.
+func isXMLNameChar(r rune) bool {
+	return isXMLNameStartChar(r) || r == '.' || r == '-' || (r >= '0' && r <= '9')
+}
+
+// sanitizeXMLElementName rewrites name so it is safe to use as an XML element
+// local name. Struct json tags are written for JSON, not XML, so a name like
+// "a/b" or one starting with a digit would otherwise produce malformed XML.
+func sanitizeXMLElementName(name string) string {
+	var b strings.Builder
+	b.Grow(len(name))
+	for _, r := range name {
+		if isXMLNameChar(r) {
+			b.WriteRune(r)
+		} else {
+			b.WriteRune('_')
+		}
+	}
+	name = b.String()
+	if name == "" {
+		return "field"
+	}
+	if !isXMLNameStartChar(rune(name[0])) {
+		name = "_" + name
+	}
+	if strings.HasPrefix(strings.ToLower(name), "xml") {
+		name = "_" + name
+	}
+	return name
+}
+
+// sanitizeXMLElementNames sanitizes each name and disambiguates any collisions
+// caused by distinct names sanitizing to the same value (e.g. "a/b" and "a b"
+// both becoming "a_b"), so no two elements in the same XML item share a name.
+func sanitizeXMLElementNames(names []string) []string {
+	used := make(map[string]bool, len(names))
+	out := make([]string, len(names))
+	for i, name := range names {
+		base := sanitizeXMLElementName(name)
+		candidate := base
+		for n := 2; used[candidate]; n++ {
+			candidate = fmt.Sprintf("%s_%d", base, n)
+		}
+		used[candidate] = true
+		out[i] = candidate
+	}
+	return out
+}
+
 // isOutputCompatibleType checks if a type can be output
 func isOutputCompatibleType(t reflect.Type) bool {
 	// Handle pointer types by checking the element type

internal/base/output/output.go

@@ -120,6 +120,8 @@ func printXML[T OutputFields](data []T, opts printOptions) error {
 		}
 	}
 
+	xmlNames := sanitizeXMLElementNames(jsonNames)
+
 	encoder := xml.NewEncoder(os.Stdout)
 	encoder.Indent("", "  ")
 
@@ -143,8 +145,8 @@ func printXML[T OutputFields](data []T, opts printOptions) error {
 			return err
 		}
 
-		for i, name := range jsonNames {
-			elemStart := xml.StartElement{Name: xml.Name{Local: name}}
+		for i := range jsonNames {
+			elemStart := xml.StartElement{Name: xml.Name{Local: xmlNames[i]}}
 			if err := encoder.EncodeToken(elemStart); err != nil {
 				return err
 			}