diff --git a/docs/design/workspace-agents-api.md b/docs/design/workspace-agents-api.md new file mode 100644 index 0000000000..2630a1a85f --- /dev/null +++ b/docs/design/workspace-agents-api.md @@ -0,0 +1,77 @@ +# Workspace Agents API + +## Goal + +Expose the complete persisted subagent definition through daemon CRUD APIs, +while keeping prompt generation a generic workspace text-generation concern. + +## Resource shape + +Agent list entries expose all YAML frontmatter fields that affect execution: + +- `name`, `description`, `level`, source/read-only metadata +- `tools` and `disallowedTools`, including MCP tool names +- `model`, `approvalMode`, `permissionMode`, `maxTurns`, and `color` +- MCP server names, Hook event names, `background`, and legacy `runConfig` + +`GET /workspace/agents/:agentType` returns the same shape plus +`systemPrompt`, `mcpServers`, and complete `hooks`. MCP environment variables, +headers, and OAuth client secrets use the same `__redacted__` placeholders as +the settings API. The list intentionally omits those three potentially large +or sensitive values and returns `mcpServerNames` / `hookEvents` instead. +Existing summary fields remain for compatibility. + +`POST /workspace/agents` and `POST /workspace/agents/:agentType` accept the +same persisted fields. The route validates optional values strictly before +writing; invalid API input returns `422 invalid_config` rather than being +silently dropped on the next disk read. Empty arrays and records are valid on +updates and clear the corresponding frontmatter field. `null` clears optional +scalar policy fields on updates. + +Workspace-qualified routes use the same resource shape and validation, while +remaining project-scope only. + +## Prompt generation + +The create and edit pages send their prompt to `POST /workspace/generate`, +consume the standard workspace generation SSE envelope, and generate the +description and system prompt as two plain-text requests. This avoids coupling +the generic endpoint to an Agent JSON schema. + +Generation is an optional dialog launched below the system-prompt field. Both +results stream into reviewable draft fields inside the dialog; cancelling +leaves the form untouched, while confirming copies both drafts into the form. +There is no separate generation mode in the persisted Agent resource. + +The existing `POST /workspace/agents/generate` compatibility route remains +unchanged, but this UI does not use its structured +`{name, description, systemPrompt}` response. The name and execution policy +remain user-owned form fields; the generated description and system prompt are +both reviewable before saving. + +The editor preheats the workspace ACP runtime before loading the tool catalog. +It obtains built-in tools from the workspace tools status, initializes MCP +discovery using the same polling flow as the MCP manager, and then loads MCP +tools from each workspace MCP server's tools endpoint. MCP tools are never +inferred from the workspace tools response, so the two sources remain distinct. + +Allowed and disallowed tools use the same cascading picker: choose built-in or +MCP, choose an MCP server when applicable, then choose a canonical tool. The +selected tools remain visible as removable rows. Configured MCP servers use a +compact select-and-add control instead of free-form JSON or an expanded list. +An empty allowed-tool selection preserves the documented "inherit all tools" +behavior. + +When the editor sends a selected server configuration back, the route restores +redacted MCP values from the effective workspace settings (or the existing +Agent during an edit) before writing. Responses remain redacted, so secrets do +not cross the daemon HTTP boundary. + +## Compatibility + +- Existing Agent CRUD fields and response fields remain valid. +- `hasTools` remains as a derived compatibility field. +- `runConfig.max_turns` remains readable/writable, while `maxTurns` is the + canonical documented turn limit and wins at runtime. +- Existing Agent-specific generation endpoints and SDK helpers remain + available for compatibility. diff --git a/packages/acp-bridge/src/status.ts b/packages/acp-bridge/src/status.ts index 9b1a0d8d1e..edb3dfbd73 100644 --- a/packages/acp-bridge/src/status.ts +++ b/packages/acp-bridge/src/status.ts @@ -814,10 +814,17 @@ export interface ServeWorkspaceAgentSummary { isBuiltin: boolean; /** Whether this agent restricts the tool set via `tools:` frontmatter. */ hasTools: boolean; + tools?: string[]; + disallowedTools?: string[]; model?: string; color?: string; background?: boolean; approvalMode?: string; + permissionMode?: string; + maxTurns?: number; + mcpServerNames?: string[]; + hookEvents?: string[]; + runConfig?: { max_time_minutes?: number; max_turns?: number }; extensionName?: string; /** Absolute path to the file backing this agent (or sentinel for built-ins). */ filePath?: string; @@ -825,9 +832,8 @@ export interface ServeWorkspaceAgentSummary { export interface ServeWorkspaceAgentDetail extends ServeWorkspaceAgentSummary { systemPrompt: string; - tools?: string[]; - disallowedTools?: string[]; - runConfig?: { max_time_minutes?: number; max_turns?: number }; + mcpServers?: Record; + hooks?: Record; } export interface ServeWorkspaceAgentsStatus { diff --git a/packages/cli/src/serve/workspace-agents.test.ts b/packages/cli/src/serve/workspace-agents.test.ts index 0af6875026..afad14807d 100644 --- a/packages/cli/src/serve/workspace-agents.test.ts +++ b/packages/cli/src/serve/workspace-agents.test.ts @@ -208,6 +208,199 @@ describe('workspace agents routes', () => { expect(reviewerEntry?.systemPrompt).toBeUndefined(); }); + it('round-trips complete frontmatter metadata through create, list, and detail', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const config = { + name: 'complete-agent', + description: 'Exercises the complete daemon Agent contract', + systemPrompt: 'You are a complete test agent.', + scope: 'workspace', + tools: ['read_file', 'mcp__github__search'], + disallowedTools: ['run_shell_command', 'mcp__slack'], + model: 'fast', + approvalMode: 'bubble', + maxTurns: 12, + color: 'cyan', + mcpServers: { + filesystem: { type: 'stdio', command: 'node', args: ['server.js'] }, + }, + hooks: { + PreToolUse: [ + { + matcher: 'run_shell_command', + hooks: [{ type: 'command', command: 'echo checking' }], + }, + ], + }, + }; + + const create = await request(app).post('/workspace/agents').send(config); + expect(create.status).toBe(201); + const { scope: _scope, ...persistedConfig } = config; + expect(create.body.agent).toMatchObject({ + ...persistedConfig, + level: 'project', + }); + + const list = await request(app).get('/workspace/agents'); + const summary = list.body.agents.find( + (agent: { name: string }) => agent.name === config.name, + ); + expect(summary).toMatchObject({ + name: config.name, + tools: config.tools, + disallowedTools: config.disallowedTools, + model: config.model, + approvalMode: config.approvalMode, + maxTurns: config.maxTurns, + color: config.color, + mcpServerNames: ['filesystem'], + hookEvents: ['PreToolUse'], + }); + expect(summary.systemPrompt).toBeUndefined(); + + const detail = await request(app).get(`/workspace/agents/${config.name}`); + expect(detail.status).toBe(200); + expect(detail.body).toMatchObject({ + ...persistedConfig, + level: 'project', + }); + }); + + it('restores selected MCP server secrets before writing an agent', async () => { + const settingsDir = path.join(workspace, QWEN_DIR); + await fs.mkdir(settingsDir, { recursive: true }); + await fs.writeFile( + path.join(settingsDir, 'settings.json'), + JSON.stringify({ + mcpServers: { + private: { + command: 'private-server', + env: { PRIVATE_TOKEN: 'secret-value' }, + }, + }, + }), + 'utf8', + ); + const app = buildApp({ + bridge: buildBridgeStub(), + boundWorkspace: workspace, + }); + + const create = await request(app) + .post('/workspace/agents') + .send({ + name: 'private-agent', + description: 'Uses a selected private MCP server', + systemPrompt: 'Use the private MCP server.', + scope: 'workspace', + mcpServers: { + private: { + command: 'private-server', + env: { PRIVATE_TOKEN: '__redacted__' }, + }, + }, + }); + + expect(create.status).toBe(201); + expect(create.body.agent.mcpServers.private.env).toEqual({ + PRIVATE_TOKEN: '__redacted__', + }); + const onDisk = await fs.readFile( + path.join(workspace, QWEN_DIR, 'agents', 'private-agent.md'), + 'utf8', + ); + expect(onDisk).toContain('PRIVATE_TOKEN: secret-value'); + expect(onDisk).not.toContain('__redacted__'); + }); + + it('keeps the existing structured agent generation route unchanged', async () => { + const bridge = buildBridgeStub(); + const generateWorkspaceAgent = vi.fn().mockResolvedValue({ + name: 'generated-agent', + description: 'generated description', + systemPrompt: 'generated prompt', + }); + bridge.generateWorkspaceAgent = generateWorkspaceAgent; + const app = buildApp({ bridge, boundWorkspace: workspace }); + + const res = await request(app) + .post('/workspace/agents/generate') + .send({ description: 'generate an agent' }); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ + name: 'generated-agent', + description: 'generated description', + systemPrompt: 'generated prompt', + }); + expect(generateWorkspaceAgent).toHaveBeenCalledWith( + 'generate an agent', + undefined, + ); + }); + + it('updates advanced agent metadata', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app).post('/workspace/agents').send({ + name: 'editable-agent', + description: 'before', + systemPrompt: 'before prompt', + scope: 'workspace', + }); + + const update = await request(app) + .post('/workspace/agents/editable-agent') + .send({ + description: 'after', + systemPrompt: 'after prompt', + tools: ['read_file', 'mcp__github__search'], + disallowedTools: ['run_shell_command'], + model: 'fast', + approvalMode: 'bubble', + maxTurns: 8, + color: 'purple', + mcpServers: { github: { type: 'http', url: 'https://example.com' } }, + hooks: { PreToolUse: [] }, + }); + + expect(update.status).toBe(200); + expect(update.body.agent).toMatchObject({ + description: 'after', + systemPrompt: 'after prompt', + tools: ['read_file', 'mcp__github__search'], + disallowedTools: ['run_shell_command'], + model: 'fast', + approvalMode: 'bubble', + maxTurns: 8, + color: 'purple', + mcpServers: { github: { type: 'http', url: 'https://example.com' } }, + hooks: { PreToolUse: [] }, + }); + + const clear = await request(app) + .post('/workspace/agents/editable-agent') + .send({ + model: null, + approvalMode: null, + maxTurns: null, + color: null, + tools: [], + disallowedTools: [], + mcpServers: {}, + hooks: {}, + }); + expect(clear.status).toBe(200); + expect(clear.body.agent).not.toHaveProperty('model'); + expect(clear.body.agent).not.toHaveProperty('approvalMode'); + expect(clear.body.agent).not.toHaveProperty('maxTurns'); + expect(clear.body.agent).not.toHaveProperty('color'); + expect(clear.body.agent).not.toHaveProperty('mcpServers'); + expect(clear.body.agent).not.toHaveProperty('hooks'); + }); + it('GET /workspace/agents reflects out-of-band agent file changes', async () => { const bridge = buildBridgeStub(); const app = buildApp({ bridge, boundWorkspace: workspace }); @@ -261,6 +454,31 @@ describe('workspace agents routes', () => { expect(res.body.level).toBe('project'); }); + it('returns a shadowed user agent when GET specifies global scope', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app).post('/workspace/agents').send({ + name: 'shadowed-agent', + description: 'project description', + systemPrompt: 'project prompt', + scope: 'workspace', + }); + await request(app).post('/workspace/agents').send({ + name: 'shadowed-agent', + description: 'user description', + systemPrompt: 'user prompt', + scope: 'global', + }); + + const res = await request(app).get( + '/workspace/agents/shadowed-agent?scope=global', + ); + + expect(res.status).toBe(200); + expect(res.body.level).toBe('user'); + expect(res.body.systemPrompt).toBe('user prompt'); + }); + it('returns 404 agent_not_found for unknown agent', async () => { const bridge = buildBridgeStub(); const app = buildApp({ bridge, boundWorkspace: workspace }); @@ -559,6 +777,43 @@ describe('workspace agents routes', () => { expect(res.body.error).toMatch(/approvalMode/); }); + it('returns 422 invalid_config for unknown permissionMode', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).post('/workspace/agents').send({ + name: 'bad-permission-mode', + description: 'a description longer than ten chars', + systemPrompt: 'you are a bad permission mode test agent', + scope: 'workspace', + permissionMode: 'invalid', + }); + expect(res.status).toBe(422); + expect(res.body.code).toBe('invalid_config'); + expect(res.body.error).toMatch(/permissionMode/); + }); + + it('round-trips and clears permissionMode without approvalMode', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const create = await request(app).post('/workspace/agents').send({ + name: 'permission-mode-agent', + description: 'an agent using the compatibility permission mode', + systemPrompt: 'you are a permission mode test agent', + scope: 'workspace', + permissionMode: 'plan', + }); + + expect(create.status).toBe(201); + expect(create.body.agent.permissionMode).toBe('plan'); + + const clear = await request(app) + .post('/workspace/agents/permission-mode-agent') + .send({ permissionMode: null }); + + expect(clear.status).toBe(200); + expect(clear.body.agent).not.toHaveProperty('permissionMode'); + }); + it('strips unknown runConfig keys and rejects malformed values', async () => { const bridge = buildBridgeStub(); const app = buildApp({ bridge, boundWorkspace: workspace }); @@ -951,6 +1206,36 @@ describe('workspace agents routes', () => { }); }); + it('detects unchanged MCP servers and hooks as a no-op', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const mcpServers = { + filesystem: { type: 'stdio', command: 'node', args: ['server.js'] }, + }; + const hooks = { + PreToolUse: [{ matcher: 'read_file', hooks: [] }], + }; + await request(app).post('/workspace/agents').send({ + name: 'record-noop', + description: 'a description longer than ten chars', + systemPrompt: 'you are a record-noop agent', + scope: 'workspace', + mcpServers, + hooks, + }); + const eventsBefore = (bridge as unknown as { events: RecordedEvent[] }) + .events.length; + + const res = await request(app) + .post('/workspace/agents/record-noop') + .send({ mcpServers, hooks }); + + expect(res.status).toBe(200); + expect(res.body.changed).toBe(false); + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + expect(events.length).toBe(eventsBefore); + }); + it('short-circuits no-op updates with changed: false and no event', async () => { const bridge = buildBridgeStub(); const app = buildApp({ bridge, boundWorkspace: workspace }); diff --git a/packages/cli/src/serve/workspace-agents.ts b/packages/cli/src/serve/workspace-agents.ts index c7df48d741..2ffa1b4083 100644 --- a/packages/cli/src/serve/workspace-agents.ts +++ b/packages/cli/src/serve/workspace-agents.ts @@ -5,6 +5,7 @@ */ import { promises as fs } from 'node:fs'; +import { isDeepStrictEqual } from 'node:util'; import type { Application, Request, RequestHandler, Response } from 'express'; import { APPROVAL_MODES, @@ -16,6 +17,11 @@ import { type SubagentConfig, type SubagentLevel, } from '@qwen-code/qwen-code-core'; +import { + redactMcpServersSetting, + restoreRedactedMcpServersSetting, +} from '../config/mcp-server-secrets.js'; +import { loadSettings } from '../config/settings.js'; import { writeStderrLine } from '../utils/stdioHelpers.js'; import { isServeDebugMode } from './debug-mode.js'; import { @@ -74,6 +80,27 @@ const MAX_DESCRIPTION_BYTES = 256 * 1024; const MAX_SYSTEM_PROMPT_BYTES = 256 * 1024; const MAX_TOOLS_ENTRIES = 256; const MAX_TOOL_ID_LENGTH = 256; +const MAX_RECORD_ENTRIES = 128; +const SUBAGENT_APPROVAL_MODES = [...APPROVAL_MODES, 'bubble'] as const; +const SUBAGENT_PERMISSION_MODES = [ + 'acceptEdits', + 'auto', + 'bypassPermissions', + 'default', + 'dontAsk', + 'plan', +] as const; +const SUBAGENT_COLORS = [ + 'auto', + 'red', + 'blue', + 'green', + 'yellow', + 'purple', + 'orange', + 'pink', + 'cyan', +] as const; import { STATUS_SCHEMA_VERSION, type ServeWorkspaceAgentDetail, @@ -188,7 +215,11 @@ export function mountWorkspaceAgentsRoutes( } const level: SubagentLevel = scope === 'workspace' ? 'project' : 'user'; - const config = parseAgentConfig(body, level, res); + const config = parseAgentConfig( + restoreAgentMcpServerSecrets(body, deps.boundWorkspace, level), + level, + res, + ); if (!config) return; // `manager.createSubagent` only checks whether the default @@ -349,7 +380,9 @@ export function mountWorkspaceAgentsRoutes( const agentType = validateAgentType(req, res); if (agentType === null) return; try { - const config = await manager.loadSubagent(agentType); + const scopedLevel = parseScopeQuery(req, res); + if (scopedLevel === null) return; + const config = await manager.loadSubagent(agentType, scopedLevel); if (!config) { res.status(404).json({ error: `Subagent "${agentType}" not found`, @@ -382,10 +415,6 @@ export function mountWorkspaceAgentsRoutes( if (clientIdResult === null) return; const originatorClientId = clientIdResult; - const body = deps.safeBody(req); - const updates = parseAgentUpdates(body, res); - if (!updates) return; - const preferredLevel = parseScopeQuery(req, res); if (preferredLevel === null) return; @@ -401,6 +430,14 @@ export function mountWorkspaceAgentsRoutes( if (assertMutableLevel(existing, agentType, res)) { return; } + const body = restoreAgentMcpServerSecrets( + deps.safeBody(req), + deps.boundWorkspace, + existing.level, + existing.mcpServers, + ); + const updates = parseAgentUpdates(body, res); + if (!updates) return; // Empty / no-op update detection. An empty body or a body whose // recognized fields all match `existing` would otherwise rewrite @@ -742,7 +779,11 @@ export function mountWorkspaceQualifiedAgentsRoutes( const level = parseWorkspaceOnlyAgentBodyScope(body, res); if (level === null) return; const manager = createDaemonSubagentManager(runtime.workspaceCwd); - const config = parseAgentConfig(body, level, res); + const config = parseAgentConfig( + restoreAgentMcpServerSecrets(body, runtime.workspaceCwd, level), + level, + res, + ); if (!config) return; const collision = await manager.loadSubagent(config.name, level); @@ -843,9 +884,6 @@ export function mountWorkspaceQualifiedAgentsRoutes( if (clientIdResult === null) return; const originatorClientId = clientIdResult; - const body = deps.safeBody(req); - const updates = parseAgentUpdates(body, res); - if (!updates) return; const manager = createDaemonSubagentManager(runtime.workspaceCwd); const existing = await manager.loadSubagent(agentType, scopedLevel); if (!existing) { @@ -857,6 +895,14 @@ export function mountWorkspaceQualifiedAgentsRoutes( return; } if (assertMutableLevel(existing, agentType, res)) return; + const body = restoreAgentMcpServerSecrets( + deps.safeBody(req), + runtime.workspaceCwd, + existing.level, + existing.mcpServers, + ); + const updates = parseAgentUpdates(body, res); + if (!updates) return; if (Object.keys(updates).length === 0) { res.status(400).json({ @@ -1196,6 +1242,34 @@ function rejectWorkspaceQualifiedAgentScope( return null; } +function restoreAgentMcpServerSecrets( + body: Record, + workspace: string, + level: SubagentLevel, + currentAgentServers?: Record, +): Record { + const incoming = body['mcpServers']; + if ( + typeof incoming !== 'object' || + incoming === null || + Array.isArray(incoming) + ) { + return body; + } + const settings = loadSettings(workspace); + const configuredServers = + level === 'user' + ? (settings.user.settings.mcpServers ?? {}) + : (settings.merged.mcpServers ?? {}); + return { + ...body, + mcpServers: restoreRedactedMcpServersSetting(incoming, { + ...configuredServers, + ...currentAgentServers, + }), + }; +} + function sendCreateAgentError( res: Response, err: unknown, @@ -1401,25 +1475,81 @@ function parseAgentConfig( // 201 with no `model` field on the file (masking client-serialization // bugs). if (rejectIfPresentWrongType(body, 'model', 'string', res)) return undefined; - if (typeof body['model'] === 'string') config.model = body['model']; + if (typeof body['model'] === 'string') { + if (!body['model'].trim()) { + return sendInvalidConfig(res, '`model` must not be empty when provided'); + } + config.model = body['model'].trim(); + } if (rejectIfPresentWrongType(body, 'color', 'string', res)) return undefined; - if (typeof body['color'] === 'string') config.color = body['color']; + if (typeof body['color'] === 'string') { + if (!SUBAGENT_COLORS.includes(body['color'] as never)) { + return sendInvalidConfig( + res, + `\`color\` must be one of ${JSON.stringify(SUBAGENT_COLORS)}`, + ); + } + config.color = body['color']; + } if (rejectIfPresentWrongType(body, 'approvalMode', 'string', res)) { return undefined; } if (typeof body['approvalMode'] === 'string') { - if (!APPROVAL_MODES.includes(body['approvalMode'] as never)) { - res.status(422).json({ - error: `\`approvalMode\` must be one of ${JSON.stringify(APPROVAL_MODES)}`, - code: 'invalid_config', - }); - return undefined; + if (!SUBAGENT_APPROVAL_MODES.includes(body['approvalMode'] as never)) { + return sendInvalidConfig( + res, + `\`approvalMode\` must be one of ${JSON.stringify(SUBAGENT_APPROVAL_MODES)}`, + ); } config.approvalMode = body['approvalMode']; } + if (rejectIfPresentWrongType(body, 'permissionMode', 'string', res)) { + return undefined; + } + if (typeof body['permissionMode'] === 'string') { + if (!SUBAGENT_PERMISSION_MODES.includes(body['permissionMode'] as never)) { + return sendInvalidConfig( + res, + `\`permissionMode\` must be one of ${JSON.stringify(SUBAGENT_PERMISSION_MODES)}`, + ); + } + config.permissionMode = body['permissionMode']; + } + + if ('maxTurns' in body) { + const maxTurns = parseMaxTurns(body['maxTurns'], res); + if (maxTurns === null) return undefined; + config.maxTurns = maxTurns; + } + + if ('mcpServers' in body) { + const mcpServers = parseRecordField( + body['mcpServers'], + 'mcpServers', + (value) => + typeof value === 'object' && value !== null && !Array.isArray(value), + 'an object of server names to server configuration objects', + res, + ); + if (mcpServers === null) return undefined; + if (Object.keys(mcpServers).length > 0) config.mcpServers = mcpServers; + } + + if ('hooks' in body) { + const hooks = parseRecordField( + body['hooks'], + 'hooks', + Array.isArray, + 'an object of hook event names to matcher arrays', + res, + ); + if (hooks === null) return undefined; + if (Object.keys(hooks).length > 0) config.hooks = hooks; + } + if (rejectIfPresentWrongType(body, 'background', 'boolean', res)) { return undefined; } @@ -1507,26 +1637,94 @@ function parseAgentUpdates( // Optional scalar fields. Match the create-side fail-closed posture // so a typo like `model: 123` returns 422 instead of silently // succeeding with no model change. - if (rejectIfPresentWrongType(body, 'model', 'string', res)) return undefined; - if (typeof body['model'] === 'string') updates.model = body['model']; - - if (rejectIfPresentWrongType(body, 'color', 'string', res)) return undefined; - if (typeof body['color'] === 'string') updates.color = body['color']; - - if (rejectIfPresentWrongType(body, 'approvalMode', 'string', res)) { + if (body['model'] === null) { + updates.model = undefined; + } else if (rejectIfPresentWrongType(body, 'model', 'string', res)) { return undefined; + } else if (typeof body['model'] === 'string') { + if (!body['model'].trim()) { + return sendInvalidConfig(res, '`model` must not be empty when provided'); + } + updates.model = body['model'].trim(); } - if (typeof body['approvalMode'] === 'string') { - if (!APPROVAL_MODES.includes(body['approvalMode'] as never)) { - res.status(422).json({ - error: `\`approvalMode\` must be one of ${JSON.stringify(APPROVAL_MODES)}`, - code: 'invalid_config', - }); - return undefined; + + if (body['color'] === null) { + updates.color = undefined; + } else if (rejectIfPresentWrongType(body, 'color', 'string', res)) { + return undefined; + } else if (typeof body['color'] === 'string') { + if (!SUBAGENT_COLORS.includes(body['color'] as never)) { + return sendInvalidConfig( + res, + `\`color\` must be one of ${JSON.stringify(SUBAGENT_COLORS)}`, + ); + } + updates.color = body['color']; + } + + if (body['approvalMode'] === null) { + updates.approvalMode = undefined; + } else if (rejectIfPresentWrongType(body, 'approvalMode', 'string', res)) { + return undefined; + } else if (typeof body['approvalMode'] === 'string') { + if (!SUBAGENT_APPROVAL_MODES.includes(body['approvalMode'] as never)) { + return sendInvalidConfig( + res, + `\`approvalMode\` must be one of ${JSON.stringify(SUBAGENT_APPROVAL_MODES)}`, + ); } updates.approvalMode = body['approvalMode']; } + if (body['permissionMode'] === null) { + updates.permissionMode = undefined; + } else if (rejectIfPresentWrongType(body, 'permissionMode', 'string', res)) { + return undefined; + } else if (typeof body['permissionMode'] === 'string') { + if (!SUBAGENT_PERMISSION_MODES.includes(body['permissionMode'] as never)) { + return sendInvalidConfig( + res, + `\`permissionMode\` must be one of ${JSON.stringify(SUBAGENT_PERMISSION_MODES)}`, + ); + } + updates.permissionMode = body['permissionMode']; + } + + if ('maxTurns' in body) { + if (body['maxTurns'] === null) { + updates.maxTurns = undefined; + } else { + const maxTurns = parseMaxTurns(body['maxTurns'], res); + if (maxTurns === null) return undefined; + updates.maxTurns = maxTurns; + } + } + + if ('mcpServers' in body) { + const mcpServers = parseRecordField( + body['mcpServers'], + 'mcpServers', + (value) => + typeof value === 'object' && value !== null && !Array.isArray(value), + 'an object of server names to server configuration objects', + res, + ); + if (mcpServers === null) return undefined; + updates.mcpServers = mcpServers; + } + + if ('hooks' in body) { + const hooks = parseRecordField( + body['hooks'], + 'hooks', + Array.isArray, + 'an object of hook event names to matcher arrays', + res, + ); + if (hooks === null) return undefined; + updates.hooks = hooks; + } + if (rejectIfPresentWrongType(body, 'background', 'boolean', res)) { return undefined; } @@ -1572,6 +1770,61 @@ function parseStringArray( return value as string[]; } +function sendInvalidConfig(res: Response, error: string): undefined { + res.status(422).json({ error, code: 'invalid_config' }); + return undefined; +} + +function parseMaxTurns(value: unknown, res: Response): number | null { + if ( + typeof value !== 'number' || + !Number.isFinite(value) || + !Number.isInteger(value) || + value <= 0 + ) { + sendInvalidConfig(res, '`maxTurns` must be a positive integer'); + return null; + } + return value; +} + +function parseRecordField( + value: unknown, + field: string, + isValidEntry: (value: unknown) => boolean, + expected: string, + res: Response, +): Record | null { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + sendInvalidConfig(res, `\`${field}\` must be ${expected}`); + return null; + } + const entries = Object.entries(value as Record); + if (entries.length > MAX_RECORD_ENTRIES) { + sendInvalidConfig( + res, + `\`${field}\` exceeds the ${MAX_RECORD_ENTRIES}-entry limit`, + ); + return null; + } + const output: Record = {}; + for (const [key, entry] of entries) { + if ( + !key || + key.length > MAX_TOOL_ID_LENGTH || + key === '__proto__' || + key === 'constructor' || + key === 'prototype' || + !isValidEntry(entry) + ) { + sendInvalidConfig(res, `\`${field}\` must be ${expected}`); + return null; + } + output[key] = entry; + } + return output; +} + /** * Returns `true` and sends a 422 when `body[key]` is present but the * wrong scalar type. The caller then returns `undefined` to short- @@ -1630,18 +1883,45 @@ function isNoOpUpdate( ) { return false; } - if (updates.model !== undefined && updates.model !== existing.model) { + if ('model' in updates && updates.model !== existing.model) { return false; } - if (updates.color !== undefined && updates.color !== existing.color) { + if ('color' in updates && updates.color !== existing.color) { return false; } if ( - updates.approvalMode !== undefined && + 'approvalMode' in updates && updates.approvalMode !== existing.approvalMode ) { return false; } + if ( + 'permissionMode' in updates && + updates.permissionMode !== existing.permissionMode + ) { + return false; + } + if ('maxTurns' in updates && updates.maxTurns !== existing.maxTurns) { + return false; + } + if ( + updates.mcpServers !== undefined && + !isDeepStrictEqual( + normalizedRecord(updates.mcpServers), + normalizedRecord(existing.mcpServers), + ) + ) { + return false; + } + if ( + updates.hooks !== undefined && + !isDeepStrictEqual( + normalizedRecord(updates.hooks), + normalizedRecord(existing.hooks), + ) + ) { + return false; + } if ( updates.background !== undefined && updates.background !== existing.background @@ -1669,6 +1949,23 @@ function isNoOpUpdate( return true; } +function normalizedRecord( + value: Record | undefined, +): Record | undefined { + if (!value || Object.keys(value).length === 0) return undefined; + return normalizeRecordValue(value) as Record; +} + +function normalizeRecordValue(value: unknown): unknown { + if (Array.isArray(value)) return value.map(normalizeRecordValue); + if (typeof value !== 'object' || value === null) return value; + return Object.fromEntries( + Object.entries(value as Record) + .filter(([, entry]) => entry !== undefined) + .map(([key, entry]) => [key, normalizeRecordValue(entry)]), + ); +} + function shallowArrayEqual( a: readonly string[] | undefined, b: readonly string[] | undefined, @@ -1748,10 +2045,30 @@ export function toSummary(config: SubagentConfig): ServeWorkspaceAgentSummary { isBuiltin: config.isBuiltin === true || config.level === 'builtin', hasTools: Array.isArray(config.tools) && config.tools.length > 0, }; + if (config.tools) summary.tools = [...config.tools]; + if (config.disallowedTools) { + summary.disallowedTools = [...config.disallowedTools]; + } if (config.model) summary.model = config.model; if (config.color) summary.color = config.color; if (config.background !== undefined) summary.background = config.background; if (config.approvalMode) summary.approvalMode = config.approvalMode; + if (config.permissionMode) summary.permissionMode = config.permissionMode; + if (config.maxTurns !== undefined) summary.maxTurns = config.maxTurns; + if (config.mcpServers) { + summary.mcpServerNames = Object.keys(config.mcpServers); + } + if (config.hooks) summary.hookEvents = Object.keys(config.hooks); + if (config.runConfig) { + const runConfig: NonNullable = {}; + if (typeof config.runConfig.max_time_minutes === 'number') { + runConfig.max_time_minutes = config.runConfig.max_time_minutes; + } + if (typeof config.runConfig.max_turns === 'number') { + runConfig.max_turns = config.runConfig.max_turns; + } + summary.runConfig = runConfig; + } if (config.extensionName) summary.extensionName = config.extensionName; if (config.filePath) summary.filePath = config.filePath; return summary; @@ -1762,26 +2079,13 @@ export function toDetail(config: SubagentConfig): ServeWorkspaceAgentDetail { ...toSummary(config), systemPrompt: config.systemPrompt, }; - if (config.tools) detail.tools = [...config.tools]; - if (config.disallowedTools) { - detail.disallowedTools = [...config.disallowedTools]; - } - if (config.runConfig) { - // Explicit field pick rather than spread-with-cast. If - // `SubagentConfig.runConfig` gains new fields in core, the - // spread-then-cast pattern would silently leak them through the - // HTTP response without a compile error. Picking `max_time_minutes` - // and `max_turns` by name forces a deliberate schema bump if a - // future core field needs to surface on the daemon route. - const runConfig: ServeWorkspaceAgentDetail['runConfig'] = {}; - if (typeof config.runConfig.max_time_minutes === 'number') { - runConfig.max_time_minutes = config.runConfig.max_time_minutes; - } - if (typeof config.runConfig.max_turns === 'number') { - runConfig.max_turns = config.runConfig.max_turns; - } - detail.runConfig = runConfig; + if (config.mcpServers) { + detail.mcpServers = redactMcpServersSetting(config.mcpServers) as Record< + string, + unknown + >; } + if (config.hooks) detail.hooks = config.hooks; return detail; } diff --git a/packages/sdk-typescript/src/daemon-mcp/serve-bridge/tools/workspaceWrite.ts b/packages/sdk-typescript/src/daemon-mcp/serve-bridge/tools/workspaceWrite.ts index 6ed6c06f67..d770e36f5a 100644 --- a/packages/sdk-typescript/src/daemon-mcp/serve-bridge/tools/workspaceWrite.ts +++ b/packages/sdk-typescript/src/daemon-mcp/serve-bridge/tools/workspaceWrite.ts @@ -245,6 +245,13 @@ export function workspaceWriteTools(state: BridgeState): any[] { .optional() .describe('Disallowed tool names.'), model: z.string().optional().describe('Model ID for the agent.'), + approval_mode: z.string().optional().describe('Approval mode.'), + permission_mode: z.string().optional().describe('Permission mode.'), + max_turns: z.number().int().positive().optional(), + color: z.string().optional(), + mcp_servers: z.record(z.string(), z.unknown()).optional(), + hooks: z.record(z.string(), z.unknown()).optional(), + background: z.boolean().optional(), }, handler(async (args) => handleAgentsManage(state, args)), ), @@ -295,7 +302,10 @@ async function handleAgentGet(state: BridgeState, args: any): Promise { return formatToolError('agent_type is required for get action.'); } return formatJsonResult( - await state.client.getWorkspaceAgent(args.agent_type), + await state.client.getWorkspaceAgent( + args.agent_type, + args.scope ? { scope: args.scope } : {}, + ), ); } @@ -314,6 +324,13 @@ async function handleAgentCreate(state: BridgeState, args: any): Promise { tools: args.tools, disallowedTools: args.disallowed_tools, model: args.model, + approvalMode: args.approval_mode, + permissionMode: args.permission_mode, + maxTurns: args.max_turns, + color: args.color, + mcpServers: args.mcp_servers, + hooks: args.hooks, + background: args.background, }), ); } @@ -327,10 +344,17 @@ async function handleAgentUpdate(state: BridgeState, args: any): Promise { args.system_prompt !== undefined || args.tools !== undefined || args.disallowed_tools !== undefined || - args.model !== undefined; + args.model !== undefined || + args.approval_mode !== undefined || + args.permission_mode !== undefined || + args.max_turns !== undefined || + args.color !== undefined || + args.mcp_servers !== undefined || + args.hooks !== undefined || + args.background !== undefined; if (!hasField) { return formatToolError( - 'At least one field to update must be provided (description, system_prompt, tools, disallowed_tools, or model).', + 'At least one field to update must be provided (description, system_prompt, tools, disallowed_tools, model, approval_mode, permission_mode, max_turns, color, mcp_servers, hooks, or background).', ); } return formatJsonResult( @@ -342,6 +366,13 @@ async function handleAgentUpdate(state: BridgeState, args: any): Promise { tools: args.tools, disallowedTools: args.disallowed_tools, model: args.model, + approvalMode: args.approval_mode, + permissionMode: args.permission_mode, + maxTurns: args.max_turns, + color: args.color, + mcpServers: args.mcp_servers, + hooks: args.hooks, + background: args.background, }, { scope: args.scope }, ), diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index 14d7cd1682..649cdbfc13 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -1913,9 +1913,13 @@ export class DaemonClient { async getWorkspaceAgent( agentType: string, + opts: { scope?: 'workspace' | 'global' } = {}, ): Promise { + const url = opts.scope + ? `${this.baseUrl}/workspace/agents/${urlEncode(agentType)}?scope=${urlEncode(opts.scope)}` + : `${this.baseUrl}/workspace/agents/${urlEncode(agentType)}`; return await this.fetchWithTimeout( - `${this.baseUrl}/workspace/agents/${urlEncode(agentType)}`, + url, { headers: this.headers() }, async (res) => { if (!res.ok) { diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index eedf9cae40..f799b0de8e 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -1721,10 +1721,17 @@ export interface DaemonWorkspaceAgentSummary { level: DaemonAgentLevel; isBuiltin: boolean; hasTools: boolean; + tools?: string[]; + disallowedTools?: string[]; model?: string; color?: string; background?: boolean; approvalMode?: string; + permissionMode?: string; + maxTurns?: number; + mcpServerNames?: string[]; + hookEvents?: string[]; + runConfig?: { max_time_minutes?: number; max_turns?: number }; extensionName?: string; filePath?: string; } @@ -1732,9 +1739,8 @@ export interface DaemonWorkspaceAgentSummary { export interface DaemonWorkspaceAgentDetail extends DaemonWorkspaceAgentSummary { systemPrompt: string; - tools?: string[]; - disallowedTools?: string[]; - runConfig?: { max_time_minutes?: number; max_turns?: number }; + mcpServers?: Record; + hooks?: Record; } export interface DaemonWorkspaceAgentsStatus { @@ -1760,6 +1766,10 @@ export interface DaemonCreateAgentRequest { runConfig?: { max_time_minutes?: number; max_turns?: number }; color?: string; approvalMode?: string; + permissionMode?: string; + maxTurns?: number; + mcpServers?: Record; + hooks?: Record; background?: boolean; } @@ -1783,10 +1793,14 @@ export interface DaemonUpdateAgentRequest { systemPrompt?: string; tools?: string[]; disallowedTools?: string[]; - model?: string; + model?: string | null; runConfig?: { max_time_minutes?: number; max_turns?: number }; - color?: string; - approvalMode?: string; + color?: string | null; + approvalMode?: string | null; + permissionMode?: string | null; + maxTurns?: number | null; + mcpServers?: Record; + hooks?: Record; background?: boolean; } diff --git a/packages/sdk-typescript/test/unit/DaemonClient.test.ts b/packages/sdk-typescript/test/unit/DaemonClient.test.ts index 3dc6ec7820..fb2b9ccce9 100644 --- a/packages/sdk-typescript/test/unit/DaemonClient.test.ts +++ b/packages/sdk-typescript/test/unit/DaemonClient.test.ts @@ -5884,6 +5884,24 @@ describe('DaemonClient', () => { expect(calls[0]?.url).toBe('http://daemon/workspace/agents/with%2Fslash'); }); + it('getWorkspaceAgent forwards the optional scope query', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, { + name: 'reviewer', + description: 'user reviewer', + level: 'user', + systemPrompt: 'review globally', + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + + await client.getWorkspaceAgent('reviewer', { scope: 'global' }); + + expect(calls[0]?.url).toBe( + 'http://daemon/workspace/agents/reviewer?scope=global', + ); + }); + it('streams stateless workspace generation with the session envelope', async () => { const { fetch } = recordingFetch(() => sseResponse( diff --git a/packages/sdk-typescript/test/unit/serve-bridge.test.ts b/packages/sdk-typescript/test/unit/serve-bridge.test.ts index 31a7b76267..92e7a9d708 100644 --- a/packages/sdk-typescript/test/unit/serve-bridge.test.ts +++ b/packages/sdk-typescript/test/unit/serve-bridge.test.ts @@ -601,6 +601,38 @@ describe('serve-bridge', () => { expect(result.isError).toBeUndefined(); }); + it('should forward scope for agents_manage get', async () => { + const { state, calls } = makeMockState({ + defaultSessionId: 'test-session', + fetchReply: () => + jsonResponse(200, { + kind: 'agent', + name: 'reviewer', + description: 'Reviews code', + level: 'user', + isBuiltin: false, + hasTools: false, + systemPrompt: 'Review the code.', + }), + }); + + const { workspaceWriteTools } = await import( + '../../src/daemon-mcp/serve-bridge/tools/workspaceWrite.js' + ); + const agentsTool = workspaceWriteTools(state).find( + (tool: { name: string }) => tool.name === 'workspace_agents_manage', + ); + + await agentsTool.handler( + { action: 'get', agent_type: 'reviewer', scope: 'global' }, + {}, + ); + + expect(calls[0]?.url).toBe( + 'http://127.0.0.1:4170/workspace/agents/reviewer?scope=global', + ); + }); + it('should reject file_write replace mode without expected_hash', async () => { const { state } = makeMockState({ defaultSessionId: 'test-session', @@ -689,5 +721,52 @@ describe('serve-bridge', () => { 'At least one field to update must be provided', ); }); + + it('should forward advanced agents_manage update fields', async () => { + const { state, calls } = makeMockState({ + defaultSessionId: 'test-session', + fetchReply: () => + jsonResponse(200, { + ok: true, + agent: { + kind: 'agent', + name: 'test-agent', + description: 'test', + level: 'project', + isBuiltin: false, + hasTools: true, + systemPrompt: 'test', + }, + }), + }); + state.allowGlobalScope = true; + const { workspaceWriteTools } = await import( + '../../src/daemon-mcp/serve-bridge/tools/workspaceWrite.js' + ); + const agentsTool = workspaceWriteTools(state).find( + (tool: { name: string }) => tool.name === 'workspace_agents_manage', + ); + + await agentsTool.handler( + { + action: 'update', + agent_type: 'test-agent', + approval_mode: 'bubble', + max_turns: 6, + color: 'cyan', + mcp_servers: { github: { type: 'http' } }, + hooks: { PreToolUse: [] }, + }, + {}, + ); + + expect(JSON.parse(calls[0]?.body ?? '{}')).toMatchObject({ + approvalMode: 'bubble', + maxTurns: 6, + color: 'cyan', + mcpServers: { github: { type: 'http' } }, + hooks: { PreToolUse: [] }, + }); + }); }); }); diff --git a/packages/web-shell/client/App.test.tsx b/packages/web-shell/client/App.test.tsx index 522e69d4e9..0aa4b9356a 100644 --- a/packages/web-shell/client/App.test.tsx +++ b/packages/web-shell/client/App.test.tsx @@ -824,7 +824,7 @@ mockComponent( 'ReleaseSessionDialog', ); mockComponent('./components/dialogs/RewindDialog', 'RewindDialog'); -mockComponent('./components/messages/AgentsMessage', 'AgentsMessage'); +mockComponent('./components/agents/AgentsManagerPage', 'AgentsManagerPage'); mockComponent('./components/messages/MemoryMessage', 'MemoryMessage'); mockComponent('./components/messages/AuthMessage', 'AuthMessage'); // Record keyboardActive so app-level tests can assert the overlay is told to @@ -3496,6 +3496,7 @@ describe('App session callbacks', () => { 'Extensions', 'MCP', 'Skills', + 'Agents', ]); expect(extensionsTab?.getAttribute('aria-selected')).toBe('true'); expect(document.activeElement).toBe(extensionsTab); diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index 6c7bc70420..9c2061d581 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -71,10 +71,7 @@ import { type ModelDialogMode, } from './components/dialogs/ModelDialog'; import { ModelFallbacksDialog } from './components/dialogs/ModelFallbacksDialog'; -import { - AgentsMessage, - type AgentsInitialMode, -} from './components/messages/AgentsMessage'; +import { AgentsManagerPage } from './components/agents/AgentsManagerPage'; import { MemoryMessage } from './components/messages/MemoryMessage'; import { AuthMessage } from './components/messages/AuthMessage'; import { ToolsDialog } from './components/dialogs/ToolsDialog'; @@ -2500,6 +2497,7 @@ export function App({ | 'mcp' | 'skills' | 'plugins' + | 'agents' | null >(null); const closePanel = useCallback(() => setActivePanel(null), []); @@ -2528,7 +2526,8 @@ export function App({ | 'extensions' | 'mcp' | 'skills' - | 'plugins', + | 'plugins' + | 'agents', ) => { setMainView('chat'); setActivePanel(panel); @@ -2931,8 +2930,9 @@ export function App({ const [memoryAddScope, setMemoryAddScope] = useState<'workspace' | 'global'>( 'workspace', ); - const [agentsDialogMode, setAgentsDialogMode] = - useState(null); + const [agentsCreateScope, setAgentsCreateScope] = useState< + 'workspace' | 'global' | null + >(null); const [escapeHintVisible, setEscapeHintVisible] = useState(false); // Whether the first Esc has armed a stream cancellation; the composer's send // button shows an "Esc again to stop" affordance while true. @@ -3229,7 +3229,6 @@ export function App({ showApprovalModeDialog || tasksDialogMessage !== null || mcpDialogMessage !== null || - agentsDialogMode !== null || showMemoryDialog || showAuthDialog || showAddWorkspaceDialog || @@ -5415,23 +5414,22 @@ export function App({ } if (cmd === 'agents') { const subCommand = text.slice(match[0].length).trim().toLowerCase(); - let agentsMode: AgentsInitialMode = 'menu'; if (subCommand === 'create') { - agentsMode = 'create'; + setAgentsCreateScope('global'); } else if ( subCommand === 'create user' || subCommand === 'create global' ) { - agentsMode = 'create-user'; + setAgentsCreateScope('global'); } else if ( subCommand === 'create project' || subCommand === 'create workspace' ) { - agentsMode = 'create-project'; - } else if (subCommand === 'manage') { - agentsMode = 'manage'; + setAgentsCreateScope('workspace'); + } else { + setAgentsCreateScope(null); } - setAgentsDialogMode(agentsMode); + openPanel('agents'); return true; } if (cmd === 'extensions') { @@ -6773,26 +6771,6 @@ export function App({ /> )} - {agentsDialogMode && ( - setAgentsDialogMode(null)} - > - store.dispatch([{ type: 'status', text }])} - onClose={() => setAgentsDialogMode(null)} - /> - - )} {showMemoryDialog && ( + + + {error ? ( + setError(null)} + > + {error} + + ) : null} + + + + + {t('agent.detail.overview')} + + + {t('agent.detail.systemPrompt')} + + {t('agent.detail.tools')} + {t('agent.detail.mcp')} + {t('agent.detail.hooks')} + + + + + + + {t('agent.create.scope')} + + + + + + + {t('agent.create.name')} + + setName(event.target.value)} + placeholder={t('agent.create.namePlaceholder')} + disabled={Boolean(agent)} + /> + {t('agent.create.nameHelp')} + + + + + {t('agent.create.description')} + +