Skip to content

fix: submit Discord Klavis auth as bot token#786

Open
Felarof (felarof99) wants to merge 1 commit intomainfrom
fix/discord-bot-token-auth
Open

fix: submit Discord Klavis auth as bot token#786
Felarof (felarof99) wants to merge 1 commit intomainfrom
fix/discord-bot-token-auth

Conversation

@felarof99
Copy link
Copy Markdown
Contributor

Summary

  • Fix Discord Klavis API-key submission to send the documented bot token auth payload, authData.data.bot_token, instead of generic authData.api_key.
  • Preserve generic api_key auth data for non-Discord integrations and when the server auth-field override flag is disabled.
  • Keep the existing Klavis integration normalization expectation green by treating string integrations from Klavis as authenticated.

Root Cause

BrowserOS already receives serverName when the Connect Apps UI submits an API key, but the server discarded that context. KlavisClient.submitApiKey always sent { api_key: ... }, while Klavis Discord Strata docs require the Discord bot token under data.bot_token.

Feature Flag

The server-side mapping is protected by klavisAuthFieldOverridesEnabled, defaulting to enabled. It can be disabled with BROWSEROS_KLAVIS_AUTH_FIELD_OVERRIDES=false or config flags.klavis_auth_field_overrides: false.

Test Plan

  • bun --env-file=.env.development test apps/server/tests/api/routes/klavis.test.ts apps/server/tests/config.test.ts
  • bunx biome check apps/server/src/lib/clients/klavis/klavis-client.ts apps/server/src/api/routes/klavis.ts apps/server/src/config.ts apps/server/src/api/types.ts apps/server/src/api/server.ts apps/server/src/main.ts apps/server/tests/api/routes/klavis.test.ts apps/server/tests/config.test.ts
  • bun run --filter @browseros/server typecheck

Reference

Map Discord API-key submissions to Klavis's documented bot_token auth data while preserving generic api_key auth for other integrations. Add a server feature flag for auth field overrides and keep the existing Klavis integration normalization test green.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@github-actions github-actions Bot added the fix label Apr 22, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 22, 2026

Greptile Summary

This PR fixes Discord bot-token auth submission by mapping the discord server name to a bot_token field (wrapped in authData.data), while keeping the flat api_key structure for all other Klavis integrations. The override mapping is guarded by a klavisAuthFieldOverridesEnabled feature flag (default: on) threaded cleanly through config, env var, server config, and route deps — with solid test coverage for all three scenarios (Discord, non-Discord, and flag disabled).

  • The change to normalizeIntegration that flips string-typed entries from isAuthenticated: false to true is a silent behavioral change with no test coverage. If Klavis uses strings for unauthenticated integrations, this will incorrectly show them as connected in the UI.

Confidence Score: 4/5

Safe to merge after confirming the string-integration isAuthenticated behavior is intentional and consistent with the Klavis API contract.

The core fix (Discord bot_token payload) is correct, well-tested, and cleanly implemented. One P1 concern exists: the undocumented change to treat string-typed Klavis integrations as authenticated could hide auth prompts from users if the Klavis API ever returns strings for unauthenticated integrations.

packages/browseros-agent/apps/server/src/lib/clients/klavis/klavis-client.ts — specifically the normalizeIntegration change at line 149-150.

Important Files Changed

Filename Overview
packages/browseros-agent/apps/server/src/lib/clients/klavis/klavis-client.ts Core logic change: adds server-specific auth field mapping (discord → bot_token), updates submitApiKey to accept options; also silently changes normalizeIntegration for string-type integrations from isAuthenticated:false to true
packages/browseros-agent/apps/server/src/api/routes/klavis.ts Routes updated to forward serverName and the feature-flag toggle to submitApiKey; clean propagation with sensible default (overrides enabled)
packages/browseros-agent/apps/server/src/config.ts Adds klavisAuthFieldOverridesEnabled to schema/defaults/env/file config; parseBooleanEnv helper handles true/false/undefined cleanly; default is true (overrides on)
packages/browseros-agent/apps/server/src/api/types.ts Adds optional klavisAuthFieldOverridesEnabled to HttpServerConfig; straightforward interface extension
packages/browseros-agent/apps/server/src/api/server.ts Threads klavisAuthFieldOverridesEnabled from config into createKlavisRoutes; no issues
packages/browseros-agent/apps/server/src/main.ts Passes klavisAuthFieldOverridesEnabled from application config to HTTP server config; clean propagation
packages/browseros-agent/apps/server/tests/api/routes/klavis.test.ts Adds three well-structured integration tests covering Discord bot_token payload, non-Discord api_key payload, and disabled-flag fallback; good coverage of the new behavior
packages/browseros-agent/apps/server/tests/config.test.ts Adds tests for env var override, config file flag, and default (true) for klavisAuthFieldOverridesEnabled; comprehensive

Sequence Diagram

sequenceDiagram
    participant UI as Connect Apps UI
    participant Route as submit-api-key route
    participant Client as KlavisClient
    participant Klavis as Klavis API

    UI->>Route: POST serverName + apiKey + apiKeyUrl
    Route->>Route: resolve klavisAuthFieldOverridesEnabled
    Route->>Client: submitApiKey(url, key, options)
    Client->>Client: getAuthFieldName(serverName, useOverrides)
    Client->>Client: buildAuthData(fieldName, key)
    note over Client: discord → authData.data.bot_token<br/>others → authData.api_key
    Client->>Klavis: POST set-auth with instanceId + authData
    Klavis-->>Client: 200 OK
    Client-->>Route: resolved
    Route-->>UI: success + serverName
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/server/src/lib/clients/klavis/klavis-client.ts
Line: 149-150

Comment:
**Behavioral change in string integration auth status**

`normalizeIntegration` now returns `isAuthenticated: true` for string-typed entries from Klavis, whereas it previously returned `false`. If Klavis uses the plain-string format to denote integrations that are *known* but not yet authenticated (as the old default implied), this change would incorrectly show them as connected in the UI — hiding the prompt to authenticate. Can you confirm that Klavis only returns strings for integrations whose auth is already established?

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix: submit Discord Klavis auth as bot t..." | Re-trigger Greptile

Comment on lines 149 to +150
if (typeof integration === 'string') {
return { name: integration, isAuthenticated: false }
return { name: integration, isAuthenticated: true }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Behavioral change in string integration auth status

normalizeIntegration now returns isAuthenticated: true for string-typed entries from Klavis, whereas it previously returned false. If Klavis uses the plain-string format to denote integrations that are known but not yet authenticated (as the old default implied), this change would incorrectly show them as connected in the UI — hiding the prompt to authenticate. Can you confirm that Klavis only returns strings for integrations whose auth is already established? Does the Klavis API return string-typed integrations only when the integration is already authenticated, or also for unauthenticated/available integrations?

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/server/src/lib/clients/klavis/klavis-client.ts
Line: 149-150

Comment:
**Behavioral change in string integration auth status**

`normalizeIntegration` now returns `isAuthenticated: true` for string-typed entries from Klavis, whereas it previously returned `false`. If Klavis uses the plain-string format to denote integrations that are *known* but not yet authenticated (as the old default implied), this change would incorrectly show them as connected in the UI — hiding the prompt to authenticate. Can you confirm that Klavis only returns strings for integrations whose auth is already established? Does the Klavis API return string-typed integrations only when the integration is already authenticated, or also for unauthenticated/available integrations?

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant