Fix the auth tools issue (#829)

* TLK-1999 Fix the auth tools issue: display an error message if a tool is enabled and visible but not available. * TLK-1999 Fix the slack.md * TLK-1999 Fix typing
cohere-ai · Nov 8, 2024 · fa64235 · fa64235
1 parent 63f80ec
commit fa64235
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 26 deletions.
diff --git a/docs/custom_tool_guides/slack.md b/docs/custom_tool_guides/slack.md
@@ -47,7 +47,7 @@ SLACK_CLIENT_SECRET=<your_client_secret from step 1>
 
 ## 4. Enable the Slack Tool in the Frontend
 
-To enable the Slack tool in the frontend, you will need to modify the `src/community/config/tools.py` file. Add the `TOOL_SLACK_ID` to the `AGENT_SETTINGS_TOOLS` list.
+To enable the Slack tool in the frontend, you will need to modify the `src/interfaces/assistants_web/src/constants/tools.ts` file. Add the `TOOL_SLACK_ID` to the `AGENT_SETTINGS_TOOLS` list.
 
 ```typescript
 export const AGENT_SETTINGS_TOOLS = [
@@ -58,7 +58,7 @@ export const AGENT_SETTINGS_TOOLS = [
 ];
 ```
 
-To enable the Slack tool in the frontend for Base Agent, you will need to modify the `src/community/config/tools.py` file. Remove the `TOOL_SLACK_ID` from the `BASE_AGENT_EXCLUDED_TOOLS` list.
+To enable the Slack tool in the frontend for Base Agent, you will need to modify the `src/interfaces/assistants_web/src/constants/tools.ts` file. Remove the `TOOL_SLACK_ID` from the `BASE_AGENT_EXCLUDED_TOOLS` list.
 By default, the Slack Tool is disabled for the Base Agent. Also if you need to exclude some tool from the Base Agent just add it to the `BASE_AGENT_EXCLUDED_TOOLS` list.
 ```typescript
 export const BASE_AGENT_EXCLUDED_TOOLS = [];

diff --git a/src/backend/routers/tool.py b/src/backend/routers/tool.py
@@ -45,7 +45,9 @@ def list_tools(
         all_tools = agent_tools
 
     for tool in all_tools:
-        if tool.is_available and tool.auth_implementation is not None:
+        # Tools with auth implementation can be enabled and visible but not accessible (e.g., if secrets are not set).
+        # Therefore, we need to set is_auth_required for these types of tools as well for the frontend.
+        if (tool.is_available or tool.is_visible) and tool.auth_implementation is not None:
             try:
                 tool_auth_service = tool.auth_implementation()
 
@@ -56,7 +58,7 @@ def list_tools(
                 tool.token = tool_auth_service.get_token(session, user_id)
             except Exception as e:
                 logger.error(event=f"Error while fetching Tool Auth: {str(e)}")
-
+                tool.is_auth_required = True
                 tool.is_available = False
                 tool.error_message = (
                     f"Error while calling Tool Auth implementation {str(e)}"

diff --git a/src/interfaces/assistants_web/src/app/(main)/settings/Settings.tsx b/src/interfaces/assistants_web/src/app/(main)/settings/Settings.tsx
@@ -135,6 +135,8 @@ const GoogleDriveConnection = () => {
   };
 
   const isGoogleDriveConnected = !(googleDriveTool.is_auth_required ?? false);
+  const isGoogleDriveAvailable = googleDriveTool.is_available ?? false;
+  const googleDriveError = googleDriveTool.error_message ?? '';
   const authUrl = getToolAuthUrl(googleDriveTool.auth_url);
 
   return (
@@ -150,7 +152,16 @@ const GoogleDriveConnection = () => {
         Connect to Google Drive and add files to the assistant
       </Text>
       <section>
-        {isGoogleDriveConnected ? (
+        {!isGoogleDriveAvailable ? (
+          <div className="justify-items-start space-y-6">
+            <div className="flex items-center justify-between">
+              <p className="font-body text-p-sm uppercase text-danger-500">
+                {googleDriveError ||
+                  'Google Drive connection is not available. Please set the required configuration parameters.'}
+              </p>
+            </div>
+          </div>
+        ) : isGoogleDriveConnected ? (
           <div className="space-y-6">
             <div className="space-y-2">
               <Text styleAs="p-sm" className="uppercase text-volcanic-400 dark:text-mushroom-950">
@@ -208,20 +219,33 @@ const SlackConnection = () => {
   };
 
   const isSlackConnected = !(slackTool.is_auth_required ?? false);
+  const isSlackAvailable = slackTool.is_available ?? false;
+  const slackError = slackTool.error_message ?? '';
 
   return (
-    <article className="rounded-md border border-marble-800 p-4 dark:border-volcanic-500">
-      <header className="mb-2 flex items-center justify-between">
-        <div className="flex items-center gap-2">
-          <Icon name="slack" size="xl" />
-          <Text className="text-volcanic-400 dark:text-mushroom-950">Slack</Text>
-        </div>
-        <StatusConnection connected={isSlackConnected} />
-      </header>
-      <Text className="mb-6 text-volcanic-400 dark:text-mushroom-800">Connect to Slack</Text>
+    <article className="flex flex-col justify-between rounded-md border border-marble-800 p-4 dark:border-volcanic-500">
+      <div>
+        <header className="mb-2 flex items-center justify-between">
+          <div className="flex items-center gap-2">
+            <Icon name="slack" size="xl" />
+            <Text className="text-volcanic-400 dark:text-mushroom-950">Slack</Text>
+          </div>
+          <StatusConnection connected={isSlackConnected} />
+        </header>
+        <Text className="mb-6 text-volcanic-400 dark:text-mushroom-800">Connect to Slack</Text>
+      </div>
       <section>
-        {isSlackConnected ? (
-          <div className="space-y-6">
+        {!isSlackAvailable ? (
+          <div className="justify-items-start space-y-6">
+            <div className="flex items-center justify-between">
+              <p className="font-body text-p-sm uppercase text-danger-500">
+                {slackError ||
+                  'Slack connection is not available. Please set the required configuration parameters.'}
+              </p>
+            </div>
+          </div>
+        ) : isSlackConnected ? (
+          <div className="justify-items-end space-y-6">
             <div className="flex items-center justify-between">
               <Button
                 label="Delete connection"

diff --git a/src/interfaces/assistants_web/src/components/AgentSettingsForm/ToolsStep.tsx b/src/interfaces/assistants_web/src/components/AgentSettingsForm/ToolsStep.tsx
@@ -34,7 +34,7 @@ export const ToolsStep: React.FC<Props> = ({
   return (
     <div className="flex flex-col space-y-4">
       {availableTools?.map(
-        ({ name, description, is_auth_required, auth_url }) =>
+        ({ name, description, is_auth_required, auth_url, is_available, error_message }) =>
           !!name &&
           description && (
             <ToolRow
@@ -46,6 +46,8 @@ export const ToolsStep: React.FC<Props> = ({
               handleSwitch={(checked: boolean) => handleUpdateActiveTools(checked, name)}
               isAuthRequired={is_auth_required}
               authUrl={auth_url?.toString()}
+              isAvailable={is_available}
+              errorMessage={error_message}
               handleAuthButtonClick={handleAuthButtonClick}
             />
           )
@@ -68,6 +70,8 @@ const ToolRow: React.FC<{
   handleSwitch: (checked: boolean) => void;
   isAuthRequired?: boolean;
   authUrl?: string;
+  isAvailable?: boolean;
+  errorMessage?: string | null;
   handleAuthButtonClick?: (toolName: string) => void;
 }> = ({
   name,
@@ -77,6 +81,8 @@ const ToolRow: React.FC<{
   handleSwitch,
   isAuthRequired,
   authUrl,
+  isAvailable,
+  errorMessage,
   handleAuthButtonClick,
 }) => {
   return (
@@ -90,17 +96,25 @@ const ToolRow: React.FC<{
             {name}
           </Text>
         </div>
-        <div className="flex items-center space-x-2">
-          <Switch
-            checked={checked}
-            onChange={(checked: boolean) => !!name && handleSwitch(checked)}
-            showCheckedState
-          />
-        </div>
+        {isAvailable && (
+          <div className="flex items-center space-x-2">
+            <Switch
+              checked={checked}
+              onChange={(checked: boolean) => !!name && handleSwitch(checked)}
+              showCheckedState
+            />
+          </div>
+        )}
       </div>
       <Text className="dark:text-marble-800">{description}</Text>
       {!isAuthRequired && !!authUrl && <StatusConnection connected={!isAuthRequired} />}
-      {isAuthRequired && !!authUrl && (
+      {!isAvailable && (
+        <Text styleAs="caption" className="dark:text-danger-500">
+          {errorMessage ||
+            'Connection is not available. Please set the required configuration parameters.'}
+        </Text>
+      )}
+      {isAuthRequired && !!authUrl && isAvailable && (
         <Button
           kind="outline"
           theme="mushroom"

diff --git a/src/interfaces/assistants_web/src/constants/tools.ts b/src/interfaces/assistants_web/src/constants/tools.ts
@@ -21,7 +21,7 @@ export const AGENT_SETTINGS_TOOLS = [
 ];
 
 // Tools won't be available for the base agent
-export const BASE_AGENT_EXCLUDED_TOOLS = [TOOL_SLACK_ID];
+export const BASE_AGENT_EXCLUDED_TOOLS: string[] = [TOOL_SLACK_ID];
 
 export const TOOL_FALLBACK_ICON = 'circles-four';
 export const TOOL_ID_TO_DISPLAY_INFO: { [id: string]: { icon: IconName } } = {