feat(platform): Simplify Credentials UX #8524

kcze · 2024-11-01T16:30:09Z

Background

Current credentials system always forces users to choose correct credentials, even when they are provided by the cloud platform and should be chosen automatically (e.g. there's only one possible choice).

Changes 🏗️

Change provider of default credentials to actual provider names (e.g. anthropic) and not llm
Add discriminator and discriminator_mapping to CredentialsField that allows to filter credentials input to only allow providers for matching models in useCredentials hook (thanks @ntindle for the idea!); e.g. user chooses GPT4_TURBO so then only OpenAI credentials are allowed
Choose credentials automatically and hide credentials input on the node completely if there's only one possible option
Move getValue and parseKeys to utils
Add ANTHROPIC, GROQ and OLLAMA to providers in frontend types.ts
Add hidden field to credentials that is used for default system keys to hide them in user profile

Testing 🔍

Note

Only for the new autogpt platform, currently in autogpt_platform/

Create from scratch and execute an agent with at least 3 blocks
Import an agent from file upload, and confirm it executes correctly
Upload agent to marketplace
Import an agent from marketplace and confirm it executes correctly
Edit an agent from monitor, and confirm it executes correctly

…improvements

ntindle · 2024-11-04T22:15:04Z

autogpt_platform/frontend/src/lib/utils.ts

+export function parseKeys(key: string): ParsedKey[] {
+  const splits = key.split(/_@_|_#_|_\$_|\./);
+  const keys: ParsedKey[] = [];
+  let currentKey: string | null = null;
+
+  splits.forEach((split) => {
+    const isInteger = /^\d+$/.test(split);
+    if (!isInteger) {
+      if (currentKey !== null) {
+        keys.push({ key: currentKey });
+      }
+      currentKey = split;
+    } else {
+      if (currentKey !== null) {
+        keys.push({ key: currentKey, index: parseInt(split, 10) });
+        currentKey = null;
+      } else {
+        throw new Error("Invalid key format: array index without a key");
+      }
+    }
+  });
+
+  if (currentKey !== null) {
+    keys.push({ key: currentKey });
+  }
+
+  return keys;
+}


we should probably add security test for this lol

I agree but this code is already in the codebase I just moved it!

ntindle · 2024-11-04T22:15:25Z

This is exactly what I was thinking! great job!

ntindle · 2024-11-04T22:31:52Z

autogpt_platform/frontend/src/lib/autogpt-server-api/types.ts

  D_ID: "d_id",
  DISCORD: "discord",
  GITHUB: "github",
  GOOGLE: "google",
  GOOGLE_MAPS: "google_maps",
+  GROQ: "groq",


probably need logos for these too

(even if fallback)

…improvements

ntindle · 2024-11-05T03:05:03Z

I think the devx and ux of this leaves something to be desired still. I think the dev specifying the valid providers as a list would be useful and then it returns one of the credentials depending on the selected API key added.

Screen.Recording.2024-11-04.at.9.04.20.PM.mov

Also not sure how they are added to the db and filtered but I'm not seeing them after creation, and crashing anytime they're rendered. I think there's an issue somewhere. lmk if you can't replicate

ntindle

Tested, needs addtional work

…improvements

kcze · 2024-11-05T14:21:40Z

I think the dev specifying the valid providers as a list would be useful and then it returns one of the credentials depending on the selected API key added.

Constraining providers already happens in the backend by specifying discriminator_mapping. You cannot add non-matching provider and even if you manage they won't be displayed in the dropdown.

Regarding bugs and video: everything should now work, including system keys being hidden in the profile.

…improvements

ntindle

This still isn't wokring for me

There's nowhere to select credentials, and we're still creating llm providers which we shouldn't need

ntindle · 2024-11-05T18:10:08Z

autogpt_platform/backend/backend/blocks/llm.py

@@ -52,6 +52,10 @@ def AICredentialsField() -> AICredentials:
        description="API key for the LLM provider.",
        provider="llm",


imo this needs to be a structure that shows the allowable providers, not its own provider. something like

valid_providers = ['openai', 'grok', 'anthropic']
and be used in conjunction with discriminator always to have one of the providers be selected

ntindle · 2024-11-05T18:10:37Z

autogpt_platform/frontend/src/app/profile/page.tsx

+    [...provider.savedOAuthCredentials, ...provider.savedApiKeys]
+      .filter((cred) => !cred.hidden)
+      .map((credentials) => ({
        ...credentials,
        provider: provider.provider,
        providerName: provider.providerName,
        ProviderIcon: providerIcons[provider.provider],
        TypeIcon: { oauth2: IconUser, api_key: IconKey }[credentials.type],
-      }),
-    ),
+      })),


this works fine for me right now

ntindle · 2024-11-05T18:11:06Z

autogpt_platform/frontend/src/components/integrations/credentials-input.tsx

@@ -56,6 +58,7 @@ export const providerIcons: Record<
  ideogram: fallbackIcon,
  llm: fallbackIcon,


we should be able to remove this once we get this right

ntindle · 2024-11-05T18:11:25Z

autogpt_platform/frontend/src/components/integrations/credentials-input.tsx

-  if (!credentials) {
+  if (!credentials || credentials.isLoading) {
    return null;
  }

-  if (credentials.isLoading) {
-    return <div>Loading...</div>;
-  }
-


why this change?

ntindle · 2024-11-05T18:13:05Z

autogpt_platform/frontend/src/components/integrations/credentials-input.tsx

+    // Only one saved credential
+  } else if (
+    savedApiKeys.length === 0 &&
+    savedOAuthCredentials.length === 1 &&
+    selectedCredentials === undefined
+  ) {
+    if (selectedCredentials === undefined) {
+      onSelectCredentials({
+        id: savedOAuthCredentials[0].id,
+        type: "oauth2",
+        provider,
+        title: savedOAuthCredentials[0].title,
+      });
+    }
+    return null;
+  } else if (savedApiKeys.length === 1 && savedOAuthCredentials.length === 0) {
+    if (selectedCredentials === undefined) {
+      onSelectCredentials({
+        id: savedApiKeys[0].id,
+        type: "api_key",
+        provider,
+        title: savedApiKeys[0].title,
+      });
+    }
+    return null;


What's the purpose of this? This whole chain of logic seems questionably necessary.

ntindle · 2024-11-05T18:15:24Z

app-index.tsx:25 
 Warning: Cannot update a component (`BatchProvider`) while rendering a different component (`CredentialsInput`). To locate the bad setState() call inside `CredentialsInput`, follow the stack trace as described in https://reactjs.org/link/setstate-in-render Error Component Stack
    at CredentialsInput (credentials-input.tsx:87:9)
    at div (<anonymous>)
    at NodeCredentialsInput (node-input-components.tsx:309:9)
    at NodeGenericInputField (

maybe this error helps

Update Credentials UX

0842f20

kcze requested review from ntindle, Pwuts, aarushik93 and majdyz November 1, 2024 16:30

kcze requested a review from a team as a code owner November 1, 2024 16:30

github-actions bot added platform/frontend AutoGPT Platform - Front end platform/backend AutoGPT Platform - Back end platform/blocks size/l labels Nov 1, 2024

kcze and others added 3 commits November 2, 2024 01:39

Format backend

30fbba3

Fix discriminatorValue

800b8e3

Merge branch 'dev' into kpczerwinski/open-1999-credentials-system-ux-…

8fb652d

…improvements

ntindle reviewed Nov 4, 2024

View reviewed changes

Merge branch 'dev' into kpczerwinski/open-1999-credentials-system-ux-…

33ba822

…improvements

ntindle requested changes Nov 5, 2024

View reviewed changes

kcze added 4 commits November 5, 2024 21:44

Merge branch 'dev' into kpczerwinski/open-1999-credentials-system-ux-…

3f54524

…improvements

Hide system credentials in Profile

4d09e53

Deselect non existing credentials

f64a0b4

Lint

f9a3993

kcze requested a review from ntindle November 5, 2024 14:21

Merge branch 'dev' into kpczerwinski/open-1999-credentials-system-ux-…

e143754

…improvements

ntindle requested changes Nov 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(platform): Simplify Credentials UX #8524

feat(platform): Simplify Credentials UX #8524

kcze commented Nov 1, 2024 •

edited

Loading

ntindle Nov 4, 2024

kcze Nov 5, 2024

ntindle commented Nov 4, 2024

ntindle Nov 4, 2024

ntindle Nov 4, 2024

kcze Nov 5, 2024

ntindle commented Nov 5, 2024

ntindle left a comment

kcze commented Nov 5, 2024

ntindle left a comment

ntindle Nov 5, 2024

ntindle Nov 5, 2024

ntindle Nov 5, 2024

ntindle Nov 5, 2024

ntindle Nov 5, 2024

ntindle commented Nov 5, 2024

		@@ -52,6 +52,10 @@ def AICredentialsField() -> AICredentials:
		description="API key for the LLM provider.",
		provider="llm",

		@@ -56,6 +58,7 @@ export const providerIcons: Record<
		ideogram: fallbackIcon,
		llm: fallbackIcon,

feat(platform): Simplify Credentials UX #8524

Are you sure you want to change the base?

feat(platform): Simplify Credentials UX #8524

Conversation

kcze commented Nov 1, 2024 • edited Loading

Background

Changes 🏗️

Testing 🔍

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntindle commented Nov 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntindle commented Nov 5, 2024

ntindle left a comment

Choose a reason for hiding this comment

kcze commented Nov 5, 2024

ntindle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntindle commented Nov 5, 2024

kcze commented Nov 1, 2024 •

edited

Loading