fix: judge plugin requiresEnv against the runtime environment

packages/coding-agent/src/step/mcp-environment.ts

@@ -0,0 +1,39 @@
+/**
+ * Environment resolution shared by the MCP runtime and the plugin doctor.
+ *
+ * It lives apart from `./mcp.ts` because `mcp.ts` imports `./plugins.ts`, so a
+ * plugin-side import of the runtime resolver would close a module cycle. Both
+ * sides must answer "does this server have a credential?" the same way: a
+ * doctor with its own rule reports a plugin as broken while the server it
+ * describes starts fine.
+ */
+
+import { readStoredCredential } from "../core/auth-storage.ts";
+import { getStepAuthPath } from "./auth.ts";
+
+/**
+ * The variables a Step login can supply on its own, so callers can tell a user
+ * whose only gap is `/login` apart from one who has to configure a variable
+ * StepCode knows nothing about.
+ */
+export const STEP_LOGIN_SUPPLIED_ENV: readonly string[] = ["STEPFUN_API_KEY"];
+
+/** Resolve the environment passed to a plugin server, including Step login fallback. */
+export function resolveStepMcpEnvironment(
+	declared: Record<string, string> | undefined,
+	input: { env?: NodeJS.ProcessEnv; authPath?: string } = {},
+): Record<string, string> {
+	const resolved: Record<string, string> = {};
+	for (const [key, value] of Object.entries(input.env ?? process.env)) if (value !== undefined) resolved[key] = value;
+	Object.assign(resolved, declared ?? {});
+	if (!resolved.STEPFUN_API_KEY?.trim()) {
+		const credential = readStoredCredential("step", input.authPath ?? getStepAuthPath());
+		if (credential?.type === "oauth" && typeof credential.access === "string" && credential.access.trim()) {
+			resolved.STEPFUN_API_KEY = credential.access;
+		}
+		if (credential?.type === "api_key" && typeof credential.key === "string" && credential.key.trim()) {
+			resolved.STEPFUN_API_KEY = credential.key;
+		}
+	}
+	return resolved;
+}

packages/coding-agent/src/step/mcp.ts

@@ -6,11 +6,10 @@ import { StreamableHTTPClientTransport, StreamableHTTPError } from "@modelcontex
 import { CallToolResultSchema, type Tool as McpTool } from "@modelcontextprotocol/sdk/types.js";
 import type { AgentToolResult } from "@step-harness/agent-core";
 import { type TSchema, Type } from "typebox";
-import { readStoredCredential } from "../core/auth-storage.ts";
 import type { ExtensionAPI, ExtensionFactory } from "../core/extensions/types.ts";
 import { theme } from "../theme/theme.ts";
-import { getStepAuthPath } from "./auth.ts";
 import { readGlobalStepConfig } from "./config-toml.ts";
+import { resolveStepMcpEnvironment } from "./mcp-environment.ts";
 import { createStoredMcpOAuthProvider, hasStoredMcpOAuthCredential } from "./mcp-oauth.ts";
 import {
 	defaultStepPluginsDir,
@@ -23,6 +22,8 @@ import {
 } from "./plugins.ts";
 import { STEPCODE_VERSION } from "./version.ts";
 
+export { resolveStepMcpEnvironment } from "./mcp-environment.ts";
+
 const MCP_STARTUP_TIMEOUT_SEC = 30;
 const MCP_CALL_TIMEOUT_SEC = 300;
 const CLIENT_INFO = { name: "step-harness", version: STEPCODE_VERSION.value } as const;
@@ -409,26 +410,6 @@ function isMissingExecutable(error: unknown): boolean {
 	return error instanceof Error && /\bENOENT\b/u.test(error.message);
 }
 
-/** Resolve the environment passed to a plugin server, including Step login fallback. */
-export function resolveStepMcpEnvironment(
-	declared: Record<string, string> | undefined,
-	input: { env?: NodeJS.ProcessEnv; authPath?: string } = {},
-): Record<string, string> {
-	const resolved: Record<string, string> = {};
-	for (const [key, value] of Object.entries(input.env ?? process.env)) if (value !== undefined) resolved[key] = value;
-	Object.assign(resolved, declared ?? {});
-	if (!resolved.STEPFUN_API_KEY?.trim()) {
-		const credential = readStoredCredential("step", input.authPath ?? getStepAuthPath());
-		if (credential?.type === "oauth" && typeof credential.access === "string" && credential.access.trim()) {
-			resolved.STEPFUN_API_KEY = credential.access;
-		}
-		if (credential?.type === "api_key" && typeof credential.key === "string" && credential.key.trim()) {
-			resolved.STEPFUN_API_KEY = credential.key;
-		}
-	}
-	return resolved;
-}
-
 interface McpCallResult {
 	content?: Array<{ type?: string; text?: string; data?: string; mimeType?: string }>;
 	structuredContent?: unknown;

packages/coding-agent/src/step/plugins.ts

@@ -16,6 +16,7 @@ import { fileURLToPath } from "node:url";
 import { promisify } from "node:util";
 import type { ExtensionAPI, ExtensionCommandContext } from "../core/extensions/types.ts";
 import { resolveStepConfigDir } from "./environment.ts";
+import { resolveStepMcpEnvironment, STEP_LOGIN_SUPPLIED_ENV } from "./mcp-environment.ts";
 import { resolveStepStorageRoot } from "./storage-root.ts";
 import { type StepTelemetryReporter, trackStepTelemetry } from "./telemetry.ts";
 
@@ -605,7 +606,10 @@ export async function uninstallPlugin(pluginsDir: string, name: string): Promise
 }
 
 /** Read MCP declarations without starting a process. */
-export async function diagnoseStepPlugin(pluginDir: string): Promise<StepPluginDiagnostics> {
+export async function diagnoseStepPlugin(
+	pluginDir: string,
+	options: { env?: NodeJS.ProcessEnv; authPath?: string } = {},
+): Promise<StepPluginDiagnostics> {
 	const read = await readStepPluginManifest(pluginDir);
 	if (read.errors.length > 0) return { mcpServers: [], warnings: [...read.errors] };
 	if (!read.manifest) return { mcpServers: [], warnings: [`No ${STEP_PLUGIN_MANIFEST_FILE} found in ${pluginDir}.`] };
@@ -640,15 +644,58 @@ export async function diagnoseStepPlugin(pluginDir: string): Promise<StepPluginD
 	}
 	if (read.manifest.entry)
 		warnings.push("Executable plugin entries are recorded but not loaded by the Step marketplace facade.");
-	const missingEnvironment = (read.manifest.provision?.requiresEnv ?? []).filter((name) => !process.env[name]?.trim());
-	if (missingEnvironment.length > 0) {
-		warnings.push(
-			`Plugin provisioning has no shell value for ${missingEnvironment.join(", ")}; a Step login credential can supply it at runtime.`,
+	// Judged against the environment the matching servers are actually spawned
+	// with: `connectStepMcpServer` layers the process environment, the server's
+	// own declared `env`, and the Step login credential. Checking `process.env`
+	// alone reported every logged-in user as missing a variable they were never
+	// expected to export by hand. Only an inline `mcpServers` record can start a
+	// server — discovery skips a string declaration path — so that is the only
+	// shape whose declared `env` can satisfy a requirement.
+	const requiredEnvironment = read.manifest.provision?.requiresEnv ?? [];
+	if (requiredEnvironment.length > 0) {
+		const candidates = provisionedServerEnvironments(read.manifest).map((declared) =>
+			resolveStepMcpEnvironment(declared, options),
+		);
+		const missingEnvironment = requiredEnvironment.filter((name) =>
+			candidates.every((candidate) => !candidate[name]?.trim()),
 		);
+		// A Step login only ever supplies its own credential, so pointing at
+		// `/login` for an unrelated variable would send the user nowhere.
+		const missingLogin = missingEnvironment.filter((name) => STEP_LOGIN_SUPPLIED_ENV.includes(name));
+		const missingOther = missingEnvironment.filter((name) => !STEP_LOGIN_SUPPLIED_ENV.includes(name));
+		if (missingLogin.length > 0) {
+			warnings.push(
+				`Plugin provisioning has no value for ${missingLogin.join(", ")}; run /login or export it before using this plugin.`,
+			);
+		}
+		if (missingOther.length > 0) {
+			warnings.push(
+				`Plugin provisioning has no value for ${missingOther.join(", ")}; export it or declare it in the plugin's mcpServers env before using this plugin.`,
+			);
+		}
 	}
 	return { mcpServers, warnings };
 }
 
+/**
+ * The declared environments of the servers a manifest's provisioning installs,
+ * matched on the provisioned command. Returns a single `undefined` when no
+ * server matches, so the caller still judges the requirement against the
+ * process environment and the login fallback.
+ */
+function provisionedServerEnvironments(manifest: StepPluginManifest): Array<Record<string, string> | undefined> {
+	const provisionCommand = manifest.provision?.command;
+	if (!provisionCommand || !isRecord(manifest.mcpServers)) return [undefined];
+	const declared = Object.values(manifest.mcpServers).flatMap((declaration) => {
+		if (!isRecord(declaration) || declaration.command !== provisionCommand) return [];
+		if (!isRecord(declaration.env)) return [undefined];
+		const env: Record<string, string> = {};
+		for (const [key, value] of Object.entries(declaration.env)) if (typeof value === "string") env[key] = value;
+		return [env];
+	});
+	return declared.length > 0 ? declared : [undefined];
+}
+
 export async function listInstalledStepPlugins(
 	input: { userDir?: string; projectDir?: string } = {},
 ): Promise<{ plugins: InstalledStepPlugin[]; warnings: string[] }> {

packages/coding-agent/test/step-plugins.test.ts

@@ -130,6 +130,79 @@ describe("Step plugin marketplace facade", () => {
 		expect(afterUninstall.plugins.map((plugin) => plugin.id)).not.toContain("steppage");
 	});
 
+	test("treats a Step login credential as satisfying a provisioned environment requirement", async () => {
+		const root = await mkdtemp(join(tmpdir(), "step-plugins-requires-env-"));
+		roots.push(root);
+		const pluginDir = join(root, "steppage");
+		await mkdir(pluginDir, { recursive: true });
+		await writeFile(
+			join(pluginDir, "step.plugin.json"),
+			JSON.stringify({
+				id: "steppage",
+				provision: {
+					command: "steppage-mcp",
+					installer: "https://example.invalid/i.sh",
+					requiresEnv: ["STEPFUN_API_KEY"],
+				},
+			}),
+		);
+		const authPath = join(root, "auth.json");
+		await writeFile(
+			authPath,
+			JSON.stringify({ step: { type: "oauth", access: "login-key", refresh: "r", expires: 0 } }),
+		);
+
+		// A logged-in user exports nothing by hand: the credential on disk is what
+		// the server is spawned with, so the doctor must not report it as missing.
+		const loggedIn = await diagnoseStepPlugin(pluginDir, { env: {}, authPath });
+		expect(loggedIn.warnings.join(" ")).not.toContain("STEPFUN_API_KEY");
+
+		// With neither a shell value nor a credential the warning is real advice.
+		const loggedOut = await diagnoseStepPlugin(pluginDir, { env: {}, authPath: join(root, "absent.json") });
+		expect(loggedOut.warnings.join(" ")).toContain("STEPFUN_API_KEY");
+		expect(loggedOut.warnings.join(" ")).toContain("/login");
+	});
+
+	test("accepts a requirement satisfied by the provisioned server's own declared env", async () => {
+		const root = await mkdtemp(join(tmpdir(), "step-plugins-declared-env-"));
+		roots.push(root);
+		const pluginDir = join(root, "declared");
+		await mkdir(pluginDir, { recursive: true });
+		await writeFile(
+			join(pluginDir, "step.plugin.json"),
+			JSON.stringify({
+				id: "declared",
+				mcpServers: { declared: { command: "steppage-mcp", env: { STEPFUN_API_KEY: "declared-key" } } },
+				provision: { command: "steppage-mcp", requiresEnv: ["STEPFUN_API_KEY"] },
+			}),
+		);
+
+		// The runtime layers the server's declared env over the process env, so a
+		// manifest that carries its own key needs neither a shell value nor a login.
+		const diagnostics = await diagnoseStepPlugin(pluginDir, { env: {}, authPath: join(root, "absent.json") });
+		expect(diagnostics.warnings.join(" ")).not.toContain("STEPFUN_API_KEY");
+	});
+
+	test("points a non-login variable at configuration rather than /login", async () => {
+		const root = await mkdtemp(join(tmpdir(), "step-plugins-other-env-"));
+		roots.push(root);
+		const pluginDir = join(root, "other");
+		await mkdir(pluginDir, { recursive: true });
+		await writeFile(
+			join(pluginDir, "step.plugin.json"),
+			JSON.stringify({
+				id: "other",
+				mcpServers: { other: { command: "other-mcp" } },
+				provision: { command: "other-mcp", requiresEnv: ["GITHUB_TOKEN"] },
+			}),
+		);
+
+		// A Step login cannot supply someone else's token, so it must not be the advice.
+		const diagnostics = await diagnoseStepPlugin(pluginDir, { env: {}, authPath: join(root, "absent.json") });
+		expect(diagnostics.warnings.join(" ")).toContain("GITHUB_TOKEN");
+		expect(diagnostics.warnings.join(" ")).not.toContain("/login");
+	});
+
 	test("does not overwrite a Claude-style plugin manifest", async () => {
 		const root = await mkdtemp(join(tmpdir(), "step-plugins-claude-"));
 		roots.push(root);