Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions packages/opencode/src/cli/cmd/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ 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`.
// altimate_change start — upstream_fix: diagnostics surfaced by `mcp status` (#701, #878).
import * as McpDiscover from "../../mcp/discover"
import { ConfigVariable } from "../../config/variable"
// altimate_change end
Expand Down Expand Up @@ -108,6 +108,12 @@ export const McpCommand = cmd({
yargs
.command(McpAddCommand)
.command(McpListCommand)
// altimate_change start — upstream_fix (#790): `status` is the name people reach for when a
// server will not connect, and it was the one name that did not exist. It shares the list
// handler rather than duplicating a view that already probes live and already prints the
// failure reason; a `list` alias would widen yargs' alias column and rewrap sibling rows.
.command({ ...McpListCommand, command: "status", aliases: [], describe: "show MCP server health" })
// altimate_change end
.command(McpAuthCommand)
.command(McpLogoutCommand)
// altimate_change start — restore `mcp remove` removed during v1.4.0 bridge merge
Expand Down Expand Up @@ -179,17 +185,23 @@ export const McpListCommand = effectCmd({
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.
// altimate_change start — upstream_fix (#878): discovery is first-source-wins, so a server
// already in altimate-code.json is skipped and a changed .vscode/mcp.json is never mentioned.
// The configured value still wins; this only says the two disagree and where to look.
for (const { server, source, fields } of McpDiscover.configDrift()) {
prompts.log.warn(`${server} differs from ${source}: ${fields.join(", ")} (config wins)`)
}

// 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)`)
}
Expand Down
9 changes: 8 additions & 1 deletion packages/opencode/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,9 @@ export const layer = Layer.effect(
const autoMcpDiscovery = (result.experimental as { auto_mcp_discovery?: boolean } | undefined)
?.auto_mcp_discovery
if (!Flag.OPENCODE_DISABLE_PROJECT_CONFIG && autoMcpDiscovery !== false) {
const { discoverExternalMcp, setDiscoveryResult } = yield* Effect.promise(() => import("../mcp/discover"))
const { discoverExternalMcp, setDiscoveryResult, driftFields, setConfigDrift } = yield* Effect.promise(
() => import("../mcp/discover"),
)
const { servers: externalMcp, sources } = yield* Effect.promise(() => discoverExternalMcp(ctx.directory))
if (Object.keys(externalMcp).length > 0) {
result.mcp ??= {}
Expand All @@ -733,6 +735,11 @@ export const layer = Layer.effect(
if (!(name in result.mcp)) {
;(result.mcp as Record<string, any>)[name] = server
added.push(name)
} else {
// altimate_change — upstream_fix (#878): the user's config still wins, but the
// difference is recorded so a surface can report it rather than silently skipping.
const configured = (result.mcp as Record<string, any>)[name]
setConfigDrift(name, sources.join(", "), driftFields(server as Record<string, any>, configured))
}
}
setDiscoveryResult(added, sources)
Expand Down
54 changes: 53 additions & 1 deletion packages/opencode/src/mcp/discover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ function resolveServerEnvVars(
}
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. */
Expand All @@ -56,6 +55,59 @@ const _unresolvedEnv = new Map<string, Set<string>>()
export function unresolvedEnvVars(server: string): string[] {
return [...(_unresolvedEnv.get(server) ?? [])].sort()
}

// altimate_change start — upstream_fix (#878): report drift instead of silently skipping.
// Discovery is first-source-wins, so a server already present in altimate-code.json is skipped
// outright and a changed `.vscode/mcp.json` (a new ALTIMATE_EXTENSION_RPC port, a moved command)
// is never mentioned. Overwriting the user's own config would be worse than the silence, so the
// differing field names are recorded and a user surface reports them; the user decides.
const _drift = new Map<string, { source: string; fields: string[] }>()

/** Fields whose difference is expected and not worth reporting. */
const DRIFT_IGNORED = new Set(["enabled"])

/**
* Field names that differ between a discovered server and the one already configured.
* Nested `environment`/`headers` differences are reported per key (`environment.FOO`) so the
* message names the thing to fix rather than just "environment".
*/
export function driftFields(discovered: Record<string, any>, configured: Record<string, any>): string[] {
const fields: string[] = []
for (const key of new Set([...Object.keys(discovered), ...Object.keys(configured)])) {
if (DRIFT_IGNORED.has(key)) continue
const a = discovered[key]
const b = configured[key]
const nested = key === "environment" || key === "headers"
if (nested && a && b && typeof a === "object" && typeof b === "object") {
for (const inner of new Set([...Object.keys(a), ...Object.keys(b)])) {
if (a[inner] !== b[inner]) fields.push(`${key}.${inner}`)
}
continue
}
if (JSON.stringify(a) !== JSON.stringify(b)) fields.push(key)
}
return fields.sort()
}

/** Record that `server` is configured differently from what discovery found in `source`. */
export function setConfigDrift(server: string, source: string, fields: string[]) {
if (fields.length > 0) _drift.set(server, { source, fields })
else _drift.delete(server)
}

/** Servers whose configured definition differs from the discovered one. */
export function configDrift(): { server: string; source: string; fields: string[] }[] {
return [..._drift.entries()]
.map(([server, info]) => ({ server, ...info }))
.sort((a, b) => a.server.localeCompare(b.server))
}

/** Test seam — drift accumulates at module level. */
export function resetConfigDrift() {
_drift.clear()
}
// altimate_change end
// altimate_change end
// altimate_change end

interface ExternalMcpSource {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ manage MCP (Model Context Protocol) servers
Commands:
altimate-code mcp add add an MCP server
altimate-code mcp list list MCP servers and their status [aliases: ls]
altimate-code mcp status show MCP server health
altimate-code mcp auth [name] authenticate with an OAuth-enabled MCP server
altimate-code mcp logout [name] remove OAuth credentials for an MCP server
altimate-code mcp remove <name> remove an MCP server [aliases: rm]
Expand Down
149 changes: 149 additions & 0 deletions packages/opencode/test/cli/mcp-status.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
// altimate_change start — upstream_fix (#790, #878): `mcp status` must exist, and a configured
// server that has drifted from the discovered config must be reported. Both are user-facing CLI
// behaviour, so this drives the real binary in an isolated HOME rather than the handler.
// The env-variable reporting these views share is covered against `mcp list` in
// test/cli/mcp-env-diagnostics.test.ts and is not duplicated here.
import { describe, expect, test } from "bun:test"
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "fs"
import { tmpdir } from "os"
import path from "path"
import { spawnSync } from "child_process"

// Each test boots the real CLI in a subprocess; the default 5s budget is not enough.
const SUBPROCESS_TIMEOUT_MS = 120_000

const repoRoot = path.resolve(import.meta.dir, "..", "..", "..", "..")
const opencodeDir = path.join(repoRoot, "packages", "opencode")
const cliEntry = path.join(opencodeDir, "src", "index.ts")

function withIsolatedCli(
mcp: Record<string, unknown>,
fn: (output: (args: string[]) => string) => void,
extraFiles: Record<string, string> = {},
) {
const root = mkdtempSync(path.join(tmpdir(), "altimate-mcp-status-"))
const home = path.join(root, "home")
const configHome = path.join(root, "config")
const configDir = path.join(configHome, "altimate-code")
mkdirSync(home, { recursive: true })
mkdirSync(configDir, { recursive: true })
writeFileSync(path.join(configDir, "altimate-code.json"), JSON.stringify({ mcp }), "utf-8")
for (const [rel, content] of Object.entries(extraFiles)) {
const target = path.join(root, rel)
mkdirSync(path.dirname(target), { recursive: true })
writeFileSync(target, content, "utf-8")
}

const run = (args: string[]) =>
// `bun run --cwd <pkg>` would make the CLI's working directory the repo package, so it would
// read the repo's own .opencode config and never see this temp project. Spawn cwd is the
// project instead; module resolution still follows cliEntry's location.
spawnSync("bun", ["--conditions=browser", cliEntry, ...args], {
cwd: root,
encoding: "utf-8",
timeout: 90_000,
env: {
...process.env,
HOME: home,
XDG_CONFIG_HOME: configHome,
XDG_DATA_HOME: path.join(root, "data"),
XDG_CACHE_HOME: path.join(root, "cache"),
XDG_STATE_HOME: path.join(root, "state"),
OPENCODE_DISABLE_TELEMETRY: "1",
OPENCODE_DISABLE_SHARE: "1",
OPENCODE_DISABLE_AUTOUPDATE: "1",
OPENCODE_DISABLE_AUTOCOMPACT: "1",
OPENCODE_DISABLE_MODELS_FETCH: "1",
OPENCODE_PURE: "1",
TERM: "dumb",
CI: "1",
},
})

const output = (args: string[]) => {
const r = run(args)
return String(r.stdout ?? "") + String(r.stderr ?? "")
}

try {
fn(output)
} finally {
rmSync(root, { recursive: true, force: true })
}
}

const brokenServer = {
broken: {
type: "local",
command: ["/nonexistent-binary-for-mcp-status-test"],
environment: { API_TOKEN: "{env:ALTIMATE_TEST_VAR_THAT_IS_NEVER_SET}" },
enabled: true,
},
}

describe("altimate-code mcp status", () => {
test(
"`status` reaches the server listing",
() =>
withIsolatedCli(brokenServer, (output) => {
const out = output(["mcp", "status"])
expect(out, out).toContain("broken")
expect(out, out).not.toContain("Unknown argument")
}),
SUBPROCESS_TIMEOUT_MS,
)
})
// altimate_change end

// altimate_change start — upstream_fix (#878): drift must reach the user, not just the record.
describe("altimate-code mcp status — discovered config drift", () => {
const configured = {
datamate: {
type: "local",
command: ["/nonexistent-binary-for-mcp-status-test"],
environment: { ALTIMATE_EXTENSION_RPC: "127.0.0.1:9000" },
enabled: true,
},
}
const vscode = (rpc: string) =>
JSON.stringify({
servers: {
datamate: {
type: "stdio",
command: "/nonexistent-binary-for-mcp-status-test",
env: { ALTIMATE_EXTENSION_RPC: rpc },
},
},
})

test(
"reports the field that drifted from the discovered config",
() =>
withIsolatedCli(
configured,
(output) => {
const out = output(["mcp", "status"])
expect(out, out).toContain("datamate")
expect(out, out).toContain("environment.ALTIMATE_EXTENSION_RPC")
},
{ ".vscode/mcp.json": vscode("127.0.0.1:9999") },
),
SUBPROCESS_TIMEOUT_MS,
)

test(
"says nothing when the discovered config agrees",
() =>
withIsolatedCli(
configured,
(output) => {
const out = output(["mcp", "status"])
expect(out, out).toContain("datamate")
expect(out, out).not.toContain("environment.ALTIMATE_EXTENSION_RPC")
},
{ ".vscode/mcp.json": vscode("127.0.0.1:9000") },
),
SUBPROCESS_TIMEOUT_MS,
)
})
// altimate_change end
53 changes: 53 additions & 0 deletions packages/opencode/test/mcp/config-drift.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// altimate_change start — upstream_fix (#878): discovery skipped already-configured servers
// without a word, so a changed .vscode/mcp.json never surfaced. These pin what counts as drift.
import { describe, expect, test, beforeEach } from "bun:test"
import { driftFields, setConfigDrift, configDrift, resetConfigDrift } from "../../src/mcp/discover"

describe("driftFields", () => {
test("identical definitions report no drift", () => {
const server = { type: "local", command: ["node", "server.js"], environment: { PORT: "1" } }
expect(driftFields({ ...server }, { ...server })).toEqual([])
})

test("names the environment key that changed, not just `environment`", () => {
const discovered = { type: "local", environment: { ALTIMATE_EXTENSION_RPC: "127.0.0.1:9001", KEEP: "same" } }
const configured = { type: "local", environment: { ALTIMATE_EXTENSION_RPC: "127.0.0.1:9000", KEEP: "same" } }
expect(driftFields(discovered, configured)).toEqual(["environment.ALTIMATE_EXTENSION_RPC"])
})

test("reports a key present on only one side", () => {
expect(driftFields({ environment: { A: "1", B: "2" } }, { environment: { A: "1" } })).toEqual(["environment.B"])
})

test("compares command arrays by value, not identity", () => {
expect(driftFields({ command: ["node", "a.js"] }, { command: ["node", "a.js"] })).toEqual([])
expect(driftFields({ command: ["node", "a.js"] }, { command: ["node", "b.js"] })).toEqual(["command"])
})

test("ignores `enabled`, which discovery sets for its own reasons", () => {
expect(driftFields({ type: "local", enabled: false }, { type: "local", enabled: true })).toEqual([])
})

test("reports a changed url", () => {
expect(driftFields({ url: "https://a" }, { url: "https://b" })).toEqual(["url"])
})
})

describe("configDrift record", () => {
beforeEach(() => resetConfigDrift())

test("records only servers that actually differ", () => {
setConfigDrift("datamate", ".vscode/mcp.json", ["environment.ALTIMATE_EXTENSION_RPC"])
setConfigDrift("clean", ".vscode/mcp.json", [])
expect(configDrift()).toEqual([
{ server: "datamate", source: ".vscode/mcp.json", fields: ["environment.ALTIMATE_EXTENSION_RPC"] },
])
})

test("a server that stops drifting is dropped from the report", () => {
setConfigDrift("datamate", ".vscode/mcp.json", ["url"])
setConfigDrift("datamate", ".vscode/mcp.json", [])
expect(configDrift()).toEqual([])
})
})
// altimate_change end
Loading