fix: Align nicocli command names and prompts with the API

[kfelternv]

Renames the following commands to better match the REST api spec.

Old Command	New Command
tenant get-current-tenant	tenant current
tenant get-current-tenant-stats	tenant stats
infrastructure-provider get-current-infrastructure-provider	infrastructure-provider current
infrastructure-provider get-current-infrastructure-provider-stats	infrastructure-provider stats
service-account get	service-account current

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5532d549-bf13-407a-8bc9-0e4880d3dd16

📥 Commits

Reviewing files that changed from the base of the PR and between ca24f41 and 5cda16a.

📒 Files selected for processing (3)

• rest-api/cli/pkg/commands.go
• rest-api/cli/pkg/commands_test.go
• rest-api/cli/tui/commands.go

🚧 Files skipped from review as they are similar to previous changes (3)

• rest-api/cli/pkg/commands.go
• rest-api/cli/pkg/commands_test.go
• rest-api/cli/tui/commands.go

---

Summary by CodeRabbit

• New Features

  • vpc-prefix create now offers an interactive picker for IP block selection, with a manual entry fallback option.

• Bug Fixes

  • Fixed command resolution for singleton resources (tenant, infrastructure-provider, service-account) to correctly surface current and stats subcommands.

Walkthrough

This PR introduces two independent enhancements to the CLI infrastructure. First, it refactors operation-to-action mapping to treat singleton resources (e.g., get-current-infrastructure-provider) distinctly by routing them to the current action instead of collapsing them to get. Second, it upgrades VPC prefix creation to use interactive IP-block selection with human-friendly labels instead of requiring raw UUID entry, with fallback to manual input.

Changes

CLI and TUI command enhancements

Layer / File(s)	Summary
Singleton resource action mapping rest-api/cli/pkg/commands.go, rest-api/cli/pkg/commands_test.go	operationAction refactored to struct-based pattern matching: get-current-* operationIds route to the current action (via p.bare), get-* routes to get, and suffix handling for -stats and -status-history remains preserved. Command surface tests replaced with TestNewApp_CurrentSingletonCommandSurface to verify current and stats subcommands are exposed while bare get and full operationId variants are excluded. New tests confirm singleton reachability and allocation-constraint read-only constraints.
VPC prefix IP-block interactive picker rest-api/cli/tui/commands.go, rest-api/cli/tui/commands_test.go	cmdVPCPrefixCreate switched to promptVPCPrefixIPBlockID for scoped IP-block selection. Picker lists available blocks with site context, renders labeled items (name + status), and falls back to manual UUID entry on resolver errors or empty results. buildIPBlockSelectItems helper handles block-to-item conversion, skipping missing IDs and unconditionally appending the manual-entry sentinel. Comprehensive unit tests verify label rendering, empty-list handling, and missing-ID skipping.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the primary objective: renaming CLI commands to align with REST API specifications.
Description check	✅ Passed	The description directly relates to the changeset by documenting the command name mappings that are implemented in the refactored code.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

🔐 TruffleHog Secret Scan

✅ No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

_{🕐 Last updated: 2026-06-10 04:41:04 UTC | Commit: ca24f41}

[kfelternv]

@coderabbitai review this pr

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the PR diff
gh pr diff 2353 --repo NVIDIA/infra-controller

Length of output: 14968

---

🏁 Script executed:

#!/bin/bash
# Read the changed files
cat rest-api/cli/pkg/commands.go
echo "---FILE_BOUNDARY---"
cat rest-api/cli/pkg/commands_test.go
echo "---FILE_BOUNDARY---"
cat rest-api/cli/tui/commands.go
echo "---FILE_BOUNDARY---"
cat rest-api/cli/tui/commands_test.go

Length of output: 251680

---

---

@kfelternv — Reviewing PR #2353.

---

Review Summary

The core logic change in operationAction is sound and well-motivated: introducing bare as a named field instead of hardcoding "get" makes the intent explicit and allows get-current- singletons to map to current without disturbing the -stats/-status-history suffix checks. The TUI IP block picker is a clean quality-of-life improvement. A few items below merit attention before merge.

---

rest-api/cli/pkg/commands.go

Collision-expansion logic is now effectively dead for the embedded spec

The buildTagSubcommands collision-expansion block (actionCounts loop) can still fire for any future tag that has two operations collapsing to the same short action, so keeping it is correct. However, for infrastructure-provider and tenant, the collision can no longer arise from the embedded spec because current and stats are now distinct. The comment above the block still references the old failure mode:

// 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 example is now stale. Recommend updating the comment to use a less prominent example (or simply note that the guard exists for any collision, with the infra-provider/tenant collision being the formerly motivating case, now resolved at the operationAction level).

---

rest-api/cli/pkg/commands_test.go

1. Stale test fixtures in collision tests

TestBuildTagSubcommands_AliasCollisionExpandsAllNames, TestBuildTagSubcommands_AliasCollisionIsOrderIndependent, and TestBuildTagSubcommands_NonCollidingActionKeepsShortAlias all supply action: "get" for infrastructure-provider operations. With the new operationAction mapping, the real actions from the embedded spec are now current and stats, so neither a collision on get nor a single get action can arise from the real spec for this tag. The tests remain valid as unit tests of the collision resolver (using a synthetic input), but they silently misrepresent the current command surface in their names and comments.

Suggestion — add a brief note to each of these tests clarifying that they exercise the collision-resolver in isolation with synthetic data; the real regression guard is now TestNewApp_CurrentSingletonCommandSurface:

// NOTE: this test exercises the collision-resolver with synthetic ops
// (action="get" on both). In the embedded spec, get-current-<resource>
// singletons now produce distinct actions ("current"/"stats"), so this
// collision is hypothetical. The live regression guard is
// TestNewApp_CurrentSingletonCommandSurface.

2. TestBuildTagSubcommands_NonCollidingActionKeepsShortAlias asserts "get" for a singleton

op := &Operation{OperationID: "get-current-infrastructure-provider", ...}
ops := []resolvedOp{
    {tag: "Infrastructure Provider", action: "get", ...},
}
cmds := buildTagSubcommands(&Spec{}, ops)
assert.Equal(t, "get", cmds[0].Name, ...)

With the new operationAction, the same OperationID would produce action: "current", not "get". The test directly injects action: "get", so it passes, but the assertion text ("a single-op tag must keep its short alias") no longer reflects reality for this specific resource. Either use a non-singleton OperationID (e.g., "get-site") for this test, or update the comment to reflect the synthetic nature of the fixture.

3. Missing get-current-service-account-stats test case in TestOperationAction

TestBuildCommands_CurrentSingletonsAreRunnable tests all three singleton tags (tenant, infrastructure-provider, service-account). TestNewApp_CurrentSingletonCommandSurface only covers tenant and infrastructure-provider. If the embedded spec includes get-current-service-account-stats, the stats mapping for that operation is untested at the integration level. A minimal gap-fill:

// In TestOperationAction cases:
{"get-current-service-account-stats", "stats"},

// In TestNewApp_CurrentSingletonCommandSurface want map (if spec has the endpoint):
"service-account": {"current"}, // or {"current", "stats"} if stats endpoint exists

---

rest-api/cli/tui/commands.go

1. Redundant strings.TrimSpace in cmdVPCPrefixCreate

promptVPCPrefixIPBlockID always returns a trimmed string (both paths go through promptIPBlockIDRaw which calls strings.TrimSpace(raw), or the picker returns an already-clean ID). The downstream callers in cmdVPCPrefixCreate trim again:

"ipBlockId": strings.TrimSpace(ipBlockID),   // redundant
// ...
"--ip-block-id", strings.TrimSpace(ipBlockID), // redundant

These are harmless but noisy. Since the contract is already enforced at the source, the callers can simply use ipBlockID directly.

2. context.Context parameter order in promptVPCPrefixIPBlockID

func promptVPCPrefixIPBlockID(s *Session, ctx context.Context) (string, error)

The Go standard convention is ctx context.Context as the first parameter. This is consistent with existing codebase style (promptAllocationTenantID has the same inversion), so it is not a blocker — but worth noting for future cleanup.

---

rest-api/cli/tui/commands_test.go

The three new TestBuildIPBlockSelectItems_* tests are well-structured and cover the important edge cases (empty list, blank IDs, status in label, label fallback to ID). No issues.

---

Minor Nit

TestBuildCommands_AllocationConstraintIsUpdateOnly asserts the actions slice with a literal []string{"update"}. If the spec ever adds a second action, the test fails. This is intentional (the test is a guard against reintroduction of removed endpoints), so no change needed — just flagging that the ordering dependency on the BuildCommands sort is relied upon here.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[github-actions]

🔍 Container Scan Summary

Service	Total	Critical	High	Medium	Low	Other
nico-flow	116	13	50	41	4	8
nico-nsm	133	11	45	66	11	0
nico-psm	118	13	52	41	4	8
nico-rest-api	182	16	84	67	7	8
nico-rest-cert-manager	95	5	47	32	3	8
nico-rest-db	116	13	50	41	4	8
nico-rest-site-agent	115	13	50	41	3	8
nico-rest-site-manager	102	6	48	37	3	8
nico-rest-workflow	118	13	52	41	4	8
TOTAL	1095	103	478	407	43	64

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

[kfelternv]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.