Skip to content
Open
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
5 changes: 5 additions & 0 deletions src/config-field-definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ export const CONFIG_FIELD_DEFINITIONS = {
description: 'Maximum number of lines that can be written in one edit operation. This helps prevent accidental oversized writes and keeps file changes predictable.',
valueType: 'number',
},
showMcpUI: {
label: 'Show MCP UI Widgets',
description: 'Controls whether tools render interactive UI widgets (file preview, config editor) in supported clients. When not set, Desktop Commander decides automatically. Set to true to always show widgets, or false to always use plain text. Note: changes take effect after restarting the app.',
valueType: 'boolean',
},
} as const satisfies Record<string, ConfigFieldDefinition>;

export type ConfigFieldKey = keyof typeof CONFIG_FIELD_DEFINITIONS;
Expand Down
1 change: 1 addition & 0 deletions src/config-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export interface ServerConfig {
defaultShell?: string;
allowedDirectories?: string[];
telemetryEnabled?: boolean; // New field for telemetry control
showMcpUI?: boolean; // Explicit user override for MCP UI widgets; unset = automatic (A/B test decides)
fileWriteLineLimit?: number; // Line limit for file write operations
fileReadLineLimit?: number; // Default line limit for file read operations (changed from character-based)
clientId?: string; // Unique client identifier for analytics
Expand Down
12 changes: 10 additions & 2 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
- fileReadLineLimit (max lines for read_file, default 1000)
- fileWriteLineLimit (max lines per write_file call, default 50)
- telemetryEnabled (boolean for telemetry opt-in/out)
- showMcpUI (boolean — explicit on/off for interactive UI widgets; unset means automatic)
- currentClient (information about the currently connected MCP client)
- clientHistory (history of all clients that have connected)
- version (version of the DesktopCommander)
Expand Down Expand Up @@ -283,8 +284,9 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
- fileReadLineLimit (number, max lines for read_file)
- fileWriteLineLimit (number, max lines per write_file call)
- telemetryEnabled (boolean)

IMPORTANT: Setting allowedDirectories to an empty array ([]) allows full access
- showMcpUI (boolean — set false to disable interactive UI widgets, true to always show them; takes effect after the client app restarts the MCP server)

IMPORTANT: Setting allowedDirectories to an empty array ([]) allows full access
to the entire file system, regardless of the operating system.

${CMD_PREFIX_DESCRIPTION}`,
Expand Down Expand Up @@ -1223,6 +1225,12 @@ server.setRequestHandler(CallToolRequestSchema, async (request: CallToolRequest)
if (name === 'set_config_value' && args && typeof args === 'object' && 'key' in args) {
telemetryData.set_config_value_key_name = (args as any).key;
telemetryData.call_origin = (args as any).origin === 'ui' ? 'ui' : 'llm';
// Capture the value only for showMcpUI so we can tell on vs off
// (boolean key, no path/PII concern). Other config keys may hold
// paths or free text, so we keep tracking key-name only for those.
if ((args as any).key === 'showMcpUI') {
telemetryData.set_config_value_bool = String((args as any).value);
}
Comment on lines +1231 to +1233

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard telemetry capture to booleans only before stringifying.

On Line 1232, String((args as any).value) records any input type for showMcpUI before validation runs. A malformed client payload could leak arbitrary text into telemetry and corrupt the intended boolean metric. Only emit this field when typeof value === 'boolean'.

Suggested patch
-            if ((args as any).key === 'showMcpUI') {
-                telemetryData.set_config_value_bool = String((args as any).value);
-            }
+            if ((args as any).key === 'showMcpUI' && typeof (args as any).value === 'boolean') {
+                telemetryData.set_config_value_bool = String((args as any).value);
+            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if ((args as any).key === 'showMcpUI') {
telemetryData.set_config_value_bool = String((args as any).value);
}
if ((args as any).key === 'showMcpUI' && typeof (args as any).value === 'boolean') {
telemetryData.set_config_value_bool = String((args as any).value);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server.ts` around lines 1231 - 1233, The telemetry capture for the
showMcpUI configuration is not validating the input type before stringifying it.
Add a type guard to verify that the value is actually a boolean using typeof
check before setting telemetryData.set_config_value_bool. Only when the value
passes the boolean type check should you then stringify it and assign it to the
telemetry field. This prevents malformed client payloads from leaking arbitrary
text into the telemetry metric.

}
if (name === 'get_prompts' && args && typeof args === 'object') {
const promptArgs = args as any;
Expand Down
18 changes: 16 additions & 2 deletions src/tools/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { SetConfigValueArgsSchema } from './schemas.js';
import { getSystemInfo } from '../utils/system-info.js';
import { currentClient } from '../server.js';
import { featureFlagManager } from '../utils/feature-flags.js';
import { shouldShowMcpUiPreviews } from '../utils/mcp-ui-ab-test.js';
import { access, readFile } from 'node:fs/promises';
import { constants as fsConstants } from 'node:fs';
import {
Expand Down Expand Up @@ -115,6 +116,8 @@ export async function getConfig() {
}
};
const availableShells = await detectAvailableShells(systemInfo);
// Effective MCP UI decision (override > A/B test > default ON) for editor display.
const effectiveShowMcpUI = await shouldShowMcpUiPreviews();

console.error(`getConfig result: ${JSON.stringify(configWithSystemInfo, null, 2)}`);
return {
Expand All @@ -129,7 +132,13 @@ export async function getConfig() {
},
entries: CONFIG_FIELD_KEYS.map((key) => {
const definition = CONFIG_FIELD_DEFINITIONS[key];
const value = (configWithSystemInfo as Record<string, unknown>)[key];
let value = (configWithSystemInfo as Record<string, unknown>)[key];
// showMcpUI is tri-state (unset = automatic via A/B test). The editor
// renders booleans as a two-state toggle, so when unset show the
// EFFECTIVE decision; flipping the toggle then pins an explicit override.
if (key === 'showMcpUI' && value === undefined) {
value = effectiveShowMcpUI;
}
return {
key,
value,
Expand Down Expand Up @@ -248,10 +257,15 @@ export async function setConfigValue(args: unknown) {
// Get the updated configuration to show the user
const updatedConfig = await configManager.getConfig();
console.error(`setConfigValue: Successfully set ${parsed.data.key} to ${JSON.stringify(valueToStore)}`);
// UI visibility is fixed per session for rendering consistency; the new
// value applies once the client restarts the MCP server.
const restartNote = parsed.data.key === 'showMcpUI'
? '\n\nNote: this setting takes effect after restarting the app (the MCP server keeps its current UI mode for the rest of this session).'
: '';
return {
content: [{
type: "text",
text: `Successfully set ${parsed.data.key} to ${JSON.stringify(valueToStore, null, 2)}\n\nUpdated configuration:\n${JSON.stringify(updatedConfig, null, 2)}`
text: `Successfully set ${parsed.data.key} to ${JSON.stringify(valueToStore, null, 2)}${restartNote}\n\nUpdated configuration:\n${JSON.stringify(updatedConfig, null, 2)}`
}],
};
} catch (saveError: any) {
Expand Down
51 changes: 41 additions & 10 deletions src/ui/config-editor/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,10 @@ export function createConfigEditorController(callTool: ToolCall, trackConfigUiEv

return {
ok: true,
tooltip: {
message: 'Saved',
tone: 'success',
},
};
} catch (error) {
const errorMessage = `Failed to apply value: ${error instanceof Error ? error.message : String(error)}`;
Expand Down Expand Up @@ -586,7 +590,7 @@ function render(container: HTMLElement, controller: ReturnType<typeof createConf
${description ? `<p>${escapeHtml(description)}</p>` : ''}
<p class="setting-summary${summary ? '' : ' hidden'}" data-setting-summary-key="${escapeHtml(entry.key)}">${summary ? escapeHtml(summary) : ''}</p>
</div>
<div class="setting-control">${controlHtml}</div>
<div class="setting-control"><span class="setting-save-status" data-save-status-key="${escapeHtml(entry.key)}" role="status" aria-live="polite" aria-atomic="true" hidden></span>${controlHtml}</div>
</section>
`;
}).join('');
Expand Down Expand Up @@ -633,10 +637,37 @@ function render(container: HTMLElement, controller: ReturnType<typeof createConf
});
};

const emitTooltip = (result: ApplyConfigResult): void => {
if (result.tooltip) {
hooks.onTooltip?.(result.tooltip);
const rowStatusTimers = new Map<string, number>();
const showRowSavedStatus = (key: string, message: string): void => {
const chip = container.querySelector(`[data-save-status-key="${CSS.escape(key)}"]`) as HTMLElement | null;
if (!chip) {
hooks.onTooltip?.({ message, tone: 'success' });
return;
}
const existingTimer = rowStatusTimers.get(key);
if (existingTimer !== undefined) {
window.clearTimeout(existingTimer);
}
chip.textContent = message;
chip.hidden = false;
rowStatusTimers.set(key, window.setTimeout(() => {
chip.hidden = true;
chip.textContent = '';
rowStatusTimers.delete(key);
}, 2200));
};

const emitTooltip = (result: ApplyConfigResult, key?: string): void => {
if (!result.tooltip) {
return;
}
// Success confirmations render inline next to the changed setting;
// errors carry longer messages and keep the floating tooltip.
if (result.tooltip.tone === 'success' && key) {
showRowSavedStatus(key, result.tooltip.message);
return;
}
hooks.onTooltip?.(result.tooltip);
};

const arrayModal = createArrayModalController({
Expand All @@ -647,7 +678,7 @@ function render(container: HTMLElement, controller: ReturnType<typeof createConf
controller.setSelection(changedKey);
controller.setDraftValue(items.join('\n'));
const result = await controller.apply();
emitTooltip(result);
emitTooltip(result, changedKey);
if (result.ok) {
emitConfigChanged(changedKey, items);
}
Expand All @@ -670,7 +701,7 @@ function render(container: HTMLElement, controller: ReturnType<typeof createConf
controller.setSelection(entry.key);
controller.setDraftValue(input.checked ? 'true' : 'false');
const result = await controller.apply();
emitTooltip(result);
emitTooltip(result, entry.key);
const updatedEntry = getUpdatedEntryByKey(entry.key);
if (updatedEntry && typeof updatedEntry.value === 'boolean') {
input.checked = updatedEntry.value;
Expand All @@ -690,7 +721,7 @@ function render(container: HTMLElement, controller: ReturnType<typeof createConf
controller.setSelection(entry.key);
controller.setDraftValue(input.value);
const result = await controller.apply();
emitTooltip(result);
emitTooltip(result, entry.key);
const updatedEntry = getUpdatedEntryByKey(entry.key);
input.value = String(updatedEntry?.value ?? controller.state.draftValue);
if (result.ok) {
Expand All @@ -714,7 +745,7 @@ function render(container: HTMLElement, controller: ReturnType<typeof createConf
controller.setSelection(entry.key);
controller.setDraftValue(select.value);
const result = await controller.apply();
emitTooltip(result);
emitTooltip(result, entry.key);
const updatedEntry = getUpdatedEntryByKey(entry.key);
const shellValue = String(updatedEntry?.value ?? select.value);
const shellCustomInput = container.querySelector(`input[data-action="shell-custom"][data-key-index="${keyIndex}"]`) as HTMLInputElement | null;
Expand All @@ -737,7 +768,7 @@ function render(container: HTMLElement, controller: ReturnType<typeof createConf
controller.setSelection(entry.key);
controller.setDraftValue(input.value.trim());
const result = await controller.apply();
emitTooltip(result);
emitTooltip(result, entry.key);
const updatedEntry = getUpdatedEntryByKey(entry.key);
input.value = String(updatedEntry?.value ?? controller.state.draftValue);
if (result.ok) {
Expand Down Expand Up @@ -769,7 +800,7 @@ function render(container: HTMLElement, controller: ReturnType<typeof createConf
controller.setSelection(entry.key);
controller.setDraftValue(input.value.replace(/\r?\n/g, ' '));
const result = await controller.apply();
emitTooltip(result);
emitTooltip(result, entry.key);
const updatedEntry = getUpdatedEntryByKey(entry.key);
input.value = String(updatedEntry?.value ?? controller.state.draftValue);
if (result.ok) {
Expand Down
13 changes: 13 additions & 0 deletions src/ui/styles/apps/config-editor.css
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,19 @@ body {
gap: 8px;
}

/* Inline "Saved" confirmation text, shown next to the control that changed. */
.setting-save-status {
font-size: 12px;
line-height: 1;
white-space: nowrap;
pointer-events: none;
color: var(--cfg-muted);
}

.setting-save-status[hidden] {
display: none;
}

.setting-inline-input {
width: 120px;
box-sizing: border-box;
Expand Down
34 changes: 26 additions & 8 deletions src/utils/mcp-ui-ab-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const MCP_UI_SHOW_VARIANT = 'showMCPUi';
export const MCP_UI_HIDE_VARIANT = 'notShowMCPUi';

export interface McpUiPreviewDecisionDeps {
getUserOverride: () => Promise<unknown>;
getExistingAssignment: () => Promise<unknown>;
isFirstRun: () => boolean;
wasLoadedFromCache: () => boolean;
Expand All @@ -24,6 +25,13 @@ function variantEnablesMcpUi(variant: unknown): boolean | null {

export async function resolveMcpUiPreviewDecision(deps: McpUiPreviewDecisionDeps): Promise<boolean> {
try {
// An explicit user choice (showMcpUI config) always wins over the A/B test.
// Unset (or any non-boolean) means "automatic": fall through to the experiment.
const userOverride = await deps.getUserOverride();
if (typeof userOverride === 'boolean') {
return userOverride;
}

const existingAssignment = await deps.getExistingAssignment();
const existingDecision = variantEnablesMcpUi(existingAssignment);
if (existingDecision !== null) {
Expand Down Expand Up @@ -65,13 +73,23 @@ export async function resolveMcpUiPreviewDecision(deps: McpUiPreviewDecisionDeps
}
}

// Decided once per server process: a session must render consistently. Flipping
// tool UI _meta mid-session confuses hosts (open widgets / other threads sharing
// this server see tools lose their UI), so config/flag changes made while the
// server is running take effect on the next restart.
let sessionDecision: Promise<boolean> | null = null;

export async function shouldShowMcpUiPreviews(): Promise<boolean> {
return resolveMcpUiPreviewDecision({
getExistingAssignment: () => configManager.getValue(`abTest_${MCP_UI_EXPERIMENT_NAME}`),
isFirstRun: () => configManager.isFirstRun(),
wasLoadedFromCache: () => featureFlagManager.wasLoadedFromCache(),
waitForFreshFlags: () => featureFlagManager.waitForFreshFlags(),
getABTestVariant,
capture,
});
if (!sessionDecision) {
sessionDecision = resolveMcpUiPreviewDecision({
getUserOverride: () => configManager.getValue('showMcpUI'),
getExistingAssignment: () => configManager.getValue(`abTest_${MCP_UI_EXPERIMENT_NAME}`),
isFirstRun: () => configManager.isFirstRun(),
wasLoadedFromCache: () => featureFlagManager.wasLoadedFromCache(),
waitForFreshFlags: () => featureFlagManager.waitForFreshFlags(),
getABTestVariant,
capture,
});
}
return sessionDecision;
}
45 changes: 45 additions & 0 deletions test/ab-test.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ function createMcpUiDeps(overrides = {}) {
return {
calls,
deps: {
getUserOverride: async () => undefined,
getExistingAssignment: async () => undefined,
isFirstRun: () => false,
wasLoadedFromCache: () => true,
Expand Down Expand Up @@ -262,6 +263,50 @@ async function runTests() {
assert.strictEqual(MCP_UI_HIDE_VARIANT, 'notShowMCPUi');
});

await test('MCP UI user override false wins without consulting the experiment', async () => {
const { deps, calls } = createMcpUiDeps({
getUserOverride: async () => false,
getExistingAssignment: async () => MCP_UI_SHOW_VARIANT,
isFirstRun: () => true,
});

const enabled = await resolveMcpUiPreviewDecision(deps);

assert.strictEqual(enabled, false);
assert.deepStrictEqual(calls.variantRequests, []);
assert.deepStrictEqual(calls.captured, []);
});

await test('MCP UI user override true wins over a hide assignment', async () => {
const { deps, calls } = createMcpUiDeps({
getUserOverride: async () => true,
getExistingAssignment: async () => MCP_UI_HIDE_VARIANT,
});

const enabled = await resolveMcpUiPreviewDecision(deps);

assert.strictEqual(enabled, true);
assert.deepStrictEqual(calls.variantRequests, []);
assert.deepStrictEqual(calls.captured, []);
});

await test('MCP UI non-boolean override falls through to the experiment', async () => {
const { deps, calls } = createMcpUiDeps({
getUserOverride: async () => 'true', // stringly-typed values must NOT count as an override
getExistingAssignment: async () => MCP_UI_HIDE_VARIANT,
isFirstRun: () => false,
getABTestVariant: async (experimentName) => {
calls.variantRequests.push(experimentName);
return MCP_UI_HIDE_VARIANT;
},
});

const enabled = await resolveMcpUiPreviewDecision(deps);

assert.strictEqual(enabled, false);
assert.deepStrictEqual(calls.variantRequests, [MCP_UI_EXPERIMENT_NAME]);
});

await test('MCP UI existing users without assignment are not enrolled', async () => {
const { deps, calls } = createMcpUiDeps({ isFirstRun: () => false });

Expand Down
Loading