diff --git a/docs/configuration.md b/docs/configuration.md index 93a3d4fa..61c5f6ef 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -101,8 +101,7 @@ MCP clients discover metadata from: `DEVSPACE_MINIMAL_TOOLS` remains a backward-compatible alias when `DEVSPACE_TOOL_MODE` is unset: `1` selects `minimal` and `0` selects `full`. -The `codex` mode must be selected through `DEVSPACE_TOOL_MODE` and always uses -its fixed short tool names regardless of `DEVSPACE_TOOL_NAMING`. +The `codex` mode must be selected through `DEVSPACE_TOOL_MODE`. Codex-mode commands run without a PTY by default. Set `tty: true` on `exec_command` for interactive terminal programs. PTY support uses the optional diff --git a/src/config.test.ts b/src/config.test.ts index 7b3eeeb6..1f9d9772 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -3,6 +3,7 @@ import { mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { loadConfig } from "./config.js"; +import { loadDevspaceFiles } from "./user-config.js"; const emptyConfigDir = mkdtempSync(join(tmpdir(), "devspace-empty-config-test-")); const baseEnv = { @@ -54,6 +55,17 @@ assert.throws( () => loadConfig({ ...baseEnv, DEVSPACE_TOOL_MODE: "invalid" }), /Invalid DEVSPACE_TOOL_MODE: invalid/, ); +for (const [name, value] of [ + ["DEVSPACE_ARTIFACTS", "treu"], + ["DEVSPACE_SKILLS", "maybe"], + ["DEVSPACE_MINIMAL_TOOLS", "sometimes"], + ["DEVSPACE_LOG_REQUESTS", "enabled"], +] as const) { + assert.throws( + () => loadConfig({ ...baseEnv, [name]: value }), + new RegExp(`Invalid ${name}: ${value}`), + ); +} assert.deepEqual(loadConfig(baseEnv).logging, { level: "info", @@ -186,3 +198,21 @@ assert.deepEqual(fileConfig.allowedHosts, [ "::1", "devspace.example.com", ]); + +const passthroughConfigDir = mkdtempSync(join(tmpdir(), "devspace-config-passthrough-test-")); +writeFileSync( + join(passthroughConfigDir, "config.json"), + JSON.stringify({ host: "127.0.0.1", futureSetting: { enabled: true } }), +); +assert.deepEqual( + (loadDevspaceFiles({ DEVSPACE_CONFIG_DIR: passthroughConfigDir }).config as Record) + .futureSetting, + { enabled: true }, +); + +const invalidConfigDir = mkdtempSync(join(tmpdir(), "devspace-invalid-config-test-")); +writeFileSync(join(invalidConfigDir, "config.json"), JSON.stringify({ port: "8787" })); +assert.throws( + () => loadDevspaceFiles({ DEVSPACE_CONFIG_DIR: invalidConfigDir }), + /Unable to read .*config\.json:[\s\S]*port/i, +); diff --git a/src/config.ts b/src/config.ts index 54a131c9..a0a4afdc 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,6 +1,7 @@ import { homedir } from "node:os"; import { join, resolve } from "node:path"; import { expandHomePath } from "./roots.js"; +import { parseEnvBoolean } from "./env-config.js"; import type { LoggingConfig, LogFormat, LogLevel } from "./logger.js"; import type { OAuthConfig } from "./oauth-provider.js"; import { devspaceAgentsDir, devspaceSkillsDir, loadDevspaceFiles } from "./user-config.js"; @@ -81,17 +82,13 @@ function normalizeAllowedHosts(rawHosts: string[], derivedHosts: string[]): stri return Array.from(new Set(hosts.map((host) => host.trim()).filter(Boolean))); } -function parseBoolean(value: string | undefined): boolean { - return ["1", "true", "yes", "on"].includes(value?.toLowerCase() ?? ""); -} - function parseToolMode(env: NodeJS.ProcessEnv): ToolMode { const mode = env.DEVSPACE_TOOL_MODE; if (mode === "minimal" || mode === "full" || mode === "codex") return mode; if (mode) throw new Error(`Invalid DEVSPACE_TOOL_MODE: ${mode}`); if (env.DEVSPACE_MINIMAL_TOOLS !== undefined) { - return parseBoolean(env.DEVSPACE_MINIMAL_TOOLS) ? "minimal" : "full"; + return parseEnvBoolean(env.DEVSPACE_MINIMAL_TOOLS, "DEVSPACE_MINIMAL_TOOLS") ? "minimal" : "full"; } return "minimal"; } @@ -148,11 +145,21 @@ function parseLoggingConfig(env: NodeJS.ProcessEnv): LoggingConfig { return { level: parseLogLevel(env.DEVSPACE_LOG_LEVEL), format: parseLogFormat(env.DEVSPACE_LOG_FORMAT), - requests: env.DEVSPACE_LOG_REQUESTS === undefined ? true : parseBoolean(env.DEVSPACE_LOG_REQUESTS), - assets: parseBoolean(env.DEVSPACE_LOG_ASSETS), - toolCalls: env.DEVSPACE_LOG_TOOL_CALLS === undefined ? true : parseBoolean(env.DEVSPACE_LOG_TOOL_CALLS), - shellCommands: parseBoolean(env.DEVSPACE_LOG_SHELL_COMMANDS), - trustProxy: parseBoolean(env.DEVSPACE_TRUST_PROXY), + requests: env.DEVSPACE_LOG_REQUESTS === undefined + ? true + : parseEnvBoolean(env.DEVSPACE_LOG_REQUESTS, "DEVSPACE_LOG_REQUESTS"), + assets: env.DEVSPACE_LOG_ASSETS === undefined + ? false + : parseEnvBoolean(env.DEVSPACE_LOG_ASSETS, "DEVSPACE_LOG_ASSETS"), + toolCalls: env.DEVSPACE_LOG_TOOL_CALLS === undefined + ? true + : parseEnvBoolean(env.DEVSPACE_LOG_TOOL_CALLS, "DEVSPACE_LOG_TOOL_CALLS"), + shellCommands: env.DEVSPACE_LOG_SHELL_COMMANDS === undefined + ? false + : parseEnvBoolean(env.DEVSPACE_LOG_SHELL_COMMANDS, "DEVSPACE_LOG_SHELL_COMMANDS"), + trustProxy: env.DEVSPACE_TRUST_PROXY === undefined + ? false + : parseEnvBoolean(env.DEVSPACE_TRUST_PROXY, "DEVSPACE_TRUST_PROXY"), }; } @@ -238,13 +245,15 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): ServerConfig { artifactsEnabled: env.DEVSPACE_ARTIFACTS === undefined ? files.config.artifactsEnabled === true - : parseBoolean(env.DEVSPACE_ARTIFACTS), + : parseEnvBoolean(env.DEVSPACE_ARTIFACTS, "DEVSPACE_ARTIFACTS"), artifactMaxFileBytes: parsePositiveInteger( env.DEVSPACE_ARTIFACT_MAX_FILE_BYTES ?? numberConfigValue(files.config.artifactMaxFileBytes), DEFAULT_ARTIFACT_MAX_FILE_BYTES, "DEVSPACE_ARTIFACT_MAX_FILE_BYTES", ), - skillsEnabled: env.DEVSPACE_SKILLS === undefined ? true : parseBoolean(env.DEVSPACE_SKILLS), + skillsEnabled: env.DEVSPACE_SKILLS === undefined + ? true + : parseEnvBoolean(env.DEVSPACE_SKILLS, "DEVSPACE_SKILLS"), skillPaths: parsePathList(env.DEVSPACE_SKILL_PATHS), devspaceSkillsDir: devspaceSkillsDir(env), devspaceAgentsDir: devspaceAgentsDir(env), diff --git a/src/env-config.ts b/src/env-config.ts new file mode 100644 index 00000000..1e820a7f --- /dev/null +++ b/src/env-config.ts @@ -0,0 +1,9 @@ +const TRUE_VALUES = new Set(["1", "true", "yes", "on"]); +const FALSE_VALUES = new Set(["0", "false", "no", "off"]); + +export function parseEnvBoolean(value: string, name: string): boolean { + const normalized = value.toLowerCase(); + if (TRUE_VALUES.has(normalized)) return true; + if (FALSE_VALUES.has(normalized)) return false; + throw new Error(`Invalid ${name}: ${value}`); +} diff --git a/src/local-agent-config.test.ts b/src/local-agent-config.test.ts index 713fed65..dbfc173b 100644 --- a/src/local-agent-config.test.ts +++ b/src/local-agent-config.test.ts @@ -28,6 +28,10 @@ assert.equal(resolveSubagentsConfig(config, { DEVSPACE_SUBAGENTS: "0" }).enabled assert.equal(resolveSubagentsConfig({ ...config, enabled: false }, { DEVSPACE_SUBAGENTS: "1", }).enabled, true); +assert.throws( + () => resolveSubagentsConfig(config, { DEVSPACE_SUBAGENTS: "maybe" }), + /Invalid DEVSPACE_SUBAGENTS: maybe/, +); assert.equal(resolveSubagentsConfig(undefined, {}).providers.length, 0); assert.equal(resolveSubagentsConfig(true, {}).providers.length, 7); diff --git a/src/local-agent-config.ts b/src/local-agent-config.ts index 3f1de5aa..9a85a72b 100644 --- a/src/local-agent-config.ts +++ b/src/local-agent-config.ts @@ -1,4 +1,5 @@ import * as z from "zod/v4"; +import { parseEnvBoolean } from "./env-config.js"; import { LOCAL_AGENT_PROVIDERS, type LocalAgentProvider, @@ -30,7 +31,8 @@ const subagentsSchema = z.object({ export type SubagentProviderConfig = z.infer; export type SubagentsConfig = z.infer; -export type StoredSubagentsConfig = boolean | SubagentsConfig; +export const storedSubagentsConfigSchema = z.union([z.boolean(), subagentsSchema]); +export type StoredSubagentsConfig = z.infer; export function resolveSubagentsConfig( value: unknown, @@ -45,7 +47,7 @@ export function resolveSubagentsConfig( ...stored, enabled: env.DEVSPACE_SUBAGENTS === undefined ? stored.enabled - : parseBoolean(env.DEVSPACE_SUBAGENTS), + : parseEnvBoolean(env.DEVSPACE_SUBAGENTS, "DEVSPACE_SUBAGENTS"), }; } @@ -71,7 +73,3 @@ function legacySubagentsConfig(enabled: boolean): SubagentsConfig { : [], }; } - -function parseBoolean(value: string): boolean { - return ["1", "true", "yes", "on"].includes(value.toLowerCase()); -} diff --git a/src/server.test.ts b/src/server.test.ts index cb29d11c..a1585eaa 100644 --- a/src/server.test.ts +++ b/src/server.test.ts @@ -7,7 +7,7 @@ import test, { type TestContext } from "node:test"; import { promisify } from "node:util"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; -import { loadConfig, type ServerConfig } from "./config.js"; +import { loadConfig, type ServerConfig, type ToolMode, type WidgetMode } from "./config.js"; import type { LocalAgentProviderAvailability } from "./local-agent-availability.js"; import { buildLocalAgentProviderStatuses } from "./local-agent-catalog.js"; import type { SubagentsConfig } from "./local-agent-config.js"; @@ -19,6 +19,36 @@ import { WorkspaceRegistry } from "./workspaces.js"; const execFileAsync = promisify(execFile); +test("configured tool modes expose one coherent tool surface", async (t) => { + const expectedByMode: Record = { + minimal: ["bash", "edit", "open_workspace", "read", "write"], + full: ["bash", "edit", "glob", "grep", "ls", "open_workspace", "read", "write"], + codex: ["apply_patch", "exec_command", "open_workspace", "read", "write_stdin"], + }; + + for (const mode of ["minimal", "full", "codex"] as const) { + const context = await fixture(t, { toolMode: mode, widgets: "off" }); + const tools = await context.client.listTools(); + assert.deepEqual( + tools.tools.map((tool) => tool.name).sort(), + expectedByMode[mode], + ); + await context.close(); + } +}); + +test("changes presentation adds only the aggregate review tool", async (t) => { + const full = await fixture(t, { toolMode: "minimal", widgets: "full" }); + const changes = await fixture(t, { toolMode: "minimal", widgets: "changes" }); + const off = await fixture(t, { toolMode: "minimal", widgets: "off" }); + + assert.equal((await full.client.listTools()).tools.some((tool) => tool.name === "show_changes"), false); + assert.equal((await off.client.listTools()).tools.some((tool) => tool.name === "show_changes"), false); + assert.equal((await changes.client.listTools()).tools.some((tool) => tool.name === "show_changes"), true); + + await Promise.all([full.close(), changes.close(), off.close()]); +}); + test("open_workspace keeps lifecycle flags out of model output and preserves complete card metadata", async (t) => { const providerNote = "available"; const context = await fixture(t, { @@ -245,6 +275,8 @@ async function fixture( t: TestContext, options: { git?: boolean; + toolMode?: ToolMode; + widgets?: WidgetMode; localAgentProviders?: LocalAgentProviderAvailability[] | (() => LocalAgentProviderAvailability[]); subagents?: SubagentsConfig; } = {}, @@ -284,8 +316,8 @@ async function fixture( DEVSPACE_ALLOWED_ROOTS: root, DEVSPACE_WORKTREE_ROOT: join(root, ".worktrees"), DEVSPACE_AGENT_DIR: agentDir, - DEVSPACE_WIDGETS: "full", - DEVSPACE_TOOL_MODE: "full", + DEVSPACE_WIDGETS: options.widgets ?? "full", + DEVSPACE_TOOL_MODE: options.toolMode ?? "full", DEVSPACE_SUBAGENTS: options.localAgentProviders ? "1" : "0", DEVSPACE_OAUTH_OWNER_TOKEN: "test-owner-token-that-is-long-enough", PORT: "1", diff --git a/src/user-config.ts b/src/user-config.ts index 98d05ac6..506b468c 100644 --- a/src/user-config.ts +++ b/src/user-config.ts @@ -7,26 +7,30 @@ import { } from "node:fs"; import { homedir } from "node:os"; import { join, resolve } from "node:path"; +import * as z from "zod/v4"; import { expandHomePath } from "./roots.js"; -import type { StoredSubagentsConfig } from "./local-agent-config.js"; - -export interface DevspaceUserConfig { - host?: string; - port?: number; - allowedRoots?: string[]; - publicBaseUrl?: string | null; - allowedHosts?: string[]; - stateDir?: string; - worktreeRoot?: string; - artifactsEnabled?: boolean; - artifactMaxFileBytes?: number; - agentDir?: string; - subagents?: StoredSubagentsConfig; -} +import { storedSubagentsConfigSchema } from "./local-agent-config.js"; -export interface DevspaceAuthConfig { - ownerToken?: string; -} +const devspaceUserConfigSchema = z.object({ + host: z.string().optional(), + port: z.number().optional(), + allowedRoots: z.array(z.string()).optional(), + publicBaseUrl: z.string().nullable().optional(), + allowedHosts: z.array(z.string()).optional(), + stateDir: z.string().optional(), + worktreeRoot: z.string().optional(), + artifactsEnabled: z.boolean().optional(), + artifactMaxFileBytes: z.number().optional(), + agentDir: z.string().optional(), + subagents: storedSubagentsConfigSchema.optional(), +}).passthrough(); + +const devspaceAuthConfigSchema = z.object({ + ownerToken: z.string().optional(), +}).passthrough(); + +export type DevspaceUserConfig = z.infer; +export type DevspaceAuthConfig = z.infer; export interface DevspaceFiles { dir: string; @@ -71,8 +75,8 @@ export function loadDevspaceFiles(env: NodeJS.ProcessEnv = process.env): Devspac authPath, configExists, authExists, - config: configExists ? readJsonFile(configPath) : {}, - auth: authExists ? readJsonFile(authPath) : {}, + config: configExists ? readJsonFile(configPath, devspaceUserConfigSchema) : {}, + auth: authExists ? readJsonFile(authPath, devspaceAuthConfigSchema) : {}, }; } @@ -100,9 +104,9 @@ export function generateOwnerToken(): string { return randomBytes(32).toString("base64url"); } -function readJsonFile(filePath: string): T { +function readJsonFile(filePath: string, schema: z.ZodType): T { try { - return JSON.parse(readFileSync(filePath, "utf8")) as T; + return schema.parse(JSON.parse(readFileSync(filePath, "utf8")) as unknown); } catch (error) { const reason = error instanceof Error ? error.message : String(error); throw new Error(`Unable to read ${filePath}: ${reason}`);