fix: Align nicocli command names and prompts with the API

rest-api/cli/pkg/commands.go

@@ -158,14 +158,19 @@ func buildTagSubcommands(spec *Spec, ops []resolvedOp) []*cli.Command {
 	}
 
 	// Resolve action-name collisions symmetrically: when two or more
-	// operations under the same tag collapse to the same short action (e.g.
-	// `get-current-infrastructure-provider` and
-	// `get-current-infrastructure-provider-stats` both -> `get`), expand ALL
-	// of them to their full OperationID. The previous "first one wins" pass
-	// produced a different command surface depending on the order of map
+	// operations under the same tag collapse to the same short action, expand
+	// ALL of them to their full OperationID. The previous "first one wins"
+	// pass produced a different command surface depending on the order of map
 	// iteration in collectOperations, which depended on whether the user's
 	// config file had been loaded -- the same binary exposed different
 	// commands in the two states.
+	//
+	// The formerly motivating case -- `get-current-infrastructure-provider`
+	// and `get-current-infrastructure-provider-stats` both collapsing to
+	// `get` -- no longer collides: get-current-* singletons now map to
+	// distinct `current` / `stats` actions in operationAction. This guard
+	// stays for any future tag whose operations still collide on a short
+	// action.
 	actionCounts := make(map[string]int)
 	for _, op := range primaryOps {
 		actionCounts[op.action]++
@@ -806,31 +811,44 @@ func operationAction(opID string) string {
 	patterns := []struct {
 		prefix string
 		action string
+		// bare is the action for a getter prefix (action == "") when the
+		// operation has no -stats / -status-history suffix. A plain
+		// get-<resource> is the `get` action; a get-current-<resource>
+		// singleton getter is the `current` action so the generated command
+		// matches the REST /current path and the interactive TUI command name
+		// (e.g. `tenant current`, `infrastructure-provider current`).
+		bare string
 	}{
-		{"batch-create-", "batch-create"},
-		{"batch-update-", "batch-update"},
-		{"get-all-", "list"},
-		{"get-current-", "get"},
-		{"create-", "create"},
-		{"update-", "update"},
-		{"delete-", "delete"},
-		{"get-", ""},
+		{prefix: "batch-create-", action: "batch-create"},
+		{prefix: "batch-update-", action: "batch-update"},
+		{prefix: "get-all-", action: "list"},
+		{prefix: "get-current-", bare: "current"},
+		{prefix: "create-", action: "create"},
+		{prefix: "update-", action: "update"},
+		{prefix: "delete-", action: "delete"},
+		{prefix: "get-", bare: "get"},
 	}
 
 	for _, p := range patterns {
-		if strings.HasPrefix(opID, p.prefix) {
-			if p.action != "" {
-				return p.action
-			}
-			suffix := opID[len(p.prefix):]
-			if strings.HasSuffix(suffix, "-status-history") {
-				return "status-history"
-			}
-			if strings.HasSuffix(suffix, "-stats") {
-				return "stats"
-			}
-			return "get"
+		if !strings.HasPrefix(opID, p.prefix) {
+			continue
+		}
+		if p.action != "" {
+			return p.action
+		}
+		// Getter prefix: a -stats or -status-history endpoint collapses to
+		// its own action so a resource's getter and its stats / history
+		// endpoints stay distinct commands instead of colliding on one action
+		// (which would force both to expand to their full operationId). This
+		// applies to both get- and get-current-.
+		suffix := opID[len(p.prefix):]
+		if strings.HasSuffix(suffix, "-status-history") {
+			return "status-history"
+		}
+		if strings.HasSuffix(suffix, "-stats") {
+			return "stats"
 		}
+		return p.bare
 	}
 
 	return opID

rest-api/cli/pkg/commands_test.go

@@ -98,9 +98,9 @@ func TestOperationAction(t *testing.T) {
 		{"get-all-site", "list"},
 		{"get-all-instance", "list"},
 		{"get-all-allocation-constraint", "list"},
-		{"get-current-infrastructure-provider", "get"},
-		{"get-current-tenant", "get"},
-		{"get-current-service-account", "get"},
+		{"get-current-infrastructure-provider", "current"},
+		{"get-current-tenant", "current"},
+		{"get-current-service-account", "current"},
 		{"create-site", "create"},
 		{"create-allocation-constraint", "create"},
 		{"update-site", "update"},
@@ -110,9 +110,13 @@ func TestOperationAction(t *testing.T) {
 		{"get-site-status-history", "status-history"},
 		{"get-instance-status-history", "status-history"},
 		{"get-machine-status-history", "status-history"},
-		// get-current-* matches before -stats suffix check
-		{"get-current-infrastructure-provider-stats", "get"},
-		{"get-current-tenant-stats", "get"},
+		// get-current-<resource> singletons resolve to `current`; their
+		// -stats endpoints resolve to `stats` (distinct actions, no collision).
+		// service-account has no -stats endpoint in the spec today, but the
+		// suffix mapping is resource-independent, so it is asserted here too.
+		{"get-current-infrastructure-provider-stats", "stats"},
+		{"get-current-tenant-stats", "stats"},
+		{"get-current-service-account-stats", "stats"},
 		{"batch-create-instance", "batch-create"},
 		{"batch-create-expected-machines", "batch-create"},
 		{"batch-update-expected-machines", "batch-update"},
@@ -674,6 +678,12 @@ func TestIsActionModifier(t *testing.T) {
 // short alias non-deterministically points at one of the two operations
 // depending on map iteration order, so the same binary exposes a different
 // command surface depending on whether the config file was loaded.
+//
+// NOTE: this exercises the collision resolver with synthetic ops (action
+// "get" injected on both). In the embedded spec these two operations now map
+// to distinct `current` / `stats` actions, so this exact collision no longer
+// arises there; the live singleton-surface guard is
+// TestNewApp_CurrentSingletonCommandSurface.
 func TestBuildTagSubcommands_AliasCollisionExpandsAllNames(t *testing.T) {
 	infraProviderGet := &Operation{
 		OperationID: "get-current-infrastructure-provider",
@@ -704,14 +714,16 @@ func TestBuildTagSubcommands_AliasCollisionExpandsAllNames(t *testing.T) {
 
 // TestBuildTagSubcommands_NonCollidingActionKeepsShortAlias is the negative
 // counterpart to the collision test above: when there is exactly one
-// operation per short action, the short alias is preserved.
+// operation per short action, the short alias is preserved. Uses a plain
+// get-<resource> op (a genuine `get` action) so the fixture is self-consistent
+// -- get-current-* singletons now map to `current`, not `get`.
 func TestBuildTagSubcommands_NonCollidingActionKeepsShortAlias(t *testing.T) {
 	op := &Operation{
-		OperationID: "get-current-infrastructure-provider",
-		Tags:        []string{"Infrastructure Provider"},
+		OperationID: "get-site",
+		Tags:        []string{"Site"},
 	}
 	ops := []resolvedOp{
-		{tag: "Infrastructure Provider", action: "get", method: "GET", path: "/p1", op: op},
+		{tag: "Site", action: "get", method: "GET", path: "/p1", op: op},
 	}
 
 	cmds := buildTagSubcommands(&Spec{}, ops)
@@ -726,6 +738,9 @@ func TestBuildTagSubcommands_NonCollidingActionKeepsShortAlias(t *testing.T) {
 // running the resolver against both possible orderings of the colliding
 // operations. The resulting command tree must be identical, so the binary's
 // command surface no longer depends on Go map iteration order.
+//
+// NOTE: synthetic action "get" is injected on both ops to force the collision;
+// in the embedded spec these resolve to distinct `current` / `stats` actions.
 func TestBuildTagSubcommands_AliasCollisionIsOrderIndependent(t *testing.T) {
 	infraProviderGet := &Operation{
 		OperationID: "get-current-infrastructure-provider",
@@ -761,27 +776,27 @@ func TestBuildTagSubcommands_AliasCollisionIsOrderIndependent(t *testing.T) {
 		"command surface must not depend on the order primaryOps is built in")
 }
 
-// TestNewApp_NoConfigDependentCommandSurface walks the embedded production
-// spec and asserts that the two known alias-collision pairs (Infrastructure
-// Provider and Tenant) expose the full operationId-derived names with no
-// short `get` alias. This is the regression guard that fires if a future
-// change re-introduces a "first one wins" alias resolver.
-func TestNewApp_NoConfigDependentCommandSurface(t *testing.T) {
+// TestNewApp_CurrentSingletonCommandSurface walks the embedded production spec
+// and asserts that the get-current-<resource> singletons expose the `current`
+// action (and `stats` for their -stats endpoints) instead of the bare `get`
+// action or the full operationId. Because `current` and `stats` are distinct
+// actions there is no get-action collision, so the command surface is
+// deterministic and never depends on Go map iteration order. This is the
+// regression guard for NVBug 6100988 (`tenant current` must be runnable from
+// the command line, matching what the interactive TUI prints) and replaces the
+// old guard that asserted the colliding pair expanded to full operationIds.
+func TestNewApp_CurrentSingletonCommandSurface(t *testing.T) {
 	app, err := NewApp(openapi.Spec)
 	require.NoError(t, err, "NewApp failed")
 
-	collidingPairs := map[string][]string{
-		"infrastructure-provider": {
-			"get-current-infrastructure-provider",
-			"get-current-infrastructure-provider-stats",
-		},
-		"tenant": {
-			"get-current-tenant",
-			"get-current-tenant-stats",
-		},
+	// tag -> actions that must be present, with no bare `get` and no
+	// full-operationId leftovers.
+	want := map[string][]string{
+		"infrastructure-provider": {"current", "stats"},
+		"tenant":                  {"current", "stats"},
 	}
 
-	for tag, expected := range collidingPairs {
+	for tag, actions := range want {
 		t.Run(tag, func(t *testing.T) {
 			var found *cli.Command
 			for _, c := range app.Commands {
@@ -796,17 +811,84 @@ func TestNewApp_NoConfigDependentCommandSurface(t *testing.T) {
 			for _, sc := range found.Subcommands {
 				subNames[sc.Name] = true
 			}
-			for _, want := range expected {
-				assert.Truef(t, subNames[want],
-					"tag %q must expose %q as a deterministic, full-name command", tag, want)
+			for _, a := range actions {
+				assert.Truef(t, subNames[a], "tag %q must expose the %q command", tag, a)
 			}
 			assert.Falsef(t, subNames["get"],
-				"tag %q must NOT expose `get` as a short alias when there are %d colliding operations",
-				tag, len(expected))
+				"tag %q must NOT expose a bare `get`; the singleton getter is `current`", tag)
+			assert.Falsef(t, subNames["get-current-"+tag],
+				"tag %q must NOT expose the full-operationId command; it collapses to `current`", tag)
 		})
 	}
 }
 
+// TestBuildCommands_CurrentSingletonsAreRunnable asserts that every
+// get-current-<resource> singleton in the embedded spec is reachable from the
+// non-interactive CLI under the `current` action that the interactive TUI
+// prints (NVBug 6100988). Driven off the spec so it stays honest as singletons
+// are added or removed.
+func TestBuildCommands_CurrentSingletonsAreRunnable(t *testing.T) {
+	spec, err := ParseSpec(openapi.Spec)
+	require.NoError(t, err)
+	cmds := BuildCommands(spec)
+
+	cmdByName := func(list []*cli.Command, name string) *cli.Command {
+		for _, c := range list {
+			if c.HasName(name) {
+				return c
+			}
+		}
+		return nil
+	}
+
+	for _, tag := range []string{"tenant", "infrastructure-provider", "service-account"} {
+		t.Run(tag, func(t *testing.T) {
+			parent := cmdByName(cmds, tag)
+			require.NotNilf(t, parent, "tag %q must be a top-level command", tag)
+			assert.NotNilf(t, cmdByName(parent.Subcommands, "current"),
+				"tag %q must expose a `current` command runnable from the CLI", tag)
+		})
+	}
+}
+
+// TestBuildCommands_AllocationConstraintIsUpdateOnly is the CLI-side guard for
+// NVBug 6232163: the server only registers PATCH for the AllocationConstraint
+// sub-resource, and the stale create/get/list/delete endpoints were removed
+// from the OpenAPI spec. Because the CLI is generated from the embedded spec,
+// the `allocation constraint` sub-resource must therefore expose only `update`.
+// This test fails if the removed endpoints are ever reintroduced into the spec.
+func TestBuildCommands_AllocationConstraintIsUpdateOnly(t *testing.T) {
+	spec, err := ParseSpec(openapi.Spec)
+	require.NoError(t, err)
+	cmds := BuildCommands(spec)
+
+	var allocation *cli.Command
+	for _, c := range cmds {
+		if c.Name == "allocation" {
+			allocation = c
+			break
+		}
+	}
+	require.NotNil(t, allocation, "allocation command must exist")
+
+	var constraint *cli.Command
+	for _, sc := range allocation.Subcommands {
+		if sc.Name == "constraint" {
+			constraint = sc
+			break
+		}
+	}
+	require.NotNil(t, constraint, "allocation `constraint` sub-resource must exist")
+
+	actions := make([]string, 0, len(constraint.Subcommands))
+	for _, sc := range constraint.Subcommands {
+		actions = append(actions, sc.Name)
+	}
+	assert.Equal(t, []string{"update"}, actions,
+		"allocation constraint must expose only `update`; create/get/list/delete were "+
+			"removed from the OpenAPI spec because the server never registered those routes (NVBug 6232163)")
+}
+
 // sortStrings is a tiny stable sort used by the order-independence test so it
 // stays self-contained and does not pull in sort.Strings (which is already
 // used elsewhere; this just keeps the test readable).

rest-api/cli/tui/commands.go

@@ -2217,18 +2217,20 @@ func cmdVPCPrefixCreate(s *Session, _ []string) error {
 	if prefixLen < 8 || prefixLen > 31 {
 		return fmt.Errorf("prefix length must be between 8 and 31")
 	}
-	ipBlockID, err := PromptText("IP block ID", true)
+	ipBlockID, err := promptVPCPrefixIPBlockID(s, context.Background())
 	if err != nil {
 		return err
 	}
 
+	// ipBlockID is already trimmed by promptVPCPrefixIPBlockID (picker IDs are
+	// clean; the manual-entry path trims), so no extra TrimSpace here.
 	body := map[string]interface{}{
 		"name":         name,
 		"vpcId":        vpc.ID,
-		"ipBlockId":    strings.TrimSpace(ipBlockID),
+		"ipBlockId":    ipBlockID,
 		"prefixLength": prefixLen,
 	}
-	LogCmd(s, "vpc-prefix", "create", "--name", name, "--vpc-id", vpc.ID, "--ip-block-id", strings.TrimSpace(ipBlockID), "--prefix-length", prefixLenText)
+	LogCmd(s, "vpc-prefix", "create", "--name", name, "--vpc-id", vpc.ID, "--ip-block-id", ipBlockID, "--prefix-length", prefixLenText)
 	bodyJSON, _ := json.Marshal(body)
 	resp, _, err := s.Client.Do("POST", apiPath(s, "vpc-prefix"), nil, nil, bodyJSON)
 	if err != nil {
@@ -2242,6 +2244,72 @@ func cmdVPCPrefixCreate(s *Session, _ []string) error {
 	return nil
 }
 
+// ipBlockManualEntrySentinel is the select ID used for the trailing "enter
+// manually" option in the IP block picker, mirroring tenantManualEntrySentinel.
+const ipBlockManualEntrySentinel = "__manual__"
+
+// promptVPCPrefixIPBlockID picks the IP block for a new VPC prefix. ipBlockId
+// is required by the API (APIVpcPrefixCreateRequest.Validate), so rather than
+// make the operator paste a raw UUID, list the IP blocks already scoped to the
+// VPC's site and let them choose one. Falls back to manual entry when no IP
+// blocks are visible, when listing fails, or when the operator opts out via
+// the trailing sentinel (NVBug 6105076).
+func promptVPCPrefixIPBlockID(s *Session, ctx context.Context) (string, error) {
+	blocks, err := s.Resolver.Fetch(ctx, "ip-block")
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "%s could not list IP blocks (%v); falling back to manual entry\n", Dim("note:"), err)
+		return promptIPBlockIDRaw()
+	}
+	items := buildIPBlockSelectItems(blocks)
+	if len(items) == 1 {
+		// Only the manual-entry sentinel: no IP blocks for this site.
+		fmt.Fprintf(os.Stderr, "%s no IP blocks found for this site; enter an IP block ID manually\n", Dim("note:"))
+		return promptIPBlockIDRaw()
+	}
+	selected, err := Select("IP block:", items)
+	if err != nil {
+		return "", err
+	}
+	if selected.ID == ipBlockManualEntrySentinel {
+		return promptIPBlockIDRaw()
+	}
+	return selected.ID, nil
+}
+
+func promptIPBlockIDRaw() (string, error) {
+	raw, err := PromptText("IP block ID", true)
+	if err != nil {
+		return "", err
+	}
+	return strings.TrimSpace(raw), nil
+}
+
+// buildIPBlockSelectItems turns the resolver's IP block list into picker
+// options whose ID is the IP block UUID and whose label surfaces the block
+// name (falling back to the UUID when unnamed) plus status. A trailing
+// manual-entry sentinel is always appended -- even for an empty list -- so the
+// operator can still type a raw UUID for a block that isn't listed in the
+// current scope. Blocks without an ID are skipped.
+func buildIPBlockSelectItems(blocks []NamedItem) []SelectItem {
+	items := make([]SelectItem, 0, len(blocks)+1)
+	for _, b := range blocks {
+		id := strings.TrimSpace(b.ID)
+		if id == "" {
+			continue
+		}
+		label := strings.TrimSpace(b.Name)
+		if label == "" {
+			label = id
+		}
+		if strings.TrimSpace(b.Status) != "" {
+			label += "  " + Dim(b.Status)
+		}
+		items = append(items, SelectItem{Label: label, ID: id})
+	}
+	items = append(items, SelectItem{Label: "Enter IP block ID manually...", ID: ipBlockManualEntrySentinel})
+	return items
+}
+
 func cmdVPCPrefixUpdate(s *Session, args []string) error {
 	item, err := s.Resolver.ResolveWithArgs(context.Background(), "vpc-prefix", "VPC Prefix to update", args)
 	if err != nil {