Skip to content

fix: migrate to v2/v3 GraphQL actions after direct-table removal#73

Merged
blue4209211 merged 1 commit into
mainfrom
claude/upbeat-hawking-tcGVS
May 23, 2026
Merged

fix: migrate to v2/v3 GraphQL actions after direct-table removal#73
blue4209211 merged 1 commit into
mainfrom
claude/upbeat-hawking-tcGVS

Conversation

@blue4209211

Copy link
Copy Markdown
Contributor

Summary

The server's actions.yaml removed almost all direct Hasura table APIs in favor of v2/v3 actions. This PR migrates nbctl off the removed APIs and then corrects the action shapes against the actual app code at nudgebee/nudgebee-enterprise/app.

Commit 1 — initial migration (f473dba)

Switched all direct-table calls to v2/v3 action equivalents:

  • cloud_accounts*get_cloud_accounts_v2
  • usersadmin_get_users_by_tenant_v2
  • eventsevents_v2
  • traces_by_pk / traces_v3traces_query
  • llm_conversations* / llm_functionsllm_conversation_list_v2 / ai_get_conversation_v3 / llm_functions_v2
  • update_cloud_accountscloud_account_update
  • delete_llm_conversation_saveds_by_pkai_delete_saved_conversation

Commit 2 — schema corrections (9ef3a1a)

After cross-checking with the web app's actual gql calls, fixed several guess-driven mistakes:

  • ai_get_conversation_v3: id/session_id/status live inside conversation { ... }, not top-level
  • ai_delete_saved_conversation: input is {conversation_id} only
  • cloud_account_update: variable is $object (not $input); response is {affected_rows} (not {id,status})
  • events_v2: fields are account_id/resource_id (not cloud_account_id/cloud_resource_id)
  • llm_conversation_list_v2: bookmark filter is is_saved (not is_bookmarked); username filter is user_username; order_by is array with enum
  • Dropped updated_at from get_cloud_accounts_v2 (not in schema)

Also restored columns that didn't actually need separate calls:

  • accounts list: agents/cloud_account_attrs jsonb on the row → Agents/Attributes count columns back
  • admin users list/get: user_roles/user_groups/last_accessed_at on the row → Roles/Groups/Last Login columns back

Test plan

  • go build ./... clean
  • go test ./... passes (unit tests for accounts, admin users, events, traces, nubi all updated to new shapes)
  • Manual smoke (recommended order — exercises riskiest changes first):
    • nbctl accounts listnbctl accounts get <id> (cloud + k8s)
    • nbctl events get <id>
    • nbctl traces querynbctl traces get <trace_id>
    • nbctl admin users listnbctl admin users get --username <u>
    • nbctl accounts enable <id> / disable <id>
    • nbctl nubi — session, history, bookmarks
  • Run with --verbose to capture GraphQL request/response in nbctl_graphql.log if anything fails

🤖 Generated with Claude Code

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates multiple CLI commands to use a new set of versioned GraphQL APIs (v2/v3), which involve changes to query structures, response handling, and the introduction of new data fields. The updates affect account management, user administration, event retrieval, and trace querying. Key feedback includes a high-severity security concern regarding GraphQL injection in cmd/traces_get.go due to string interpolation of user input, and a usability regression in cmd/admin_users_get.go where user roles and groups are now printed as raw JSON instead of formatted strings.

Comment thread cmd/traces_get.go Outdated
Comment thread cmd/admin_users_get.go Outdated
The server's actions.yaml removed almost all direct-table Hasura APIs
in favor of v2/v3 actions. This migrates nbctl off the removed APIs,
corrects the action shapes against the actual web-app gql calls, and
addresses fallout discovered during live smoke testing against the
dev cluster.

### Direct-table → v2/v3 migrations
- cloud_accounts / cloud_accounts_by_pk → get_cloud_accounts_v2
- users / users_aggregate → admin_get_users_by_tenant_v2
- events → events_v2
- traces_by_pk / traces_v3 → traces_query
- llm_conversations / llm_conversation_messages → llm_conversation_list_v2 + ai_get_conversation_v3
- llm_functions → llm_functions_v2
- delete_llm_conversation_saveds_by_pk → ai_delete_saved_conversation
- update_cloud_accounts → cloud_account_update

### Schema corrections (verified against /Users/shiv/Workspace/nudgebee/app)
- ai_get_conversation_v3: id/session_id/status live inside `conversation { ... }`
- llm_conversation_list_v2: bookmark filter is `is_saved` (not is_bookmarked);
  username filter is `user_username`; order_by is array with enum
- ai_delete_saved_conversation: input is `{conversation_id}` only
- cloud_account_update: variable is `$object`; response is `{affected_rows}`
- events_v2: fields are `account_id`/`resource_id`
- get_agent_health_v2: where key is `cloud_account_id`, not account_id
- accounts_get: drop non-existent updated_at column
- ai_*: drop unused `user_id` to match app exactly
- logs/metrics list-* actions: use `query`, not `mutation`

### jsonb double-encoding
agents, cloud_account_attrs, user_roles, user_groups, and connection_status
come back wire-encoded as JSON strings, not nested objects. Helpers and
the connection-status decoder now peel the outer string layer first.
This restores Agents/Attributes/Roles/Groups columns and unbreaks
`accounts get` for k8s/cloud.

### New command
`nbctl integrations list` — backed by admin_get_integrations_v2,
filterable by status/type/name.

### Other
- Replace WriteString(fmt.Sprintf(...)) with fmt.Fprintf (lint).
- traces_get: use $trace_id GraphQL variable instead of fmt.Sprintf
  injection (Gemini PR review).
- admin_users_get: format user_roles/user_groups via joinJSONNames
  instead of dumping raw json arrays (Gemini PR review).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@blue4209211 blue4209211 force-pushed the claude/upbeat-hawking-tcGVS branch from a37f3eb to a0d07b6 Compare May 23, 2026 08:47
@blue4209211 blue4209211 merged commit 57c4503 into main May 23, 2026
1 check passed
blue4209211 added a commit that referenced this pull request May 23, 2026
- traces_query: replace fmt.Sprintf string interpolation with
  $request: TracesV3Input! variable. Same GraphQL-injection class as
  the Gemini comment on traces_get.go fixed in PR #73; this finishes
  the job for the query command.
- events_get: peel double-encoded JSON-string layer for Evidences and
  Labels (same pattern as connection_status in accounts_get). Text
  output stays readable, JSON output now returns real objects/arrays
  instead of escaped strings.
- logs_list_log_groups: wrap namespace flag in regexp.QuoteMeta before
  interpolating into the PromQL container_id regex. Prevents a flag
  like "prod|.*" from silently matching everything.
- Add unit tests for countJSONArray, joinJSONNames, and the new
  peelJSONString helper covering both native and double-encoded inputs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
blue4209211 added a commit that referenced this pull request May 24, 2026
* fix(cli): address obvious follow-up issues from PR #73

- traces_query: replace fmt.Sprintf string interpolation with
  $request: TracesV3Input! variable. Same GraphQL-injection class as
  the Gemini comment on traces_get.go fixed in PR #73; this finishes
  the job for the query command.
- events_get: peel double-encoded JSON-string layer for Evidences and
  Labels (same pattern as connection_status in accounts_get). Text
  output stays readable, JSON output now returns real objects/arrays
  instead of escaped strings.
- logs_list_log_groups: wrap namespace flag in regexp.QuoteMeta before
  interpolating into the PromQL container_id regex. Prevents a flag
  like "prod|.*" from silently matching everything.
- Add unit tests for countJSONArray, joinJSONNames, and the new
  peelJSONString helper covering both native and double-encoded inputs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: logs query response shape + restrict config file perms

logs_query:
- Response key was `logs_list` but the v2/v3 action returns `logs_query`,
  so rows were silently dropped — empty output looked like the command
  was broken. Renamed the unmarshal target.
- Print "No logs found." on empty/null instead of nothing, so users
  can tell the call succeeded with no results.
- Drop the synthesized inner `query=...&start=...&end=...&limit=...`
  string. Those fields are already passed as top-level request fields;
  duplicating them inside the `query` value just confused the upstream
  provider and produced `null` for empty user queries.

configure:
- `~/.nudgebee/config.yaml` stores plaintext API keys and was being
  written as 0644 (world-readable). Explicitly chmod to 0600 after
  both `add` and `set-current` paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
blue4209211 added a commit that referenced this pull request May 24, 2026
README:
- Replace nonexistent `nbctl traces list` section with the real
  `nbctl traces query` command, including all flags.
- Add `nbctl integrations` section (added in PR #73 but undocumented).
- Add Contributing / Security / License sections pointing at the
  new top-level files.

OSS hygiene:
- LICENSE — Apache 2.0 (matches the rest of the Nudgebee project).
- CODE_OF_CONDUCT.md — Contributor Covenant 2.x.
- CONTRIBUTING.md — adapted from the monorepo template for this
  single-Go-module repo (dev setup, Makefile targets, commit
  conventions, PR guidelines).
- SECURITY.md — vulnerability reporting policy and response timeline.
- .github/pull_request_template.md — standard template.
- .github/ISSUE_TEMPLATE/{BUG-REPORT,FEATURE-REQUEST,config}.yml —
  CLI-flavored (asks for `nbctl version`, install method, OS/arch
  instead of web-only fields).
- .github/dependabot.yml — weekly gomod + github-actions updates.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
blue4209211 added a commit that referenced this pull request May 24, 2026
README:
- Replace nonexistent `nbctl traces list` section with the real
  `nbctl traces query` command, including all flags.
- Add `nbctl integrations` section (added in PR #73 but undocumented).
- Add Contributing / Security / License sections at the bottom
  pointing at the new top-level files.

OSS hygiene:
- LICENSE — Apache 2.0 (matches the rest of the Nudgebee project).
- CODE_OF_CONDUCT.md — Contributor Covenant 2.x.
- CONTRIBUTING.md — adapted from the monorepo template for this
  single-Go-module repo (dev setup, Makefile targets, commit
  conventions, PR guidelines).
- SECURITY.md — vulnerability reporting policy and response timeline.
- .github/pull_request_template.md — standard template.
- .github/ISSUE_TEMPLATE/{BUG-REPORT,FEATURE-REQUEST,config}.yml —
  CLI-flavored (asks for `nbctl version`, install method, OS/arch
  instead of web-only fields).
- .github/dependabot.yml — weekly gomod + github-actions updates.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
blue4209211 added a commit that referenced this pull request May 25, 2026
README:
- Replace nonexistent `nbctl traces list` section with the real
  `nbctl traces query` command, including all flags.
- Add `nbctl integrations` section (added in PR #73 but undocumented).
- Add Contributing / Security / License sections at the bottom
  pointing at the new top-level files.

OSS hygiene:
- LICENSE — Apache 2.0 (matches the rest of the Nudgebee project).
- CODE_OF_CONDUCT.md — Contributor Covenant 2.x.
- CONTRIBUTING.md — adapted from the monorepo template for this
  single-Go-module repo (dev setup, Makefile targets, commit
  conventions, PR guidelines).
- SECURITY.md — vulnerability reporting policy and response timeline.
- .github/pull_request_template.md — standard template.
- .github/ISSUE_TEMPLATE/{BUG-REPORT,FEATURE-REQUEST,config}.yml —
  CLI-flavored (asks for `nbctl version`, install method, OS/arch
  instead of web-only fields).
- .github/dependabot.yml — weekly gomod + github-actions updates.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants