Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(platform): Simplify Credentials UX #8524

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,21 @@
)
openai_credentials = APIKeyCredentials(
id="53c25cb8-e3ee-465c-a4d1-e75a4c899c2a",
provider="llm",
provider="openai",
api_key=SecretStr(settings.secrets.openai_api_key),
title="Use Credits for OpenAI",
expires_at=None,
)
anthropic_credentials = APIKeyCredentials(
id="24e5d942-d9e3-4798-8151-90143ee55629",
provider="llm",
provider="anthropic",
api_key=SecretStr(settings.secrets.anthropic_api_key),
title="Use Credits for Anthropic",
expires_at=None,
)
groq_credentials = APIKeyCredentials(
id="4ec22295-8f97-4dd1-b42b-2c6957a02545",
provider="llm",
provider="groq",
api_key=SecretStr(settings.secrets.groq_api_key),
title="Use Credits for Groq",
expires_at=None,
Expand Down
4 changes: 4 additions & 0 deletions autogpt_platform/backend/backend/blocks/llm.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ def AICredentialsField() -> AICredentials:
description="API key for the LLM provider.",
provider="llm",
Copy link
Member

Choose a reason for hiding this comment

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

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

supported_credential_types={"api_key"},
discriminator="model",
discriminator_mapping={
model.value: model.metadata.provider for model in LlmModel
},
)


Expand Down
4 changes: 4 additions & 0 deletions autogpt_platform/backend/backend/data/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ def CredentialsField(
supported_credential_types: set[CT],
required_scopes: set[str] = set(),
*,
discriminator: Optional[str] = None,
discriminator_mapping: Optional[dict[str, Any]] = None,
title: Optional[str] = None,
description: Optional[str] = None,
**kwargs,
Expand All @@ -170,6 +172,8 @@ def CredentialsField(
"credentials_provider": provider,
"credentials_scopes": list(required_scopes) or None, # omit if empty
"credentials_types": list(supported_credential_types),
"discriminator": discriminator,
"discriminator_mapping": discriminator_mapping,
}.items()
if v is not None
}
Expand Down
68 changes: 13 additions & 55 deletions autogpt_platform/frontend/src/components/CustomNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ import {
BlockUIType,
BlockCost,
} from "@/lib/autogpt-server-api/types";
import { beautifyString, cn, setNestedProperty } from "@/lib/utils";
import {
beautifyString,
cn,
getValue,
parseKeys,
setNestedProperty,
} from "@/lib/utils";
import { Button } from "@/components/ui/button";
import { Switch } from "@/components/ui/switch";
import { history } from "./history";
Expand All @@ -36,8 +42,6 @@ import * as Separator from "@radix-ui/react-separator";
import * as ContextMenu from "@radix-ui/react-context-menu";
import { DotsVerticalIcon, TrashIcon, CopyIcon } from "@radix-ui/react-icons";

type ParsedKey = { key: string; index?: number };

export type ConnectionData = Array<{
edge_id: string;
source: string;
Expand Down Expand Up @@ -181,7 +185,7 @@ export function CustomNode({
nodeId={id}
propKey={propKey}
propSchema={propSchema}
currentValue={getValue(propKey)}
currentValue={getValue(propKey, data.hardcodedValues)}
connections={data.connections}
handleInputChange={handleInputChange}
handleInputClick={handleInputClick}
Expand All @@ -204,7 +208,7 @@ export function CustomNode({
className=""
selfKey={noteKey}
schema={noteSchema as BlockIOStringSubSchema}
value={getValue(noteKey)}
value={getValue(noteKey, data.hardcodedValues)}
handleInputChange={handleInputChange}
handleInputClick={handleInputClick}
error={data.errors?.[noteKey] ?? ""}
Expand Down Expand Up @@ -240,7 +244,7 @@ export function CustomNode({
nodeId={id}
propKey={propKey}
propSchema={propSchema}
currentValue={getValue(propKey)}
currentValue={getValue(propKey, data.hardcodedValues)}
connections={data.connections}
handleInputChange={handleInputChange}
handleInputClick={handleInputClick}
Expand All @@ -261,11 +265,7 @@ export function CustomNode({
return (
(isRequired || isAdvancedOpen || isConnected || !isAdvanced) && (
<div key={propKey} data-id={`input-handle-${propKey}`}>
{"credentials_provider" in propSchema ? (
<span className="text-m green mb-0 text-gray-900">
Credentials
</span>
) : (
{!("credentials_provider" in propSchema) && (
<NodeHandle
keyName={propKey}
isConnected={isConnected}
Expand All @@ -279,7 +279,7 @@ export function CustomNode({
nodeId={id}
propKey={propKey}
propSchema={propSchema}
currentValue={getValue(propKey)}
currentValue={getValue(propKey, data.hardcodedValues)}
connections={data.connections}
handleInputChange={handleInputChange}
handleInputClick={handleInputClick}
Expand Down Expand Up @@ -336,48 +336,6 @@ export function CustomNode({
setErrors({ ...errors });
};

// Helper function to parse keys with array indices
//TODO move to utils
const 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;
};

const getValue = (key: string) => {
const keys = parseKeys(key);
return keys.reduce((acc, k) => {
if (acc === undefined) return undefined;
if (k.index !== undefined) {
return Array.isArray(acc[k.key]) ? acc[k.key][k.index] : undefined;
}
return acc[k.key];
}, data.hardcodedValues as any);
};

const isHandleConnected = (key: string) => {
return (
data.connections &&
Expand All @@ -400,7 +358,7 @@ export function CustomNode({
const handleInputClick = (key: string) => {
console.debug(`Opening modal for key: ${key}`);
setActiveKey(key);
const value = getValue(key);
const value = getValue(key, data.hardcodedValues);
setInputModalValue(
typeof value === "object" ? JSON.stringify(value, null, 2) : value,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,10 @@ export const CredentialsInput: FC<{
useState<AbortController | null>(null);
const [oAuthError, setOAuthError] = useState<string | null>(null);

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

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

Comment on lines -94 to -101
Copy link
Member

Choose a reason for hiding this comment

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

why this change?

const {
schema,
provider,
Expand Down Expand Up @@ -226,6 +222,7 @@ export const CredentialsInput: FC<{
if (savedApiKeys.length === 0 && savedOAuthCredentials.length === 0) {
return (
<>
<span className="text-m green mb-0 text-gray-900">Credentials</span>
<div className={cn("flex flex-row space-x-2", className)}>
{supportsOAuth2 && (
<Button onClick={handleOAuthLogin}>
Expand All @@ -246,6 +243,31 @@ export const CredentialsInput: FC<{
)}
</>
);
// 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;
Comment on lines +259 to +283
Copy link
Member

Choose a reason for hiding this comment

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

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

}

function handleValueChange(newValue: string) {
Expand All @@ -263,7 +285,7 @@ export const CredentialsInput: FC<{
onSelectCredentials({
id: selectedCreds.id,
type: selectedCreds.type,
provider: schema.credentials_provider,
provider: provider,
// title: customTitle, // TODO: add input for title
});
}
Expand All @@ -272,6 +294,7 @@ export const CredentialsInput: FC<{
// Saved credentials exist
return (
<>
<span className="text-m green mb-0 text-gray-900">Credentials</span>
<Select value={selectedCredentials?.id} onValueChange={handleValueChange}>
<SelectTrigger>
<SelectValue placeholder={schema.placeholder} />
Expand Down
20 changes: 16 additions & 4 deletions autogpt_platform/frontend/src/hooks/useCredentials.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import { useContext } from "react";
import { CustomNodeData } from "@/components/CustomNode";
import { BlockIOCredentialsSubSchema } from "@/lib/autogpt-server-api";
import {
BlockIOCredentialsSubSchema,
CredentialsProviderName,
} from "@/lib/autogpt-server-api";
import { Node, useNodeId, useNodesData } from "@xyflow/react";
import {
CredentialsProviderData,
CredentialsProvidersContext,
} from "@/components/integrations/credentials-provider";
import { getValue } from "@/lib/utils";

export type CredentialsData =
| {
Expand Down Expand Up @@ -39,9 +43,17 @@ export default function useCredentials(): CredentialsData | null {
return null;
}

const provider = allProviders
? allProviders[credentialsSchema?.credentials_provider]
: null;
const discriminatorValue: CredentialsProviderName | null =
credentialsSchema.discriminator
? credentialsSchema.discriminator_mapping![
getValue(credentialsSchema.discriminator, data.hardcodedValues)
]
: null;

const providerName =
discriminatorValue || credentialsSchema.credentials_provider;

const provider = allProviders ? allProviders[providerName] : null;

const supportsApiKey =
credentialsSchema.credentials_types.includes("api_key");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,19 @@ export type CredentialsType = "api_key" | "oauth2";

// --8<-- [start:BlockIOCredentialsSubSchema]
export const PROVIDER_NAMES = {
ANTHROPIC: "anthropic",
D_ID: "d_id",
DISCORD: "discord",
GITHUB: "github",
GOOGLE: "google",
GOOGLE_MAPS: "google_maps",
GROQ: "groq",
Copy link
Member

Choose a reason for hiding this comment

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

probably need logos for these too

Copy link
Member

Choose a reason for hiding this comment

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

(even if fallback)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

IDEOGRAM: "ideogram",
JINA: "jina",
LLM: "llm",
MEDIUM: "medium",
NOTION: "notion",
OLLAMA: "ollama",
OPENAI: "openai",
OPENWEATHERMAP: "openweathermap",
PINECONE: "pinecone",
Expand All @@ -122,6 +125,8 @@ export type BlockIOCredentialsSubSchema = BlockIOSubSchemaMeta & {
credentials_provider: CredentialsProviderName;
credentials_scopes?: string[];
credentials_types: Array<CredentialsType>;
discriminator?: string;
discriminator_mapping?: { [key: string]: CredentialsProviderName };
};

export type BlockIONullSubSchema = BlockIOSubSchemaMeta & {
Expand Down
45 changes: 45 additions & 0 deletions autogpt_platform/frontend/src/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,3 +312,48 @@ export function findNewlyAddedBlockCoordinates(
y: 0,
};
}

type ParsedKey = { key: string; index?: number };

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;
}
Comment on lines +318 to +345
Copy link
Member

Choose a reason for hiding this comment

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

we should probably add security test for this lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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


/**
* Get the value of a nested key in an object, handles arrays and objects.
*/
export function getValue(key: string, value: any) {
const keys = parseKeys(key);
return keys.reduce((acc, k) => {
if (acc === undefined) return undefined;
if (k.index !== undefined) {
return Array.isArray(acc[k.key]) ? acc[k.key][k.index] : undefined;
}
return acc[k.key];
}, value);
}
Loading