fix: surface the real reason an MCP server is unavailable

packages/opencode/src/cli/cmd/mcp.ts

@@ -9,6 +9,10 @@ import { LATEST_PROTOCOL_VERSION } from "@modelcontextprotocol/sdk/types.js"
 import * as prompts from "@clack/prompts"
 import { UI } from "../ui"
 import { MCP } from "../../mcp"
+// altimate_change start — upstream_fix (#701): env diagnostics surfaced by `mcp list`.
+import * as McpDiscover from "../../mcp/discover"
+import { ConfigVariable } from "../../config/variable"
+// altimate_change end
 import { McpAuth } from "../../mcp/auth"
 import { McpOAuthProvider } from "../../mcp/oauth-provider"
 import { Config } from "@/config/config"
@@ -167,12 +171,30 @@ export const McpListCommand = effectCmd({
         hint = "\n    " + status.error
       }
 
+      // altimate_change start — upstream_fix (#701): name variables that resolved to "".
+      // A blank `${SNOWFLAKE_PASSWORD}` often connects and only fails on first real use, so
+      // this is appended regardless of status rather than only on the failure branch.
+      const unresolved = McpDiscover.unresolvedEnvVars(name)
+      if (unresolved.length > 0) {
+        hint += "\n    unresolved env: " + unresolved.join(", ") + " (set or remove)"
+      }
+      // altimate_change end
+
       const typeHint = serverConfig.type === "remote" ? serverConfig.url : serverConfig.command.join(" ")
       prompts.log.info(
         `${statusIcon} ${name} ${UI.Style.TEXT_DIM}${statusText}${hint}\n    ${UI.Style.TEXT_DIM}${typeHint}`,
       )
     }
 
+    // altimate_change start — upstream_fix (#701): a missing `{env:VAR}` becomes "" and the config
+    // parses clean, so a blank credential reaches the server and fails much later with an error
+    // naming neither. Attribution to a single server is not available here (substitution runs on
+    // raw config text, before any structure exists), so this is reported against the file.
+    for (const { source, names } of ConfigVariable.blankedEnvVars()) {
+      prompts.log.warn(`${names.join(", ")} resolved to empty in ${source} (set or remove)`)
+    }
+    // altimate_change end
+
     prompts.outro(`${servers.length} server(s)`)
   }),
 })

packages/opencode/src/config/config.ts

@@ -159,6 +159,10 @@ async function substituteWellKnownRemoteConfig(input: {
 }) {
   if (!isRecord(input.value) || typeof input.value.url !== "string") return undefined
 
+  // altimate_change start — upstream_fix (#701): the url and every header below publish under
+  // this same source, so clear once here and let those calls union into one record.
+  ConfigVariable.resetBlankedEnvVars(input.source)
+  // altimate_change end
   const url = await ConfigVariable.substitute({
     text: input.value.url,
     type: "virtual",
@@ -341,6 +345,14 @@ export const layer = Layer.effect(
 
     const loadFile = Effect.fnUntraced(function* (filepath: string, env?: Record<string, string>) {
       yield* Effect.logInfo("loading", { path: filepath })
+      // altimate_change start — upstream_fix (#701): substitution unions now, so whoever begins a
+      // load clears this source first. Before the empty-file return, not after: a config that is
+      // deleted or emptied must drop the names it recorded while it still had a `{env:VAR}`,
+      // otherwise `mcp list` warns about a variable that appears in no config at all.
+      // Deliberately NOT inside loadConfig — the well-known flow records url/header blanks under
+      // the same source before calling it, and a reset in there threw those names away.
+      ConfigVariable.resetBlankedEnvVars(filepath)
+      // altimate_change end
       const text = yield* readConfigFile(filepath)
       if (!text) return {} as Info
       return yield* loadConfig(text, { path: filepath }, env)
@@ -598,6 +610,9 @@ export const layer = Layer.effect(
 
         if (process.env.OPENCODE_CONFIG_CONTENT) {
           const source = "OPENCODE_CONFIG_CONTENT"
+          // altimate_change start — upstream_fix (#701): clear before this load.
+          ConfigVariable.resetBlankedEnvVars(source)
+          // altimate_change end
           const next = yield* loadConfig(process.env.OPENCODE_CONFIG_CONTENT, {
             dir: ctx.directory,
             source,
@@ -625,6 +640,9 @@ export const layer = Layer.effect(
 
             if (Option.isSome(configOpt)) {
               const source = `${url}/api/config`
+              // altimate_change start — upstream_fix (#701): clear before this load.
+              ConfigVariable.resetBlankedEnvVars(source)
+              // altimate_change end
               const next = yield* loadConfig(JSON.stringify(configOpt.value), {
                 dir: path.dirname(source),
                 source,
@@ -671,6 +689,9 @@ export const layer = Layer.effect(
         // macOS managed preferences (.mobileconfig deployed via MDM) override everything
         const managed = yield* Effect.promise(() => ConfigManaged.readManagedPreferences())
         if (managed) {
+          // altimate_change start — upstream_fix (#701): clear before this load.
+          ConfigVariable.resetBlankedEnvVars(managed.source)
+          // altimate_change end
           // altimate_change start — note a managed datamate key before merging
           const managedPrefs = yield* loadConfig(managed.text, {
             dir: path.dirname(managed.source),

packages/opencode/src/config/tui.ts

@@ -104,6 +104,11 @@ const loadState = Effect.fn("TuiConfig.loadState")(function* (ctx: { directory:
 
   const load = (text: string, configFilepath: string): Effect.Effect<Info> =>
     Effect.gen(function* () {
+      // altimate_change start — upstream_fix (#701): substitution unions now instead of
+      // replacing, so every caller clears first. Without this a `{env:VAR}` in tui.json that
+      // was later fixed kept being reported blank for the life of the process.
+      ConfigVariable.resetBlankedEnvVars(configFilepath)
+      // altimate_change end
       const expanded = yield* Effect.promise(() =>
         ConfigVariable.substitute({ text, type: "path", path: configFilepath, missing: "empty" }),
       )

packages/opencode/src/config/variable.ts

@@ -28,6 +28,27 @@ type SubstituteInput = ParseSource & {
   // altimate_change end
 }
 
+// altimate_change start — upstream_fix (#701): keep the names of variables that silently blanked.
+// An unresolved bare `${VAR}` is left LITERAL above on purpose, so it stays visible and is not
+// recorded here. `{env:VAR}` has no such deferral — it becomes "" and the config parses clean, so
+// a missing `{env:SNOWFLAKE_PASSWORD}` launches an MCP server with a blank credential and fails
+// later with an error naming neither the variable nor this file. Keyed by config source; the
+// newest parse of a file replaces its entry so a fixed variable stops being reported.
+const _blankedEnv = new Map<string, Set<string>>()
+
+/** Drop `src`'s record so a load starts clean; substitution then unions within that load. */
+export function resetBlankedEnvVars(src: string) {
+  _blankedEnv.delete(src)
+}
+
+/** Variable names that silently became "" during config substitution, grouped by config source. */
+export function blankedEnvVars(): { source: string; names: string[] }[] {
+  return [..._blankedEnv.entries()]
+    .map(([src, names]) => ({ source: src, names: [...names].sort() }))
+    .sort((a, b) => a.source.localeCompare(b.source))
+}
+// altimate_change end
+
 function source(input: ParseSource) {
   return input.type === "path" ? input.path : input.source
 }
@@ -42,6 +63,9 @@ export async function substitute(input: SubstituteInput) {
   // altimate_change start — upstream_fix: restore ${VAR}/${VAR:-default}/$${VAR} config interpolation
   const format = input.format ?? "json"
   const encode = (value: string) => (format === "raw" ? value : JSON.stringify(value).slice(1, -1))
+  // altimate_change — upstream_fix (#701): collect blanked names for this parse, replacing any
+  // earlier entry for the same source rather than accumulating stale ones.
+  const blanked = new Set<string>()
   let text = input.text.replace(ConfigPaths.ENV_VAR_PATTERN, (match, escaped, dollarVar, dollarDefault, braceVar) => {
     if (escaped !== undefined) return "$" + escaped
     if (dollarVar !== undefined) {
@@ -56,12 +80,27 @@ export async function substitute(input: SubstituteInput) {
       return match
     }
     if (braceVar !== undefined) {
-      return (input.env?.[braceVar] ?? process.env[braceVar]) || ""
+      const value = input.env?.[braceVar] ?? process.env[braceVar]
+      // altimate_change — upstream_fix (#701): record the blank, then behave exactly as before.
+      if (!value) blanked.add(braceVar)
+      return value || ""
     }
     return match
   })
   // altimate_change end
 
+  // altimate_change start — upstream_fix (#701): publish after the whole text is scanned.
+  // Union, not replace: one source is substituted more than once — a remote config resolves
+  // its `url` and then each header separately, all under the same source. Replacing meant a
+  // later clean call erased the names an earlier call had found, so `mcp list` silently
+  // omitted a blank credential. Clearing is `resetBlankedEnvVars`, called per load below.
+  if (blanked.size > 0) {
+    const existing = _blankedEnv.get(source(input))
+    if (existing) for (const name of blanked) existing.add(name)
+    else _blankedEnv.set(source(input), blanked)
+  }
+  // altimate_change end
+
   const fileMatches = Array.from(text.matchAll(/\{file:[^}]+\}/g))
   if (!fileMatches.length) return text
 

packages/opencode/src/mcp/discover.ts

@@ -34,11 +34,47 @@ function resolveServerEnvVars(
       field: context.field,
       unresolved: stats.unresolvedNames.join(", "),
     })
+    // altimate_change start — upstream_fix: remember it for the user, not just the log (#701).
+    // An unresolved `${SNOWFLAKE_PASSWORD}` becomes "" and the server launches with a blank
+    // credential, failing later with something that names neither the variable nor the config
+    // file. The log line already had the answer; nobody reads it. Recorded here so `/mcps` can
+    // say so. Mirrors the `setDiscoveryResult` handoff below.
+    const seen = _unresolvedEnv.get(context.server) ?? new Set<string>()
+    for (const name of stats.unresolvedNames) seen.add(name)
+    _unresolvedEnv.set(context.server, seen)
+    // altimate_change end
   }
   return out
 }
 // altimate_change end
 
+// altimate_change start — upstream_fix: unresolved-variable record for the user surface (#701).
+/** Server name -> variable names that resolved to "" during discovery. */
+const _unresolvedEnv = new Map<string, Set<string>>()
+
+/**
+ * Variable names that silently became "" for `server`, from the most recent discovery.
+ *
+ * The record is cleared at the start of every `discoverExternalMcp` run and then unioned
+ * within that run, because one server is resolved twice — once for `headers` and once for
+ * `environment`. Without the reset the map only ever grew: a server whose `{env:VAR}` had
+ * since been fixed kept its old entry (the recording site below is inside an
+ * `unresolvedNames.length > 0` guard, so a clean run never touched it), and `/mcps` went on
+ * telling the user to set a variable that already resolved.
+ *
+ * Only the latest run's servers are present, so a daemon that discovers for a second project
+ * replaces the first project's entries rather than mixing the two under a shared server name.
+ */
+export function unresolvedEnvVars(server: string): string[] {
+  return [...(_unresolvedEnv.get(server) ?? [])].sort()
+}
+
+/** Drop the previous run's records. Called once per `discoverExternalMcp`. */
+function resetUnresolvedEnv() {
+  _unresolvedEnv.clear()
+}
+// altimate_change end
+
 interface ExternalMcpSource {
   /** Relative path from base directory */
   file: string
@@ -302,6 +338,8 @@ export async function discoverExternalMcp(projectDir: string): Promise<{
   sources: string[]
 }> {
   log.info("Discovering MCP servers from external AI tool configs...")
+  // Start from a clean slate so a variable fixed since the last run stops being reported.
+  resetUnresolvedEnv()
   const result: Record<string, ConfigMCPV1.Info> = Object.create(null)
   const contributingSources: string[] = []
   const homedir = os.homedir()

packages/opencode/src/mcp/index.ts

@@ -143,6 +143,24 @@ export const Status = Schema.Union([
 ]).annotate({ identifier: "MCPStatus", discriminator: "status" })
 export type Status = Schema.Schema.Type<typeof Status>
 
+// altimate_change start — upstream_fix: do not swallow the connect error (#1121).
+// The failure path already carries the real message — `401 Unauthorized`, a transport
+// error, `Invalid MCP URL for "<key>"` — in `status.error`, but the warning logged only
+// `status.status`, which is the constant string "failed". An external user had to read
+// this source to find out why their server would not connect.
+//
+// Split out as a pure function so the payload is testable without standing up a
+// transport, and so a future edit cannot quietly drop the field again.
+export function unavailableLogFields(
+  key: string,
+  type: string,
+  status: Status,
+): { key: string; type: string; status: string; error?: string } {
+  const error = "error" in status && typeof status.error === "string" ? status.error : undefined
+  return error ? { key, type, status: status.status, error } : { key, type, status: status.status }
+}
+// altimate_change end
+
 // Store transports for OAuth servers to allow finishing auth
 type TransportWithAuth = StreamableHTTPClientTransport | SSEClientTransport
 const pendingOAuthTransports = new Map<string, TransportWithAuth>()
@@ -627,7 +645,9 @@ export const layer = Layer.effect(
 
         if (!mcpClient) {
           if (status.status !== "connected" && status.status !== "disabled") {
-            yield* Effect.logWarning("server unavailable", { key, type: mcp.type, status: status.status })
+            // altimate_change start — upstream_fix: include the real error (#1121).
+            yield* Effect.logWarning("server unavailable", unavailableLogFields(key, mcp.type, status))
+            // altimate_change end
           }
           return { status } satisfies CreateResult
         }

packages/opencode/src/session/prompt.ts

@@ -34,6 +34,11 @@ import PROMPT_PLAN from "../session/prompt/plan.txt"
 import BUILD_SWITCH from "../session/prompt/build-switch.txt"
 import MAX_STEPS from "../session/prompt/max-steps.txt"
 import { defer } from "../util/defer"
+// altimate_change — upstream_fix (#701): unresolved-env record for the /mcps view.
+import * as McpDiscover from "../mcp/discover"
+// altimate_change start — upstream_fix (#701): file-scoped blank-variable diagnostics.
+import { ConfigVariable } from "../config/variable"
+// altimate_change end
 import { ToolRegistry } from "../tool/registry"
 import { MCP } from "../mcp"
 import { LSP } from "../lsp"
@@ -2894,11 +2899,29 @@ NOTE: At any point in time through this workflow you should feel free to ask the
 
   // altimate_change start — shared text formatter for /mcps runtime status (#972)
   /** @internal Exported for tests. */
-  export function formatMcpStatusForDisplay(name: string, status: MCP.Status) {
+  // altimate_change start — upstream_fix (#701): exported so the wording is testable without
+  // standing up a session; `/mcps` is otherwise only reachable through the whole handler.
+  /** File-scoped blank-variable lines for `/mcps`, empty string when there are none. */
+  export function formatBlankedEnvForDisplay(entries: { source: string; names: string[] }[]): string {
+    return entries
+      .map(({ source, names }) => "- `" + names.join(", ") + "` resolved to empty in `" + source + "` (set or remove)")
+      .join("\n")
+  }
+  // altimate_change end
+
+  export function formatMcpStatusForDisplay(name: string, status: MCP.Status, unresolvedEnv: string[] = []) {
     const icon = status.status === "connected" ? "\u2713" : "\u25cb"
-    if (status.status === "failed") return icon + " " + status.status + " (" + status.error + ")"
-    if (status.status === "needs_auth") return icon + " Needs authentication (run: altimate mcp auth " + name + ")"
-    return icon + " " + status.status
+    // upstream_fix (#701): a server whose `${VAR}` did not resolve launched with that value
+    // blank — most often a password. It then fails with a downstream error naming neither the
+    // variable nor the config file, and the only trace is a log line nobody opens. Say it here,
+    // where the user is already looking, and say it even when the server appears connected: a
+    // blank credential often connects and fails on first use.
+    const blanks =
+      unresolvedEnv.length > 0 ? " \u2014 unresolved: " + unresolvedEnv.join(", ") + " (set or remove)" : ""
+    if (status.status === "failed") return icon + " " + status.status + " (" + status.error + ")" + blanks
+    if (status.status === "needs_auth")
+      return icon + " Needs authentication (run: altimate mcp auth " + name + ")" + blanks
+    return icon + " " + status.status + blanks
   }
   // altimate_change end
 
@@ -2953,11 +2976,20 @@ NOTE: At any point in time through this workflow you should feel free to ask the
         const model = await lastModel(input.sessionID)
         const statusMap = await MCP.status()
         const rows = Object.entries(statusMap)
-          .map(([srv, s]) => "| `" + srv + "` | " + formatMcpStatusForDisplay(srv, s) + " |")
+          .map(
+            ([srv, s]) =>
+              "| `" + srv + "` | " + formatMcpStatusForDisplay(srv, s, McpDiscover.unresolvedEnvVars(srv)) + " |",
+          )
           .join("\n")
-        const responseText = rows
-          ? "MCP servers:\n\n| Server | Status |\n|---|---|\n" + rows
-          : "No MCP servers configured."
+        // altimate_change start — upstream_fix (#701): `/mcps` showed only the per-server
+        // unresolved variables from discovery, while `mcp list` also reported file-scoped blanks.
+        // A server templated as `"url": "https://{env:MY_HOST}/mcp"` records against the config
+        // file rather than the server, so it appeared in the CLI and not here — in the session
+        // view, which is where someone is when a server will not connect.
+        const blanked = formatBlankedEnvForDisplay(ConfigVariable.blankedEnvVars())
+        const table = rows ? "MCP servers:\n\n| Server | Status |\n|---|---|\n" + rows : "No MCP servers configured."
+        const responseText = blanked ? table + "\n\n" + blanked : table
+        // altimate_change end
 
         return respond(userMsg.info.id, responseText, model)
       }