refactor(cli): various cleanups and bug fixes

[geovannewashington]

What changed?

A series of small correctness fixes and cleanups across the CLI service
and command layers.

Why

UserList was returning raw store errors directly, unlike every other
service method which wraps them in sentinel values from errors.go.
This leaked implementation details to the cmd layer.

NamespaceDelete.Namespace had no validation tag. Cobra's ExactArgs
only enforces argument count - invalid names like My.Namespace were
reaching the store instead of being rejected early with a clear error.

namespaceInspect was making N+1 DB round trips - one UserResolve
call per member plus a separate one for the owner. Replaced with a
single UserList call upfront and an in-memory map for lookups,
bringing it down to 2 queries regardless of member count.

Deprecated commands had been marked for removal since v0.16.0, over
two years and eight minor versions ago. Removed with team approval.

getMaxDevices() lived in its own utils.go file despite being a
single function called only by NamespaceCreate. Moved to
namespaces.go and deleted the file.

Added the missing doc comment to UserList in the UserStore
interface, which was the only method without one.

How to test

1. Start the environment with ./bin/docker-compose up -d
2. Run the following and verify expected behavior:

# invalid namespace name must be rejected early
./bin/cli namespace delete My.Namespace  # must error: namespace is invalid

# inspect must show all members correctly
./bin/cli user create user1 Secret123! user1@test.com
./bin/cli user create user2 Secret123! user2@test.com
./bin/cli namespace create testns user1
./bin/cli namespace member add user2 testns observer
./bin/cli namespace inspect testns

# deprecated commands must no longer exist
./bin/cli add-user testuser2 Secret123! test2@test.com  # must error

[github-actions]

Claude finished @geovannewashington's task in 6m 56s —— View job

---

Code Review Complete

Reviewed 10 files across code quality, security, testing, Go patterns, and architecture — 1 issue found.

Test assertion not updated for UserList error wrapping

Severity: high  ·  File: cli/services/users_test.go:563

UserList was updated to return ErrFailedListUsers instead of the raw store error (users.go:155-157), but TestUserList still asserts the raw storeErr:

// line 563 — current (will fail)
expectedErr: storeErr,

// fix
expectedErr: ErrFailedListUsers,

The parallel NamespaceList change had its test correctly updated to expect ErrFailedListNamespaces (namespaces_test.go:773), but this one was missed. The storeErr variable is still needed for the mock's .Return(nil, 0, storeErr) call — only the assertion needs to change.

Fix this →

---

If you've addressed the feedback and want a new review, tag @shellhub-io/admin and a team member can trigger it.

[otavio]

/review

[github-actions]

Claude finished @otavio's task in 6m 12s —— View job

---

Code Review Complete

Reviewed 10 files across code quality, security, testing, Go patterns, and architecture — no issues found. The code looks good as-is.

The issue from the previous review (users_test.go:563 asserting storeErr instead of ErrFailedListUsers) has been correctly addressed.

---

If you push additional changes and want a new review, tag @shellhub-io/admin and a team member can trigger it.