Skip to content

Commit 29aee79

Browse files
fix: dedupe function env vars (#772)
fixes AGE-828 fixes potentially duplicate env vars from functions in the UX and MCP config - Ensures we don't return duplicated env vars more the same function. - If there happened to be an env var for http tools and functions in the same toolset ensures we don't duplicate the UX on it. We will still return the data so both fields are accurate.
1 parent 4296f6e commit 29aee79

File tree

8 files changed

+115
-99
lines changed

8 files changed

+115
-99
lines changed

.changeset/sad-flies-clap.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"dashboard": patch
3+
"server": patch
4+
---
5+
6+
fixes potentially duplicate env vars from functions in the UX and MCP config
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { useMemo } from "react";
2+
3+
import { Toolset } from "@/lib/toolTypes";
4+
import { ToolsetEntry } from "@gram/client/models/components";
5+
6+
type ToolsetLike = (Toolset | ToolsetEntry) | undefined;
7+
8+
export function useToolsetEnvVars(
9+
toolset: ToolsetLike,
10+
requiresServerURL: boolean,
11+
): string[] {
12+
return useMemo(() => {
13+
if (!toolset) return [];
14+
15+
const securityVars =
16+
toolset.securityVariables?.flatMap((secVar) => secVar.envVariables) ?? [];
17+
18+
const functionEnvVars =
19+
toolset.functionEnvironmentVariables?.map((fnVar) => fnVar.name) ?? [];
20+
21+
const serverVars =
22+
toolset.serverVariables?.flatMap((serverVar) =>
23+
serverVar.envVariables.filter((envVar) => {
24+
if (requiresServerURL) return true;
25+
return !envVar.toLowerCase().includes("server_url");
26+
}),
27+
) ?? [];
28+
29+
return [...new Set([...securityVars, ...serverVars, ...functionEnvVars])];
30+
}, [toolset, requiresServerURL]);
31+
}

client/dashboard/src/pages/mcp/MCPDetails.tsx

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { useProject, useSession } from "@/contexts/Auth";
1919
import { useSdkClient } from "@/contexts/Sdk";
2020
import { useTelemetry } from "@/contexts/Telemetry";
2121
import { useListTools, useToolset } from "@/hooks/toolTypes";
22+
import { useToolsetEnvVars } from "@/hooks/useToolsetEnvVars";
2223
import { isHttpTool, Toolset } from "@/lib/toolTypes";
2324
import { cn, getServerURL } from "@/lib/utils";
2425
import { useRoutes } from "@/routes";
@@ -668,31 +669,19 @@ export const useMcpConfigs = (toolset: ToolsetEntry | undefined) => {
668669
const { url: mcpUrl } = useMcpUrl(toolset);
669670
const { data: tools } = useListTools();
670671

671-
if (!toolset) return { public: "", internal: "" };
672+
const toolsetTools = toolset
673+
? tools?.tools.filter((tool) => toolset.tools.some((t) => t.id === tool.id))
674+
: undefined;
672675

673-
const toolsetTools = tools?.tools.filter((tool) =>
674-
toolset.tools.some((t) => t.id === tool.id),
675-
);
676-
const requiresServerURL = toolsetTools?.some(
677-
(tool) => isHttpTool(tool) && !tool.defaultServerUrl,
676+
const requiresServerURL =
677+
toolsetTools?.some((tool) => isHttpTool(tool) && !tool.defaultServerUrl) ??
678+
false;
679+
680+
const envHeaders = useToolsetEnvVars(toolset, requiresServerURL).filter(
681+
(header) => !header.toLowerCase().includes("token_url"),
678682
);
679683

680-
const envHeaders: string[] = [
681-
// Security variables (exclude token_url)
682-
...(toolset.securityVariables?.flatMap((secVar) =>
683-
secVar.envVariables.filter(
684-
(v) => !v.toLowerCase().includes("token_url"), // direct token url is always a hidden option right now
685-
),
686-
) ?? []),
687-
// Function environment variables
688-
...(toolset.functionEnvironmentVariables?.map((fnVar) => fnVar.name) ?? []),
689-
// Server variables (filter server_url unless required)
690-
...(toolset.serverVariables?.flatMap((serverVar) =>
691-
serverVar.envVariables.filter(
692-
(v) => !v.toLowerCase().includes("server_url") || requiresServerURL,
693-
),
694-
) ?? []),
695-
];
684+
if (!toolset) return { public: "", internal: "" };
696685

697686
// Build the args array for public MCP config
698687
const mcpJsonPublicArgs = [

client/dashboard/src/pages/toolsets/ToolsetAuth.tsx

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { AlertCircle, Plus } from "lucide-react";
1313
import { useCallback, useEffect, useMemo, useState } from "react";
1414
import { useEnvironments } from "../environments/Environments";
1515
import { EnvironmentSelector } from "./EnvironmentSelector";
16+
import { useToolsetEnvVars } from "@/hooks/useToolsetEnvVars";
1617

1718
const PASSWORD_MASK = "••••••••";
1819
const SECRET_FIELD_INDICATORS = ["SECRET", "KEY"] as const;
@@ -143,30 +144,11 @@ export function ToolsetAuth({
143144
setSaveError(null);
144145
}, []);
145146

146-
const requiresServerURL = toolset.tools?.some(
147-
(tool) => isHttpTool(tool) && !tool.defaultServerUrl,
148-
);
149-
150-
const relevantEnvVars = useMemo(() => {
151-
const securityVars =
152-
toolset?.securityVariables?.flatMap((secVar) => secVar.envVariables) ??
153-
[];
154-
const serverVars =
155-
toolset?.serverVariables?.flatMap((serverVar) =>
156-
serverVar.envVariables.filter(
157-
(v) => !v.toLowerCase().includes("server_url") || requiresServerURL,
158-
),
159-
) ?? [];
160-
const functionEnvVars =
161-
toolset?.functionEnvironmentVariables?.map((fnVar) => fnVar.name) ?? [];
147+
const requiresServerURL =
148+
toolset.tools?.some((tool) => isHttpTool(tool) && !tool.defaultServerUrl) ??
149+
false;
162150

163-
return [...securityVars, ...serverVars, ...functionEnvVars];
164-
}, [
165-
toolset?.securityVariables,
166-
toolset?.serverVariables,
167-
toolset.functionEnvironmentVariables,
168-
requiresServerURL,
169-
]);
151+
const relevantEnvVars = useToolsetEnvVars(toolset, requiresServerURL);
170152

171153
const environmentVariableInputs = useMemo(() => {
172154
return relevantEnvVars.map((varName) => {

client/dashboard/src/pages/toolsets/ToolsetAuthAlert.tsx

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,28 +2,7 @@ import { useMemo } from "react";
22
import { Alert } from "@speakeasy-api/moonshine";
33
import { useEnvironments } from "../environments/Environments";
44
import { Toolset } from "@/lib/toolTypes";
5-
6-
function useRelevantEnvVars(toolset: Toolset) {
7-
return useMemo(() => {
8-
const requiresServerURL = toolset.tools?.some(
9-
(tool) => tool.type === "http" && !tool.defaultServerUrl,
10-
);
11-
12-
const securityVars =
13-
toolset?.securityVariables?.flatMap((secVar) => secVar.envVariables) ??
14-
[];
15-
const functionEnvironmentVariables =
16-
toolset?.functionEnvironmentVariables?.map((fnVar) => fnVar.name) ?? [];
17-
const serverVars =
18-
toolset?.serverVariables?.flatMap((serverVar) =>
19-
serverVar.envVariables.filter(
20-
(v) => !v.toLowerCase().includes("server_url") || requiresServerURL,
21-
),
22-
) ?? [];
23-
24-
return [...securityVars, ...serverVars, ...functionEnvironmentVariables];
25-
}, [toolset]);
26-
}
5+
import { useToolsetEnvVars } from "@/hooks/useToolsetEnvVars";
276

287
// Types
298
interface EnvironmentEntry {
@@ -89,7 +68,12 @@ function PlaygroundAuthAlert({
8968
onConfigureClick,
9069
}: PlaygroundAuthAlertProps) {
9170
const environments = useEnvironments();
92-
const relevantEnvVars = useRelevantEnvVars(toolset);
71+
const requiresServerURL =
72+
toolset.tools?.some(
73+
(tool) => tool.type === "http" && !tool.defaultServerUrl,
74+
) ?? false;
75+
76+
const relevantEnvVars = useToolsetEnvVars(toolset, requiresServerURL);
9377

9478
const envSlug = environmentSlug ?? toolset.defaultEnvironmentSlug;
9579
const environment = environments.find((env) => env.slug === envSlug);
@@ -131,7 +115,12 @@ function ToolsetPageAuthAlert({
131115
onConfigureClick,
132116
}: ToolsetPageAuthAlertProps) {
133117
const environments = useEnvironments();
134-
const relevantEnvVars = useRelevantEnvVars(toolset);
118+
const requiresServerURL =
119+
toolset.tools?.some(
120+
(tool) => tool.type === "http" && !tool.defaultServerUrl,
121+
) ?? false;
122+
123+
const relevantEnvVars = useToolsetEnvVars(toolset, requiresServerURL);
135124

136125
const hasAllRequiredVars = useMemo(() => {
137126
if (relevantEnvVars.length === 0) return true;

client/dashboard/src/pages/toolsets/ToolsetEnvironmentBadge.tsx

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { Check } from "lucide-react";
1515
import { useEnvironments } from "../environments/Environments";
1616
import { useState } from "react";
1717
import { isHttpTool, Toolset } from "@/lib/toolTypes";
18+
import { useToolsetEnvVars } from "@/hooks/useToolsetEnvVars";
1819

1920
export const ToolsetEnvironmentBadge = ({
2021
toolset,
@@ -35,6 +36,13 @@ export const ToolsetEnvironmentBadge = ({
3536
const [envVarsDialogOpen, setEnvVarsDialogOpen] = useState(false);
3637
const [envVars, setEnvVars] = useState<Record<string, string>>({});
3738

39+
const requiresServerURL =
40+
toolset?.tools?.some(
41+
(tool) => isHttpTool(tool) && !tool.defaultServerUrl,
42+
) ?? false;
43+
44+
const relevantEnvVars = useToolsetEnvVars(toolset, requiresServerURL);
45+
3846
const updateEnvironmentMutation = useUpdateEnvironmentMutation({
3947
onSuccess: () => {
4048
telemetry.capture("environment_event", { action: "environment_updated" });
@@ -52,24 +60,6 @@ export const ToolsetEnvironmentBadge = ({
5260

5361
const environment = environments.find((env) => env.slug === envSlug);
5462

55-
const requiresServerURL = toolset.tools?.some(
56-
(tool) => isHttpTool(tool) && !tool.defaultServerUrl,
57-
);
58-
59-
const relevantEnvVars: string[] = [
60-
// Security variables (no filtering)
61-
...(toolset.securityVariables?.flatMap((secVar) => secVar.envVariables) ??
62-
[]),
63-
// Function environment variables
64-
...(toolset.functionEnvironmentVariables?.map((fnVar) => fnVar.name) ?? []),
65-
// Server variables (filter server_url unless required)
66-
...(toolset.serverVariables?.flatMap((serverVar) =>
67-
serverVar.envVariables.filter(
68-
(v) => !v.toLowerCase().includes("server_url") || requiresServerURL,
69-
),
70-
) ?? []),
71-
];
72-
7363
const missingEnvVars =
7464
relevantEnvVars?.filter(
7565
(varName) =>

server/internal/mcpmetadata/impl.go

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,7 @@ func (s *Service) collectEnvironmentVariables(mode securityMode, toolsetDetails
483483
case securityModePublic, securityModeOAuth:
484484
var inputs []securityInput
485485
isOAuthEnabled := mode == securityModeOAuth
486+
seen := make(map[string]bool)
486487

487488
for _, secVar := range toolsetDetails.SecurityVariables {
488489
for _, envVar := range secVar.EnvVariables {
@@ -495,20 +496,26 @@ func (s *Service) collectEnvironmentVariables(mode securityMode, toolsetDetails
495496
continue
496497
}
497498

498-
inputs = append(inputs, securityInput{
499-
SystemName: fmt.Sprintf("MCP-%s", envVar),
500-
DisplayName: fmt.Sprintf("MCP-%s", strings.ReplaceAll(envVar, "_", "-")),
501-
Sensitive: true,
502-
})
499+
if !seen[envVar] {
500+
seen[envVar] = true
501+
inputs = append(inputs, securityInput{
502+
SystemName: fmt.Sprintf("MCP-%s", envVar),
503+
DisplayName: fmt.Sprintf("MCP-%s", strings.ReplaceAll(envVar, "_", "-")),
504+
Sensitive: true,
505+
})
506+
}
503507
}
504508
}
505509

506510
for _, functionEnvVar := range toolsetDetails.FunctionEnvironmentVariables {
507-
inputs = append(inputs, securityInput{
508-
SystemName: fmt.Sprintf("MCP-%s", functionEnvVar.Name),
509-
DisplayName: fmt.Sprintf("MCP-%s", strings.ReplaceAll(functionEnvVar.Name, "_", "-")),
510-
Sensitive: true,
511-
})
511+
if !seen[functionEnvVar.Name] {
512+
seen[functionEnvVar.Name] = true
513+
inputs = append(inputs, securityInput{
514+
SystemName: fmt.Sprintf("MCP-%s", functionEnvVar.Name),
515+
DisplayName: fmt.Sprintf("MCP-%s", strings.ReplaceAll(functionEnvVar.Name, "_", "-")),
516+
Sensitive: true,
517+
})
518+
}
512519
}
513520

514521
return inputs

server/internal/mv/toolset.go

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ func DescribeToolsetEntry(
241241
DefaultEnvironmentSlug: conv.FromPGText[types.Slug](toolset.DefaultEnvironmentSlug),
242242
SecurityVariables: securityVars,
243243
ServerVariables: serverVars,
244-
FunctionEnvironmentVariables: functionEnvVars,
244+
FunctionEnvironmentVariables: dedupeFunctionEnvVars(functionEnvVars),
245245
Description: conv.FromPGText[string](toolset.Description),
246246
McpSlug: conv.FromPGText[types.Slug](toolset.McpSlug),
247247
McpEnabled: &toolset.McpEnabled,
@@ -446,7 +446,7 @@ func DescribeToolset(
446446
DefaultEnvironmentSlug: conv.FromPGText[types.Slug](toolset.DefaultEnvironmentSlug),
447447
SecurityVariables: toolsetTools.SecurityVars,
448448
ServerVariables: toolsetTools.ServerVars,
449-
FunctionEnvironmentVariables: toolsetTools.FunctionEnvVars,
449+
FunctionEnvironmentVariables: dedupeFunctionEnvVars(toolsetTools.FunctionEnvVars),
450450
Description: conv.FromPGText[string](toolset.Description),
451451
Tools: toolsetTools.Tools,
452452
Resources: toolsetTools.Resources,
@@ -898,6 +898,28 @@ func environmentVariablesForTools(ctx context.Context, tx DBTX, tools []toolEnvL
898898
return slices.Collect(maps.Values(securityVarsMap)), serverVars, nil
899899
}
900900

901+
// dedupeFunctionEnvVars returns function environment variables deduplicated by name.
902+
func dedupeFunctionEnvVars(vars []*types.FunctionEnvironmentVariable) []*types.FunctionEnvironmentVariable {
903+
if len(vars) == 0 {
904+
return vars
905+
}
906+
907+
seen := make(map[string]bool, len(vars))
908+
var deduped []*types.FunctionEnvironmentVariable
909+
for _, envVar := range vars {
910+
if envVar == nil {
911+
continue
912+
}
913+
if _, exists := seen[envVar.Name]; exists {
914+
continue
915+
}
916+
seen[envVar.Name] = true
917+
deduped = append(deduped, envVar)
918+
}
919+
920+
return deduped
921+
}
922+
901923
const defaultPromptTemplateKind = "prompt"
902924

903925
func parseKind(pt tsr.GetPromptTemplatesForToolsetRow) string {

0 commit comments

Comments
 (0)