fix(auth): align GraphQL queries/mutations with backend schema and add nubi fallbacks

[blue4209211]

Description

Fixes compatibility issues between nbctl and backend GraphQL schemas for auth subcommands, and improves resilience for nubi commands.

Auth Subcommand Fixes

• cmd/auth_groups.go:
  • Update usergroups_list query to request group_roles and member_count instead of roles and user_count.
  • Update usergroup_create mutation to pass (name: $name, description: $description) returning { id }.
• cmd/auth_assign_role.go:
  • Update userroles_upsert_group mutation to use auth_tenant_group_roles_upsert_one_input!.
  • Update userroles_upsert_account_group mutation to use auth_account_group_roles_upsert_one_input! with nested account_roles.
• cmd/auth_roles.go:
  • Update customroles_list query to traverse { roles { id name description } }.
  • Update customroles_create mutation to pass (name, description, permissions) with structured [CustomRolePermissionInput!].
• cmd/auth_test.go:
  • Add unit tests covering auth groups list, auth roles list, auth users list, and auth users get.

Nubi Command Resilience

• pkg/nubi/nubi.go:
  • In ListAgents() and ListTools(), add automatic fallback to tenant-wide discovery (request: {account_id: ""}) if querying with c.AccountID returns access denied (user does not have access).
• cmd/nubi.go, cmd/nubi_agents.go, cmd/nubi_tools.go:
  • Use initNubiClientOptionalAccount so nubi agents and nubi tools don't strictly require an account-id.
• cmd/nubi_query.go:
  • Add --account-id flag via resolveAccountID(cmd) so users can explicitly target or override the account ID.

Verification

• Unit test suite: go test -count=1 ./... passes.
• Live verification against backend for nbctl auth groups list, nbctl auth roles list, nbctl auth users list, nbctl nubi agents, nbctl nubi tools, and nbctl nubi query.

[gemini-code-assist]

Code Review

This pull request updates the CLI to align with backend GraphQL schema changes for role assignments, user groups, and custom roles. It also introduces the ability to initialize the Nubi client with an optional account ID, adds fallback logic for listing agents and tools when access is restricted, and includes a new test suite for authentication commands. The review feedback highlights two key improvement opportunities: optimizing the inefficient double-serialization of group roles in cmd/auth_groups.go using a custom unmarshaler or manual type assertions, and improving input robustness in cmd/auth_roles.go by trimming whitespace and skipping empty permission strings.

[blue4209211]

/gemini review

[blue4209211]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request updates several CLI commands to align with new backend GraphQL schemas, particularly around role assignments, user groups, and custom roles. It also introduces unit tests for auth commands, makes the account ID optional for listing Nubi agents and tools, and implements fallback queries when access is denied. The review feedback highlights a risk in pkg/nubi/nubi.go where reusing the respData variable for fallback queries could lead to stale or corrupted data in the event of partial unmarshaling failures, recommending the use of fresh local variables instead.

[gemini-code-assist]

Code Review

This pull request updates several CLI commands to align with updated GraphQL schemas, introduces custom JSON unmarshaling for group roles, adds unit tests for auth commands, and refactors Nubi client initialization to make the account ID optional for certain commands. Feedback is provided on cmd/auth_roles.go to add omitempty to the Class field of customRolePermissionInput to prevent sending empty strings when only a module is specified.

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates the authentication and Nubi CLI commands to align with updated GraphQL schemas, including changes to role assignments, group roles, and custom role permissions. It also introduces fallback queries for listing agents and tools when account access is restricted, and adds unit tests for the auth commands. The review feedback suggests improving the custom JSON unmarshaler's error handling to avoid misleading errors, and making the fallback error-string checks case-insensitive to ensure reliability.

[blue4209211]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request updates the authentication and role assignment commands to align with new GraphQL schema definitions, including updated mutation inputs, restructured queries, and custom JSON unmarshaling for group roles. It also introduces unit tests for these commands, adds an --account-id flag to nubi query, and implements fallback logic for listing agents and tools when account access is restricted. Feedback is provided regarding cmd/auth_roles.go, where client-side validation should be added to prevent empty permission modules (e.g., when a permission string starts with a colon) from causing backend GraphQL validation errors.

[blue4209211]

/gemini review

[blue4209211]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request updates the authentication and role management commands to align with updated GraphQL schemas, including changes to role assignments, group listing, and custom role creation. It introduces a custom JSON unmarshaler to handle both direct and stringified role arrays, adds comprehensive unit tests, and refactors the Nubi client to support optional account IDs with fallback queries. Feedback is provided regarding input validation when parsing permission strings to prevent sending malformed requests with empty module names to the backend.

[blue4209211]

/gemini review

[blue4209211]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request updates several CLI commands to align with schema changes, improves the parsing of permissions, adds unit tests, and introduces a fallback mechanism for listing agents and tools when account access is restricted. Feedback highlights a potential validation issue with malformed permission strings in cmd/auth_roles.go and suggests passing a context.Context to ListAgents and ListTools in pkg/nubi/nubi.go to support request cancellation.

[blue4209211]

/gemini review

[blue4209211]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request refactors several CLI commands and GraphQL queries related to authentication, user groups, and roles, including adding a custom JSON unmarshaler for group roles and introducing fallback logic for agent and tool listings when account access is restricted. The feedback highlights a logic flaw in the custom UnmarshalJSON error handling where a less specific error is returned on failure. Additionally, it is recommended to address code duplication between ListAgents and ListTools and to propagate context.Context properly instead of using context.Background().

[gemini-code-assist]

Code Review

This pull request updates the authentication commands to use revised GraphQL mutations, introduces a custom JSON unmarshaler for group roles, adds unit tests for auth commands, and implements a context-aware fallback mechanism for listing agents and tools when access is denied. The review feedback suggests improving the robustness of the custom JSON unmarshaler, refactoring the fallback logic to avoid fragile string-based error matching and discarded errors, and adding unit tests to cover the fallback scenarios.

I am having trouble creating individual review comments. Click here to see my feedback.

cmd/auth_groups.go (25-48)

The custom UnmarshalJSON implementation has some areas for improvement for clarity and robustness:

• The check if len(data) == 0 is not a reliable way to handle empty JSON values like null or "".
• The error returned on failure can be misleading. If the data is a string containing invalid JSON, the error from the initial attempt to unmarshal as an array is returned, which hides the real issue.

Consider refactoring for better error handling and clarity.

func (g *groupRolesField) UnmarshalJSON(data []byte) error {
	if string(data) == "null" || string(data) == `""` {
		*g = nil
		return nil
	}

	var items []groupRoleItem
	// First, try to unmarshal as a direct array of objects.
	if err := json.Unmarshal(data, &items); err == nil {
		*g = items
		return nil
	}

	// If that fails, try to unmarshal as a string, which might contain a JSON array.
	var str string
	if errStr := json.Unmarshal(data, &str); errStr != nil {
		// If it's not a direct array and not a string, we can't process it.
		// The original error from the array unmarshal attempt is likely more informative.
		return err
	}

	if str == "" {
		*g = nil
		return nil
	}

	// Now, unmarshal the string content.
	if errArr := json.Unmarshal([]byte(str), &items); errArr != nil {
		return errArr
	}

	*g = items
	return nil
}

pkg/nubi/nubi.go (855-871)

The fallback logic is a great addition for resilience. However, there are a couple of points to consider for making it more robust and maintainable:

1. Fragile Error Matching: Relying on strings.Contains(strings.ToLower(err.Error()), "user does not have access") is brittle. If the backend error message is changed, this logic will break. It would be more robust to check for a specific error code from the GraphQL response if one is available in the extensions field of the error.

2. Discarded Fallback Error: If the fallback request reqFallback also fails, its error errFallback is discarded, and the original error err is returned. This can make debugging difficult as the reason for the fallback failure is hidden.

Consider logging the errFallback or wrapping it with the original error to provide more context on failure.

pkg/nubi/nubi.go (905-921)

Similar to ListAgents, the fallback logic here could be made more robust.

1. Fragile Error Matching: The check strings.Contains(strings.ToLower(err.Error()), "user does not have access") is dependent on a specific error message string, which can be brittle. If possible, matching on a structured error code from the API would be more reliable.

2. Discarded Fallback Error: When the fallback request fails, the error errFallback is ignored. This hides potentially useful information for debugging.

Consider logging the fallback error or returning a wrapped error to improve diagnostics.

pkg/nubi/nubi_test.go (313)

While the existing tests were updated for the new function signatures, the new fallback logic in ListAgents and ListTools is not covered by unit tests. This is a critical piece of new functionality that adds resilience and should be tested.

Please consider adding test cases for the fallback mechanism, including:

• A case where the primary call fails with an "access denied" error and the fallback succeeds.
• A case where both the primary call and the fallback call fail.

[blue4209211]

/gemini review

[blue4209211]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request updates several authentication and AI agent/tool commands to align with updated GraphQL schemas, introduces a fallback mechanism for listing agents and tools when access is denied, and adds comprehensive unit tests. The reviewer identified a potential GraphQL injection risk in pkg/nubi/nubi.go where queryName is directly interpolated into a query string, and suggested validating it against an allowlist.

[blue4209211]

/gemini review

[blue4209211]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request updates authentication and Nubi commands to align with schema changes, including refactoring role assignments, group listings, and custom role creation. It also introduces context propagation to Nubi client calls, makes the account ID optional for listing agents and tools by implementing a fallback mechanism on access-denied errors, and adds comprehensive unit tests. Feedback is provided to improve the robustness of the custom JSON unmarshaling logic for group roles by trimming whitespace before performing string comparisons.

[gemini-code-assist]

Code Review

This pull request refactors the authentication and Nubi client commands. It updates several GraphQL mutations to use individual arguments instead of a single request object, introduces custom JSON unmarshaling for group roles, and implements an account fallback mechanism in the Nubi client when listing agents or tools fails due to access denial. Additionally, it adds comprehensive unit tests for these changes. The feedback suggests avoiding multiple %w verbs in fmt.Errorf to ensure compatibility with older Go versions and simplify error wrapping.

[blue4209211]

/gemini review

[blue4209211]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request updates several CLI commands and client queries to align with GraphQL schema changes, improves role assignment and custom role creation logic, and introduces a fallback mechanism in the Nubi client to query agents and tools without an account ID if access is denied. It also adds comprehensive unit tests for these changes. The feedback highlights a redundancy in the isAccessDeniedError helper function, where a fall-through string check on the error message is unnecessarily executed even when the error has already been fully processed as a structured client.GraphQLErrors type.

[blue4209211]

/gemini review

[blue4209211]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request updates several authentication and AI agent commands. It refactors GraphQL mutations for role assignments, user groups, and custom roles to use updated input types and structures, and introduces a custom JSON unmarshaler for group roles. Additionally, the Nubi client's agent and tool listing functions are updated to accept a context and implement an automatic fallback mechanism to query with an empty account ID upon encountering access-denied errors. Comprehensive unit tests are also added. The feedback suggests explicitly passing nil for the description variable when creating user groups if the description is empty, ensuring consistency with other commands and compatibility with strict GraphQL parsers.

[gemini-code-assist]

Code Review

This pull request updates several CLI commands to align with GraphQL schema changes, adds a fallback mechanism for listing agents and tools when access is denied, and introduces comprehensive unit tests. The review feedback suggests optimizing string allocations during JSON unmarshaling in cmd/auth_groups.go and omitting the description variable when empty in cmd/auth_roles.go for consistency and cleaner payloads.

[blue4209211]

/gemini review

[blue4209211]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request refactors authentication and Nubi client commands. Key changes include updating GraphQL mutations to use individual variables, implementing a custom JSON unmarshaler for group roles to handle stringified arrays, passing contexts to Nubi client calls, and adding an account-fallback mechanism when listing agents or tools. Comprehensive unit tests have also been added. The feedback suggests explicitly passing nil for the description variable when creating a user group if the description is empty, ensuring consistency with custom roles creation and preventing potential issues with strict GraphQL servers.

[blue4209211]

/gemini review

[blue4209211]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request refactors several CLI commands and client methods. Key changes include updating GraphQL mutations and queries for role assignments, user groups, and custom roles to use new input structures and fields. It introduces custom JSON unmarshaling for group roles to handle both structured and stringified arrays, adds context support and fallback logic for Nubi client queries when access is denied, and introduces comprehensive unit tests for these features. I have no feedback to provide as there are no review comments.

[gemini-code-assist]

Code Review

This pull request updates several CLI commands and GraphQL queries to align with schema changes. Specifically, it refactors role assignment, user group listing, and custom role creation to use updated GraphQL inputs and nested fields. It also introduces a custom JSON unmarshaler for group roles to handle both direct and stringified JSON arrays. Additionally, the Nubi client is updated to support context propagation, optional account requirements, and a fallback mechanism that queries with an empty account ID upon encountering access-denied errors. Comprehensive unit tests have been added to verify these changes. I have no feedback to provide.

[gemini-code-assist]

Code Review

This pull request refactors several CLI commands and GraphQL queries. Key changes include updating role assignment and group creation mutations to use flatter input structures, introducing a custom JSON unmarshaling type (groupRolesField) to handle both raw and stringified JSON arrays, and implementing an automatic fallback mechanism in the Nubi client to retry queries with an empty account ID upon encountering access-denied errors. Additionally, context propagation has been improved across client calls, and comprehensive unit tests have been added. I have no feedback to provide as there are no review comments.

[gemini-code-assist]

Code Review

This pull request refactors several authentication and Nubi-related commands. Key changes include updating GraphQL mutations and variable structures for role assignments, user groups, and custom roles, implementing custom JSON unmarshaling for group roles, and introducing a fallback mechanism in the Nubi client to query without an account ID when access is denied. Additionally, comprehensive unit tests are added. Feedback is provided to reduce redundancy in the isAccessDeniedError helper function by extracting the repeated string checks into a local helper.

[gemini-code-assist]

Code Review

This pull request refactors and updates several authentication and Nubi-related CLI commands. Key changes include updating GraphQL mutations and queries for role assignments, user groups, and custom roles, introducing a custom JSON unmarshaler for group roles, and implementing an account-fallback mechanism for listing Nubi agents and tools when access is denied. Additionally, comprehensive unit tests have been added to verify these changes. There are no review comments, so we have no further feedback to provide.

[gemini-code-assist]

Code Review

This pull request updates authentication commands, refactors role and group queries to match schema changes, and implements a fallback mechanism in the Nubi client to list agents and tools without an account ID if access is denied. Additionally, it introduces a custom JSON unmarshaler for group roles and adds comprehensive unit tests. The review feedback suggests de-duplicating roles in the group list output and de-duplicating permissions on the client side during custom role creation to prevent redundant payloads.

[gemini-code-assist]

Code Review

This pull request refactors several CLI commands and GraphQL queries related to authentication, user groups, roles, and Nubi AI client operations. Key changes include updating GraphQL mutations to use individual variables instead of input objects, introducing a custom JSON unmarshaler (groupRolesField) to handle both direct and stringified arrays, adding fallback mechanisms for ListAgents and ListTools when access is denied, and adding comprehensive unit tests. I have no feedback to provide.