fix: use manifest default values for MCP server env vars and headers by justinas-b · Pull Request #5922 · obot-platform/obot

justinas-b · 2026-02-27T12:41:30Z

Summary

Manifest-provided value fields on MCP server env vars were ignored for non-system servers. This caused servers to appear unconfigured and users to be prompted for values that already had defaults.

Backend

ServerToServerConfig and legacyServerToServerConfig now fall back to env.Value when no user credential exists (credential takes precedence so users can still override defaults)
ConvertMCPServer and mcpServerOrInstanceFromConnectURL skip env vars and headers with static values when checking for missing required configuration

Frontend

Configuration forms pre-fill with manifest defaults when no stored credential exists
hasEditableConfiguration still shows the edit dialog for env vars with defaults so users can override them

Env vars with a `value` field in MCP server manifests were ignored for non-system servers. The backend skipped env vars not found in stored credentials, even when the manifest provided a default. The frontend also overwrote manifest defaults with empty strings. This aligns the behavior of env vars with headers, which already correctly fall back to manifest values. The fix mirrors the existing pattern in SystemServerToServerConfig. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR aims to make MCP server manifest-provided env var value fields take effect (backend + frontend), so servers can be treated as configured and configuration UIs can prefill values when defaults exist.

Changes:

Backend: incorporate manifest env.value into server config generation and “configured/missing required vars” evaluation.
Backend: allow auto-creating catalog-entry-based servers when required env vars have manifest defaults.
Frontend: prefill configuration forms using storedValue ?? manifestValue ?? '' and adjust “hasEditableConfiguration” env handling.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
ui/user/src/lib/services/chat/mcp.ts	Adjusts configurability detection and composite form env initialization to consider manifest env defaults.
ui/user/src/lib/components/mcp/McpServerInfoAndTools.svelte	Prefills env inputs with manifest defaults when no stored value exists.
ui/user/src/lib/components/mcp/EditExistingDeployment.svelte	Prefills env inputs with manifest defaults when no stored value exists.
ui/user/src/lib/components/chat/McpServerRequirements.svelte	Prefills env inputs with manifest defaults when no stored value exists.
pkg/mcp/types.go	Updates env var resolution in server config generation to incorporate manifest `env.Value` and adjust prefix application.
pkg/api/handlers/mcp.go	Treats required env vars with manifest defaults as non-missing and relaxes auto-create blocking based on required envs.

Comments suppressed due to low confidence (2)

ui/user/src/lib/services/chat/mcp.ts:103

hasEditableConfiguration is used to decide whether to show configuration/edit flows, but this change treats env vars with a manifest value as non-editable. That means catalog entries with only defaulted env vars will no longer open the configuration dialog (and won’t show “Edit Configuration”), even though the forms now prefill defaults and appear to support user overrides. If defaults are meant to be overridable, consider counting env vars as editable regardless of env.value (or split the concepts of “required to launch” vs “editable”).

			const hasEnvs =
				(component.manifest?.env?.filter?.((env) => !env.value)?.length ?? 0) > 0;
			const hasHeaders =
				(component?.manifest?.remoteConfig?.headers?.filter?.((header) => !header.value)?.length ??
					0) > 0;
			const hasUrlToFill =
				!component.manifest?.remoteConfig?.fixedURL && component.manifest?.remoteConfig?.hostname;
			return hasEnvs || hasHeaders || hasUrlToFill;
		});
	}

	const hasUrlToFill =
		!item.manifest?.remoteConfig?.fixedURL && item.manifest?.remoteConfig?.hostname;
	const hasEnvsToFill =
		(item.manifest?.env?.filter?.((env) => !env.value)?.length ?? 0) > 0;
	const hasHeadersToFill =
		(item?.manifest?.remoteConfig?.headers?.filter?.((header) => !header.value)?.length ?? 0) > 0;

	return hasUrlToFill || hasEnvsToFill || hasHeadersToFill;

pkg/api/handlers/mcp.go:2509

This function now skips required env vars that have a manifest value, but the required header check below still ignores header.Value. A remote server/catalog entry with a required header that is statically provided by the manifest will still be reported as missing/unconfigured. Consider applying the same “static value means not missing” logic for headers for consistency.

	// Check for missing required env vars
	for _, env := range server.Spec.Manifest.Env {
		if !env.Required {
			continue
		}

		// Env vars with a static default value are never considered missing
		if env.Value != "" {
			continue
		}

		if _, ok := credEnv[env.Key]; !ok {
			missingEnvVars = append(missingEnvVars, env.Key)
		}
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Swap precedence in ServerToServerConfig and legacyServerToServerConfig to check user credentials before manifest defaults, allowing overrides - Add header.Value check in ConvertMCPServer and mcpServerOrInstanceFromConnectURL so required headers with static values aren't treated as missing - Revert hasEditableConfiguration env filter so users can still open the config dialog to override manifest defaults

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-02T08:29:16Z

 	// Check for missing required env vars
 	for _, env := range server.Spec.Manifest.Env {
 		if !env.Required {
 			continue
 		}

+		// Env vars with a static default value are never considered missing
+		if env.Value != "" {
+			continue
+		}
+
 		if _, ok := credEnv[env.Key]; !ok {
 			missingEnvVars = append(missingEnvVars, env.Key)
 		}


ConvertMCPServer determines whether required env vars are missing by checking only for key presence in credEnv. Elsewhere (e.g., ServerToServerConfig and system server conversion) empty credential values are treated as missing, so a present-but-empty value would incorrectly mark the server as configured. Consider treating env vars as missing when credEnv[env.Key] == "" (unless env.Value is set).

Copilot · 2026-03-02T08:29:17Z

+				continue
+			}
+
 			if _, ok := credEnv[header.Key]; !ok {


ConvertMCPServer determines whether required headers are missing by checking only for key presence in credEnv. If a header key exists with an empty value, it will be treated as configured even though ServerToServerConfig ignores empty credential values. Consider checking credEnv[header.Key] == "" (unless header.Value is set) so the configured/missing state stays consistent.

Suggested change

if _, ok := credEnv[header.Key]; !ok {

if val, ok := credEnv[header.Key]; !ok || val == "" {

Copilot · 2026-03-02T08:29:17Z

 	for _, env := range mcpServer.Spec.Manifest.Env {
-		val, ok := credEnv[env.Key]
-		if !ok || val == "" {
+		var (
+			val            string
+			hasValue       bool
+			fromCredential bool
+		)
+
+		// Check user-configured value from credentials first
+		credVal, ok := credEnv[env.Key]
+		if ok && credVal != "" {
+			val = credVal
+			hasValue = true
+			fromCredential = true
+		} else if env.Value != "" {
+			// Fall back to static default from manifest
+			val = env.Value
+			hasValue = true
+		}
+
+		if !hasValue {
 			if env.Required {
 				missingRequiredNames = append(missingRequiredNames, env.Key)
 			}
 			continue
 		}

-		// Apply prefix if specified (e.g., "Bearer ", "sk-")
-		val = applyPrefix(val, env.Prefix)
+		// Apply prefix only to user-supplied values, not static defaults
+		if fromCredential {
+			val = applyPrefix(val, env.Prefix)
+		}



The env var resolution logic now falls back to manifest defaults (env.Value) and only applies env.Prefix to user-supplied credential values. There don't appear to be tests covering env.Value defaults (and the prefix/not-prefix behavior for static defaults), so this could regress silently. Consider adding types_test.go cases for both ServerToServerConfig and legacyServerToServerConfig that assert: (1) required env with env.Value is not reported missing, (2) default value is used when no credential exists, and (3) prefix is not applied to the static default but is applied to credential overrides.

Copilot · 2026-03-02T08:29:17Z

 			envs: isMultiUser
 				? []
 				: (m.env ?? []).map((e) => ({
 						...(e as unknown as Record<string, unknown>),
 						key: e.key,
-						value: init?.config?.[e.key] ?? ''
+						value: init?.config?.[e.key] ?? e.value ?? ''
 					})),


PR description mentions updating hasEditableConfiguration to ignore env vars that have static manifest defaults (env.value), but this file's hasEditableConfiguration logic still treats any env entries as editable/required by checking only env.length > 0. If the intent is to avoid prompting users to configure when all required envs are satisfied by defaults, hasEditableConfiguration likely needs a similar filter to the one used for headers (e.g., exclude envs where env.value is set).

Copilot AI review requested due to automatic review settings February 27, 2026 12:41

Copilot started reviewing on behalf of justinas-b February 27, 2026 12:41 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

Comment thread pkg/api/handlers/mcp.go

Comment thread pkg/mcp/types.go Outdated

Comment thread pkg/mcp/types.go Outdated

justinas-b mentioned this pull request Feb 27, 2026

Default values for environment variables are not used #5923

Open

justinas-b requested a review from Copilot March 2, 2026 08:20

Copilot started reviewing on behalf of justinas-b March 2, 2026 08:20 View session

justinas-b changed the title ~~fix: use env var default values from manifest for MCP servers~~ fix: use manifest default values for MCP server env vars and headers Mar 2, 2026

Copilot AI reviewed Mar 2, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix: use manifest default values for MCP server env vars and headers - #5922

fix: use manifest default values for MCP server env vars and headers#5922
justinas-b wants to merge 2 commits into
obot-platform:mainfrom
justinas-b:fix/env-default-values

justinas-b commented Feb 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if _, ok := credEnv[header.Key]; !ok {
	if val, ok := credEnv[header.Key]; !ok \|\| val == "" {

Uh oh!

Conversation

justinas-b commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Backend

Frontend

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

justinas-b commented Feb 27, 2026 •

edited

Loading