diff --git a/.changeset/session-capability-wiring.md b/.changeset/session-capability-wiring.md new file mode 100644 index 00000000..1b446240 --- /dev/null +++ b/.changeset/session-capability-wiring.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": minor +--- + +Make `agent_config.tools` and `agent_config.mcp_servers` reach the running agent. A session profile update now persists the selection, merges each field independently so supplying one half does not clear the other, resumes an inactive session before the mutation, and applies the result through a single `setActiveTools` call. MCP server names are turned into tool patterns with the shared naming helper, so a server whose name needs sanitizing still matches its tools. diff --git a/packages/agent-core/src/agent/tool/index.ts b/packages/agent-core/src/agent/tool/index.ts index b90810bf..3dbbe7f3 100644 --- a/packages/agent-core/src/agent/tool/index.ts +++ b/packages/agent-core/src/agent/tool/index.ts @@ -349,6 +349,16 @@ export class ToolManager { this.mcpAccessPatterns = names.filter((name) => isMcpToolName(name)); } + patchActiveTools(input: { + readonly tools?: readonly string[]; + readonly mcpPatterns?: readonly string[]; + }): void { + this.setActiveTools([ + ...(input.tools ?? this.enabledTools), + ...(input.mcpPatterns ?? this.mcpAccessPatterns), + ]); + } + copyLoopToolsFrom(source: ToolManager): void { this.loopToolsOverride = source.loopTools; } diff --git a/packages/agent-core/src/mcp/tool-naming.ts b/packages/agent-core/src/mcp/tool-naming.ts index 87dca29a..f7dea93c 100644 --- a/packages/agent-core/src/mcp/tool-naming.ts +++ b/packages/agent-core/src/mcp/tool-naming.ts @@ -6,6 +6,7 @@ const MCP_NAME_SEPARATOR = '__'; * hash suffix so collisions remain extremely unlikely. */ const MAX_QUALIFIED_LENGTH = 64; +const MAX_HASH_SUFFIX_LENGTH = 10; /** * Replace any character outside the safe ASCII set with `_`, then collapse @@ -22,6 +23,11 @@ export function isMcpToolName(name: string): boolean { return name.startsWith(MCP_NAME_PREFIX); } +export function mcpServerToolPattern(serverName: string): string { + const prefix = `${MCP_NAME_PREFIX}${sanitizeMcpNamePart(serverName)}${MCP_NAME_SEPARATOR}`; + return `${prefix.slice(0, MAX_QUALIFIED_LENGTH - MAX_HASH_SUFFIX_LENGTH)}*`; +} + /** * Produce the qualified MCP tool name used inside the agent and on the wire. * If the result would exceed {@link MAX_QUALIFIED_LENGTH}, a deterministic diff --git a/packages/agent-core/src/services/session/session.ts b/packages/agent-core/src/services/session/session.ts index 9655a01f..504c93d8 100644 --- a/packages/agent-core/src/services/session/session.ts +++ b/packages/agent-core/src/services/session/session.ts @@ -122,6 +122,8 @@ export function toProtocolSession( metadata: mergedMetadata, agent_config: { model: '', + tools: meta?.agentConfig?.tools?.slice(), + mcp_servers: meta?.agentConfig?.mcpServers?.slice(), }, usage: emptySessionUsage(), permission_rules: [], diff --git a/packages/agent-core/src/services/session/sessionService.ts b/packages/agent-core/src/services/session/sessionService.ts index 2b6793d5..b668c69a 100644 --- a/packages/agent-core/src/services/session/sessionService.ts +++ b/packages/agent-core/src/services/session/sessionService.ts @@ -44,6 +44,8 @@ const DEFAULT_UNDO_MESSAGE_PAGE_SIZE = 50; const MAX_UNDO_MESSAGE_PAGE_SIZE = 100; const CHILD_SESSION_KIND = 'child'; +type ToolSelectionPatch = NonNullable; + function asJsonObject(value: Record): JsonObject { return value as unknown as JsonObject; } @@ -250,6 +252,10 @@ export class SessionService extends Disposable implements ISessionService { } catch { } } + const toolPatch = this.toToolPatch(input.agent_config); + if (toolPatch !== undefined) { + await this.persistToolSelection(summary.id, toolPatch); + } const meta = await this.tryGetMeta(summary.id); const session = this._patchSessionStatus( toProtocolSession(summary, meta, await this.tryResolveWorkspaceId(summary.workDir)), @@ -315,6 +321,7 @@ export class SessionService extends Disposable implements ISessionService { if (summary === undefined) { throw new SessionNotFoundError(id); } + await this.core.rpc.resumeSession({ sessionId: id }); if (input.title !== undefined) { await this.core.rpc.renameSession({ sessionId: id, title: input.title }); @@ -330,6 +337,10 @@ export class SessionService extends Disposable implements ISessionService { const ac = input.agent_config; if (ac !== undefined) { + const toolPatch = this.toToolPatch(ac); + if (toolPatch !== undefined) { + await this.persistToolSelection(id, toolPatch); + } const patch: AgentStatePatch = {}; if (ac.model !== undefined && ac.model !== '') patch.model = ac.model; if (ac.thinking !== undefined) patch.thinking = ac.thinking; @@ -359,6 +370,30 @@ export class SessionService extends Disposable implements ISessionService { ); } + private toToolPatch( + agentConfig: SessionCreate['agent_config'], + ): ToolSelectionPatch | undefined { + if (agentConfig?.tools === undefined && agentConfig?.mcp_servers === undefined) { + return undefined; + } + return { + tools: agentConfig.tools, + mcpServers: agentConfig.mcp_servers, + }; + } + + private async persistToolSelection(id: string, patch: ToolSelectionPatch): Promise { + await this.core.rpc.updateSessionMetadata({ + sessionId: id, + metadata: { + agentConfig: { + tools: patch.tools, + mcpServers: patch.mcpServers, + }, + }, + }); + } + async fork(id: string, input: SessionFork): Promise { const source = await this.get(id); const title = input.title ?? `Fork: ${source.title || source.id}`; diff --git a/packages/agent-core/src/session/index.ts b/packages/agent-core/src/session/index.ts index df1be11a..261bc6df 100644 --- a/packages/agent-core/src/session/index.ts +++ b/packages/agent-core/src/session/index.ts @@ -213,6 +213,10 @@ export interface SessionMeta { lastPrompt?: string; forkedFrom?: string; agents: Record; + agentConfig?: { + readonly tools?: readonly string[]; + readonly mcpServers?: readonly string[]; + }; custom: Record; } @@ -238,6 +242,13 @@ const SessionMetaSchema = z lastPrompt: z.string().optional(), forkedFrom: z.string().optional(), agents: z.record(z.string(), AgentMetaSchema), + agentConfig: z + .object({ + tools: z.array(z.string()).optional(), + mcpServers: z.array(z.string()).optional(), + }) + .strict() + .optional(), custom: z.record(z.string(), z.unknown()), }) .strict(); diff --git a/packages/agent-core/src/session/rpc.ts b/packages/agent-core/src/session/rpc.ts index d2572599..ed950c4e 100644 --- a/packages/agent-core/src/session/rpc.ts +++ b/packages/agent-core/src/session/rpc.ts @@ -1,5 +1,6 @@ import { ErrorCodes, PythinkerError } from '#/errors'; import { convertMCPContentBlock } from '#/mcp/output'; +import { mcpServerToolPattern } from '#/mcp/tool-naming'; import type { ActivateSkillPayload, AdvisorStatus, @@ -71,13 +72,33 @@ export class SessionAPIImpl implements PromisableMethods { 'sessionFormatVersion cannot be updated', ); } + const incoming = payload.metadata.agentConfig; + const previous = this.session.metadata.agentConfig; + const agentConfig = + incoming === undefined + ? previous + : { + tools: incoming.tools ?? previous?.tools, + mcpServers: incoming.mcpServers ?? previous?.mcpServers, + }; this.session.metadata = { ...this.session.metadata, ...payload.metadata, + agentConfig, agents: this.session.metadata.agents, sessionFormatVersion: this.session.metadata.sessionFormatVersion, }; await this.session.writeMetadata(); + if ( + incoming !== undefined && + (incoming.tools !== undefined || incoming.mcpServers !== undefined) + ) { + const agent = await this.session.ensureAgentResumed('main'); + agent.tools.patchActiveTools({ + tools: incoming.tools, + mcpPatterns: incoming.mcpServers?.map(mcpServerToolPattern), + }); + } } getSessionMetadata(_payload: EmptyPayload): SessionMeta { diff --git a/packages/agent-core/test/mcp/tool-naming.test.ts b/packages/agent-core/test/mcp/tool-naming.test.ts index 7c26b885..70f55c99 100644 --- a/packages/agent-core/test/mcp/tool-naming.test.ts +++ b/packages/agent-core/test/mcp/tool-naming.test.ts @@ -1,6 +1,12 @@ +import picomatch from 'picomatch'; import { describe, expect, it } from 'vitest'; -import { isMcpToolName, qualifyMcpToolName, sanitizeMcpNamePart } from '../../src/mcp/tool-naming'; +import { + isMcpToolName, + mcpServerToolPattern, + qualifyMcpToolName, + sanitizeMcpNamePart, +} from '../../src/mcp/tool-naming'; describe('sanitizeMcpNamePart', () => { it('passes alphanumeric, underscore, and dash through unchanged', () => { @@ -62,3 +68,22 @@ describe('isMcpToolName', () => { expect(isMcpToolName('mcp_one_underscore__no')).toBe(false); }); }); + +describe('mcpServerToolPattern', () => { + it.each(['My Search', 'files[*]'])('matches qualified tools for server %s', (serverName) => { + const pattern = mcpServerToolPattern(serverName); + expect(picomatch.isMatch(qualifyMcpToolName(serverName, 'lookup'), pattern)).toBe(true); + expect(pattern).not.toContain('['); + expect(pattern).not.toContain(']'); + }); + + it('matches qualified tools when the server prefix is truncated', () => { + const serverName = 'long server '.repeat(8); + expect( + picomatch.isMatch( + qualifyMcpToolName(serverName, 'lookup'), + mcpServerToolPattern(serverName), + ), + ).toBe(true); + }); +}); diff --git a/packages/agent-core/test/services/session-service.test.ts b/packages/agent-core/test/services/session-service.test.ts index 9e5b1fa2..99db290d 100644 --- a/packages/agent-core/test/services/session-service.test.ts +++ b/packages/agent-core/test/services/session-service.test.ts @@ -15,6 +15,8 @@ import { type UpdateSessionMetadataPayload, } from '../../src'; import { TestInstantiationService } from '../../src/di/test'; +import type { Agent } from '../../src/agent'; +import { ToolManager } from '../../src/agent/tool'; import { SessionAPIImpl } from '../../src/session/rpc'; import { emptySessionUsage, type Event, type Session } from '@pymodel/protocol'; @@ -57,6 +59,7 @@ interface FakeBridgeState { compactions: Array<{ sessionId: string; agentId: string; instruction?: string }>; undoPayloads: Array<{ sessionId: string; agentId: string; count: number }>; resumedIds: string[]; + activeIds: Set; contexts: Map; postUndoContexts: Map; } @@ -78,6 +81,16 @@ function makeFakeBridge(state: FakeBridgeState): ICoreProcessService { title: undefined, }; state.sessions.push(created); + state.activeIds.add(id); + state.metas.set(id, { + sessionFormatVersion: 2, + title: 'New Session', + createdAt: new Date(created.createdAt).toISOString(), + updatedAt: new Date(created.updatedAt).toISOString(), + isCustomTitle: false, + agents: {}, + custom: { ...payload.metadata }, + }); return created; }), listSessions: vi @@ -166,7 +179,27 @@ function makeFakeBridge(state: FakeBridgeState): ICoreProcessService { .fn() .mockImplementation( async (payload: WithSessionId) => { + if (!state.activeIds.has(payload.sessionId)) { + throw new Error(`inactive session ${payload.sessionId}`); + } state.metadataPatches.set(payload.sessionId, payload.metadata); + const existing = state.metas.get(payload.sessionId); + if (existing !== undefined) { + const incoming = payload.metadata.agentConfig; + const previous = existing.agentConfig; + const agentConfig = + incoming === undefined + ? previous + : { + tools: incoming.tools ?? previous?.tools, + mcpServers: incoming.mcpServers ?? previous?.mcpServers, + }; + state.metas.set(payload.sessionId, { + ...existing, + ...payload.metadata, + agentConfig, + }); + } }, ), getSessionMetadata: vi @@ -187,6 +220,7 @@ function makeFakeBridge(state: FakeBridgeState): ICoreProcessService { state.resumedIds.push(sessionId); const found = state.sessions.find((session) => session.id === sessionId); if (found === undefined) throw new Error(`missing session ${sessionId}`); + state.activeIds.add(sessionId); return found as ResumeSessionResult; }), undoHistory: vi @@ -232,6 +266,7 @@ function freshState(): FakeBridgeState { compactions: [], undoPayloads: [], resumedIds: [], + activeIds: new Set(), contexts: new Map(), postUndoContexts: new Map(), }; @@ -441,6 +476,33 @@ describe('toProtocolSession adapter', () => { expect(proto.metadata['other_key']).toBe('x'); }); + it('returns persisted tool and MCP server selections from SessionMeta', () => { + const summary: SessionSummary = { + id: 'sess_agent_config', + workDir: '/tmp/wd', + sessionDir: '/tmp/sd', + createdAt: 0, + updatedAt: 0, + }; + const meta: SessionMeta = { + sessionFormatVersion: 2, + title: 'Configured', + createdAt: new Date(0).toISOString(), + updatedAt: new Date(0).toISOString(), + isCustomTitle: false, + agents: {}, + agentConfig: { + tools: ['Read'], + mcpServers: ['github'], + }, + custom: {}, + }; + expect(toProtocolSession(summary, meta).agent_config).toMatchObject({ + tools: ['Read'], + mcp_servers: ['github'], + }); + }); + it('preserves custom metadata from the summary when SessionMeta is unavailable', () => { const summary: SessionSummary = { id: 'sess_summary_meta', @@ -522,6 +584,19 @@ describe('SessionService.create', () => { expect(state.sessions[0]!.metadata?.['cwd']).toBe('/tmp/x'); }); + it('persists tool and MCP server selections when supplied', async () => { + const session = await svc.create({ + metadata: { cwd: '/tmp/x' }, + agent_config: { tools: ['Read'], mcp_servers: ['github'] }, + }); + expect(state.metadataPatches.get(session.id)).toEqual({ + agentConfig: { tools: ['Read'], mcpServers: ['github'] }, + }); + expect(promptStub.calls).toEqual([]); + expect(session.agent_config.tools).toEqual(['Read']); + expect(session.agent_config.mcp_servers).toEqual(['github']); + }); + it('passes client telemetry metadata through to core createSession', async () => { await svc.create( { metadata: { cwd: '/tmp/web' } }, @@ -704,6 +779,20 @@ describe('SessionService.update', () => { ]); }); + it('resumes an inactive session before persisting tools + mcp_servers', async () => { + state.activeIds.delete(created.id); + const after = await svc.update(created.id, { + agent_config: { tools: ['Read', 'Bash'], mcp_servers: ['github'] }, + }); + expect(state.metadataPatches.get(created.id)).toEqual({ + agentConfig: { tools: ['Read', 'Bash'], mcpServers: ['github'] }, + }); + expect(promptStub.calls).toEqual([]); + expect(state.resumedIds).toEqual([created.id]); + expect(after.agent_config.tools).toEqual(['Read', 'Bash']); + expect(after.agent_config.mcp_servers).toEqual(['github']); + }); + it('combines model + runtime controls into a single applyAgentState call', async () => { await svc.update(created.id, { agent_config: { model: 'pythinker-code/k9', plan_mode: false }, @@ -726,6 +815,132 @@ describe('SessionService.update', () => { }); describe('SessionAPIImpl persisted metadata boundary', () => { + function makeToolSession( + metadata: SessionMeta, + activeNames: readonly string[], + ): { + readonly session: import('../../src/session').Session; + readonly records: unknown[]; + } { + const records: unknown[] = []; + const tools = new ToolManager({ + config: { hasProvider: false }, + records: { logRecord: (record: unknown) => records.push(record) }, + } as unknown as Agent); + tools.setActiveTools(activeNames); + records.length = 0; + const session = { + metadata, + writeMetadata: vi.fn(async () => {}), + ensureAgentResumed: vi.fn(async () => ({ tools })), + } as unknown as import('../../src/session').Session; + return { session, records }; + } + + function persistedMetadata(agentConfig?: SessionMeta['agentConfig']): SessionMeta { + return { + sessionFormatVersion: 2, + createdAt: new Date(0).toISOString(), + updatedAt: new Date(0).toISOString(), + title: 'Persisted metadata', + isCustomTitle: false, + agents: { main: { type: 'main', parentAgentId: null } }, + agentConfig, + custom: {}, + }; + } + + it('a tools-only patch preserves persisted and active MCP servers', async () => { + const { session, records } = makeToolSession( + persistedMetadata({ tools: ['Read'], mcpServers: ['github'] }), + ['Read', 'mcp__github__*'], + ); + + await new SessionAPIImpl(session).updateSessionMetadata({ + metadata: { agentConfig: { tools: ['Bash'] } }, + }); + + expect(session.metadata.agentConfig).toEqual({ + tools: ['Bash'], + mcpServers: ['github'], + }); + expect(records).toEqual([ + { type: 'tools.set_active_tools', names: ['Bash', 'mcp__github__*'] }, + ]); + }); + + it('an MCP-only patch preserves persisted and active exact tools', async () => { + const { session, records } = makeToolSession( + persistedMetadata({ tools: ['Read', 'Bash'], mcpServers: ['github'] }), + ['Read', 'Bash', 'mcp__github__*'], + ); + + await new SessionAPIImpl(session).updateSessionMetadata({ + metadata: { agentConfig: { mcpServers: ['My Search'] } }, + }); + + expect(session.metadata.agentConfig).toEqual({ + tools: ['Read', 'Bash'], + mcpServers: ['My Search'], + }); + expect(records).toEqual([ + { type: 'tools.set_active_tools', names: ['Read', 'Bash', 'mcp__My_Search__*'] }, + ]); + }); + + it.each([ + { + label: 'tools', + activeNames: ['Read', 'mcp__github__*'], + patch: { tools: [] }, + expectedConfig: { tools: [], mcpServers: ['github'] }, + expectedNames: ['mcp__github__*'], + }, + { + label: 'MCP servers', + activeNames: ['Read', 'mcp__github__*'], + patch: { mcpServers: [] }, + expectedConfig: { tools: ['Read'], mcpServers: [] }, + expectedNames: ['Read'], + }, + ])('an empty $label array clears only that field', async ({ + activeNames, + patch, + expectedConfig, + expectedNames, + }) => { + const { session, records } = makeToolSession( + persistedMetadata({ tools: ['Read'], mcpServers: ['github'] }), + activeNames, + ); + + await new SessionAPIImpl(session).updateSessionMetadata({ + metadata: { agentConfig: patch }, + }); + + expect(session.metadata.agentConfig).toEqual(expectedConfig); + expect(records).toEqual([{ type: 'tools.set_active_tools', names: expectedNames }]); + }); + + it('a legacy session without agentConfig preserves the active default tools', async () => { + const { session, records } = makeToolSession( + persistedMetadata(), + ['Read', 'Bash', 'mcp__*'], + ); + + await new SessionAPIImpl(session).updateSessionMetadata({ + metadata: { agentConfig: { mcpServers: ['github'] } }, + }); + + expect(session.metadata.agentConfig).toEqual({ + tools: undefined, + mcpServers: ['github'], + }); + expect(records).toEqual([ + { type: 'tools.set_active_tools', names: ['Read', 'Bash', 'mcp__github__*'] }, + ]); + }); + it('rejects a runtime sessionFormatVersion patch even when it equals the current format', async () => { const metadata: SessionMeta = { sessionFormatVersion: 2, diff --git a/packages/agent-core/test/session/init.test.ts b/packages/agent-core/test/session/init.test.ts index d6f8afe4..874a41b1 100644 --- a/packages/agent-core/test/session/init.test.ts +++ b/packages/agent-core/test/session/init.test.ts @@ -900,6 +900,58 @@ describe('Session.init', () => { await session.close(); } }, 20000); + + it('restores a partial tool selection from the complete agent record', async () => { + const workDir = await makeTempDir(); + const sessionDir = await makeTempDir(); + const makeSession = () => + new Session({ + id: 'test-tool-selection-resume', + kaos: testKaos.withCwd(workDir), + homedir: sessionDir, + rpc: createSessionRpc([]), + providerManager: testProviderManager(), + mcpConfig: { + servers: { + github: { + transport: 'stdio', + command: process.execPath, + args: [mcpStdioFixture], + }, + }, + }, + }); + let session = makeSession(); + + try { + await session.mcp.waitForInitialLoad(); + const { agent } = await session.createAgent( + { type: 'main' }, + { + profile: { + name: 'resume-tools', + systemPrompt: () => '', + tools: ['Read'], + }, + }, + ); + agent.config.update({ modelAlias: 'mock-model', thinkingLevel: 'off' }); + await new SessionAPIImpl(session).updateSessionMetadata({ + metadata: { agentConfig: { mcpServers: ['github'] } }, + }); + await session.flushMetadata(); + await session.close(); + + session = makeSession(); + await session.resume(); + await session.mcp.waitForInitialLoad(); + const resumed = await session.ensureAgentResumed('main'); + expect(resumed.tools.loopTools.map((tool) => tool.name)).toContain('Read'); + expect(resumed.tools.loopTools.map((tool) => tool.name)).toContain('mcp__github__echo'); + } finally { + await session.close(); + } + }, 20000); }); describe('AgentAPI.startBtw', () => { diff --git a/packages/server/test/sessions.e2e.test.ts b/packages/server/test/sessions.e2e.test.ts index a117b80a..0a6776e2 100644 --- a/packages/server/test/sessions.e2e.test.ts +++ b/packages/server/test/sessions.e2e.test.ts @@ -41,7 +41,7 @@ import type { TelemetryClient, TelemetryProperties } from '@pymodel/agent-core'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { WebSocket } from 'ws'; -import { IRestGateway, startServer, type RunningServer } from '../src'; +import { ICoreProcessService, IRestGateway, startServer, type RunningServer } from '../src'; let tmpDir: string; let lockPath: string; @@ -422,6 +422,40 @@ describe('GET /api/v1/sessions/{session_id}/status — fetch live status', () => }); describe('POST /api/v1/sessions/{session_id}/profile — update profile', () => { + it('updates tool and MCP server selections after the session becomes inactive', async () => { + const r = await bootDaemon(); + const created = envelopeOf<{ id: string }>( + (await appOf(r).inject({ + method: 'POST', + url: '/api/v1/sessions', + payload: { metadata: { cwd: join(tmpDir, 'workspace-profile-tools') } }, + })).json(), + ).data!; + await r.services.invokeFunction((a) => + a.get(ICoreProcessService).rpc.closeSession({ sessionId: created.id }), + ); + + const updateRes = await appOf(r).inject({ + method: 'POST', + url: `/api/v1/sessions/${created.id}/profile`, + payload: { + agent_config: { + tools: ['Read', 'Bash'], + mcp_servers: ['github', 'slack'], + }, + }, + }); + expect(envelopeOf(updateRes.json()).code).toBe(0); + + const getRes = await appOf(r).inject({ + method: 'GET', + url: `/api/v1/sessions/${created.id}`, + }); + const session = sessionSchema.parse(envelopeOf(getRes.json()).data); + expect(session.agent_config.tools).toEqual(['Read', 'Bash']); + expect(session.agent_config.mcp_servers).toEqual(['github', 'slack']); + }); + it('updates the title and returns the post-update Session', async () => { const r = await bootDaemon(); const cwd = join(tmpDir, 'workspace-profile-update');