diff --git a/.changeset/acp-local-execution-and-stdio-mcp.md b/.changeset/acp-local-execution-and-stdio-mcp.md new file mode 100644 index 000000000..197c90794 --- /dev/null +++ b/.changeset/acp-local-execution-and-stdio-mcp.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": patch +--- + +Run a command locally when an ACP client provides no terminal or the command is not a shell, accept stdio MCP servers in ACP sessions, and let a reloaded ACP session bind its runtime again. diff --git a/.changeset/guard-background-questions.md b/.changeset/guard-background-questions.md new file mode 100644 index 000000000..c6800ebe3 --- /dev/null +++ b/.changeset/guard-background-questions.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": patch +--- + +Prevent AskUserQuestion from starting background tasks when task controls are unavailable. diff --git a/.changeset/stable-session-system-prompt.md b/.changeset/stable-session-system-prompt.md new file mode 100644 index 000000000..61d93d290 --- /dev/null +++ b/.changeset/stable-session-system-prompt.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": patch +--- + +Keep the system prompt unchanged for the rest of a session when AGENTS.md is edited. diff --git a/packages/acp-server/src/acp-terminal/acpTerminalRunner.ts b/packages/acp-server/src/acp-terminal/acpTerminalRunner.ts index a92533499..c65639a04 100644 --- a/packages/acp-server/src/acp-terminal/acpTerminalRunner.ts +++ b/packages/acp-server/src/acp-terminal/acpTerminalRunner.ts @@ -24,8 +24,25 @@ const OUTPUT_BYTE_LIMIT = 4 * 1024 * 1024; const OUTPUT_POLL_MS = 250; let nextGeneration = 1; -function isBashToolInvocation(args: readonly string[], options?: HostProcessOptions): boolean { +const SHELL_EXECUTABLES = new Set(['sh', 'bash', 'zsh', 'dash', 'ksh', 'fish']); + +/** + * The Bash tool always spawns the configured shell. Classifying the executable + * keeps another caller's `-c` invocation — `python -c ...` carrying the same + * non-interactive env — on the local path, where the client cannot refuse it. + */ +function isShellExecutable(command: string): boolean { + const base = (command.split(/[\\/]/).pop() ?? command).toLowerCase(); + return SHELL_EXECUTABLES.has(base.endsWith('.exe') ? base.slice(0, -4) : base); +} + +function isBashToolInvocation( + command: string, + args: readonly string[], + options?: HostProcessOptions, +): boolean { return ( + isShellExecutable(command) && args.length === 2 && args[0] === '-c' && options?.env?.['NO_COLOR'] === '1' && @@ -47,6 +64,7 @@ class AcpProcessService implements IHostProcessService { private readonly sessionId: string, private readonly cwd: string, private readonly connection: IAcpConnection, + private readonly local: IHostProcessService, ) {} async spawn( @@ -54,11 +72,8 @@ class AcpProcessService implements IHostProcessService { args: readonly string[] = [], options?: HostProcessOptions, ): Promise { - if (!this.connection.terminalEnabled) { - throw new Error('ACP terminal capability is unavailable'); - } - if (!isBashToolInvocation(args, options)) { - throw new Error('ACP runtime only supports interactive Bash tool processes'); + if (!this.connection.terminalEnabled || !isBashToolInvocation(command, args, options)) { + return this.local.spawn(command, args, { ...options, cwd: options?.cwd ?? this.cwd }); } const handle = await this.connection.get().createTerminal({ @@ -178,6 +193,7 @@ class AcpSessionRuntime implements Runtime { cwd: string, connection: IAcpConnection, environment: IHostEnvironment, + local: IHostProcessService, ) { this.identity = { workspaceId, @@ -205,7 +221,7 @@ class AcpSessionRuntime implements Runtime { dirname: (p: string) => path.dirname(p), }; this.fs = new AcpHostFileSystem({ sessionId } as unknown as ISessionContext, connection); - this.process = new AcpProcessService(sessionId, cwd, connection); + this.process = new AcpProcessService(sessionId, cwd, connection, local); } dispose(): void {} @@ -219,13 +235,21 @@ class AcpWorkspaceRuntimeAttachment implements RuntimeProviderAttachment { private readonly host: RuntimeProviderHost, private readonly connection: IAcpConnection, private readonly environment: IHostEnvironment, + private readonly local: IHostProcessService, ) {} bindSession(sessionId: string, cwd: string): string { const runtimeId = AcpRuntimeProviderFactory.runtimeId(sessionId); if (this.sessions.has(sessionId)) return runtimeId; const registration = this.host.registerRuntime( - new AcpSessionRuntime(this.workspace.id, sessionId, cwd, this.connection, this.environment), + new AcpSessionRuntime( + this.workspace.id, + sessionId, + cwd, + this.connection, + this.environment, + this.local, + ), ); this.sessions.set(sessionId, registration); return runtimeId; @@ -241,7 +265,7 @@ class AcpWorkspaceRuntimeAttachment implements RuntimeProviderAttachment { async dispose(): Promise { const registrations = [...this.sessions.values()]; this.sessions.clear(); - for (const registration of registrations.reverse()) await registration.remove(); + for (const registration of registrations.toReversed()) await registration.remove(); } } @@ -253,6 +277,7 @@ export class AcpRuntimeProviderFactory implements RuntimeProviderFactory { constructor( private readonly connection: IAcpConnection, private readonly environment: IHostEnvironment, + private readonly local: IHostProcessService, ) {} static runtimeId(sessionId: string): string { @@ -260,7 +285,13 @@ export class AcpRuntimeProviderFactory implements RuntimeProviderFactory { } async attach(workspace: RuntimeProviderContext, host: RuntimeProviderHost): Promise { - const attachment = new AcpWorkspaceRuntimeAttachment(workspace, host, this.connection, this.environment); + const attachment = new AcpWorkspaceRuntimeAttachment( + workspace, + host, + this.connection, + this.environment, + this.local, + ); this.attachments.set(workspace.id, attachment); return { dispose: async () => { diff --git a/packages/acp-server/src/convert.ts b/packages/acp-server/src/convert.ts index 5e2c2939a..d7d921756 100644 --- a/packages/acp-server/src/convert.ts +++ b/packages/acp-server/src/convert.ts @@ -176,7 +176,14 @@ export function acpMcpServersToConfigRecord( const out: Record = {}; for (const server of servers) { if (!('type' in server)) { - throw new Error(`ACP stdio MCP server ${server.name} does not declare a runtime identity`); + out[server.name] = { + transport: 'stdio', + command: server.command, + args: server.args, + env: namedPairsToRecord(server.env), + runtime_id: 'local', + }; + continue; } if (server.type === 'http' || server.type === 'sse') { out[server.name] = { diff --git a/packages/acp-server/src/start.ts b/packages/acp-server/src/start.ts index dc57e9d26..1ecb2ca92 100644 --- a/packages/acp-server/src/start.ts +++ b/packages/acp-server/src/start.ts @@ -25,6 +25,7 @@ import { IAgentRuntimeBindingService, IAppendLogStore, IHostEnvironment, + IHostProcessService, ISessionContext, ISessionIndexMirror, IWorkspaceInstanceManager, @@ -141,7 +142,11 @@ export async function runAcpServerWithStream( // `IAcpConnection.get()`. acpConnection.bind(client); const workspaceManager = core.accessor.get(IWorkspaceInstanceManager); - const acpRuntimeProvider = new AcpRuntimeProviderFactory(acpConnection, core.accessor.get(IHostEnvironment)); + const acpRuntimeProvider = new AcpRuntimeProviderFactory( + acpConnection, + core.accessor.get(IHostEnvironment), + core.accessor.get(IHostProcessService), + ); const acpProviderRegistration = await workspaceManager.addProvider(acpRuntimeProvider); const sessionWorkspaces = new Map(); server = new AcpServer(client, klient, acpConnection, { diff --git a/packages/acp-server/test/acp-terminal.test.ts b/packages/acp-server/test/acp-terminal.test.ts index d158d2238..721a063b9 100644 --- a/packages/acp-server/test/acp-terminal.test.ts +++ b/packages/acp-server/test/acp-terminal.test.ts @@ -1,24 +1,29 @@ import { describe, expect, it } from 'vitest'; import type { + HostProcessOptions, IHostEnvironment, + IHostProcess, + IHostProcessService, Runtime, RuntimeProviderHost, } from '@pymodel/agent-core-v2'; -import type { IAcpConnection } from '../src/acp-fs/acpConnection'; +import type { IAcpConnection, IAcpTerminalHandle } from '../src/acp-fs/acpConnection'; import { AcpHostFileSystem } from '../src/acp-fs/acpFsService'; import { AcpRuntimeProviderFactory } from '../src/acp-terminal/acpTerminalRunner'; -function makeConnection(): IAcpConnection { +function makeConnection( + options: { terminalEnabled?: boolean; createTerminal?: () => IAcpTerminalHandle } = {}, +): IAcpConnection { return { _serviceBrand: undefined, bound: true, fsReadTextFile: true, fsWriteTextFile: true, - terminalEnabled: true, + terminalEnabled: options.terminalEnabled ?? true, bind: () => {}, - get: () => ({}) as never, + get: () => ({ createTerminal: async () => options.createTerminal?.() }) as never, bindFsCapabilities: () => {}, bindTerminalCapability: () => {}, notifyTerminalCreated: () => {}, @@ -26,6 +31,24 @@ function makeConnection(): IAcpConnection { }; } +interface LocalSpawnCall { + readonly command: string; + readonly args: readonly string[]; + readonly options: HostProcessOptions | undefined; +} + +function makeLocalProcessService(): { local: IHostProcessService; calls: LocalSpawnCall[] } { + const calls: LocalSpawnCall[] = []; + const local: IHostProcessService = { + _serviceBrand: undefined, + spawn: async (command, args = [], options) => { + calls.push({ command, args, options }); + return {} as IHostProcess; + }, + }; + return { local, calls }; +} + function makeEnvironment(overrides: Partial = {}): IHostEnvironment { return { _serviceBrand: undefined, @@ -41,7 +64,10 @@ function makeEnvironment(overrides: Partial = {}): IHostEnviro } as IHostEnvironment; } -async function bindRuntime(environment: IHostEnvironment): Promise { +async function bindRuntime( + environment: IHostEnvironment, + options: { connection?: IAcpConnection; local?: IHostProcessService } = {}, +): Promise { const runtimes: Runtime[] = []; const host = { registerRuntime: (runtime: Runtime) => { @@ -49,7 +75,11 @@ async function bindRuntime(environment: IHostEnvironment): Promise { return { remove: async () => {} }; }, } as unknown as RuntimeProviderHost; - const factory = new AcpRuntimeProviderFactory(makeConnection(), environment); + const factory = new AcpRuntimeProviderFactory( + options.connection ?? makeConnection(), + environment, + options.local ?? makeLocalProcessService().local, + ); await factory.attach({ id: 'w1' } as never, host); factory.bindSession('w1', 's1', '/repo'); const runtime = runtimes[0]; @@ -61,7 +91,7 @@ describe('AcpSessionRuntime', () => { it('mirrors the probed host environment and exposes fs + process capabilities', async () => { const runtime = await bindRuntime(makeEnvironment()); - expect([...runtime.capabilities].sort()).toEqual(['fs', 'process']); + expect([...runtime.capabilities].toSorted()).toEqual(['fs', 'process']); expect(runtime.environment).toMatchObject({ osKind: 'macOS', osArch: 'arm64', @@ -98,3 +128,112 @@ describe('AcpSessionRuntime', () => { expect(runtime.path.resolve('C:\\repo', 'src')).toBe('C:\\repo\\src'); }); }); + +describe('AcpProcessService local fallback', () => { + const bashEnv = { NO_COLOR: '1', TERM: 'dumb' }; + + function makeTerminalHandle(): IAcpTerminalHandle { + return { + id: 'term-1', + currentOutput: async () => ({ output: '', truncated: false }), + waitForExit: async () => ({ exitCode: 0 }), + kill: async () => ({}), + release: async () => ({}), + }; + } + + it('runs Bash-shaped spawns in the client terminal when the capability is advertised', async () => { + let created = 0; + const connection = makeConnection({ + terminalEnabled: true, + createTerminal: () => { + created += 1; + return makeTerminalHandle(); + }, + }); + const { local, calls } = makeLocalProcessService(); + const runtime = await bindRuntime(makeEnvironment(), { connection, local }); + + await runtime.process!.spawn('/bin/bash', ['-c', 'echo hi'], { env: { ...bashEnv } }); + + expect(created).toBe(1); + expect(calls).toHaveLength(0); + }); + + it('falls back to local execution for Bash-shaped spawns without the terminal capability', async () => { + const connection = makeConnection({ terminalEnabled: false }); + const { local, calls } = makeLocalProcessService(); + const runtime = await bindRuntime(makeEnvironment(), { connection, local }); + + await runtime.process!.spawn('/bin/bash', ['-c', 'echo hi'], { env: { ...bashEnv } }); + + expect(calls).toHaveLength(1); + expect(calls[0]).toMatchObject({ + command: '/bin/bash', + args: ['-c', 'echo hi'], + options: { env: bashEnv, cwd: '/repo' }, + }); + }); + + it('falls back to local execution for a non-shell -c command carrying the Bash env', async () => { + let created = 0; + const connection = makeConnection({ + terminalEnabled: true, + createTerminal: () => { + created += 1; + return makeTerminalHandle(); + }, + }); + const { local, calls } = makeLocalProcessService(); + const runtime = await bindRuntime(makeEnvironment(), { connection, local }); + + await runtime.process!.spawn('python', ['-c', 'print(1)'], { env: { ...bashEnv } }); + + expect(created).toBe(0); + expect(calls).toHaveLength(1); + expect(calls[0]).toMatchObject({ command: 'python', args: ['-c', 'print(1)'] }); + }); + + it('routes a shell spawn to the terminal regardless of the shell binary or its path', async () => { + for (const shell of ['/bin/zsh', '/usr/local/bin/fish', 'C:\\Program Files\\Git\\bin\\bash.exe']) { + let created = 0; + const connection = makeConnection({ + terminalEnabled: true, + createTerminal: () => { + created += 1; + return makeTerminalHandle(); + }, + }); + const { local, calls } = makeLocalProcessService(); + const runtime = await bindRuntime(makeEnvironment(), { connection, local }); + + await runtime.process!.spawn(shell, ['-c', 'echo hi'], { env: { ...bashEnv } }); + + expect(created, shell).toBe(1); + expect(calls, shell).toHaveLength(0); + } + }); + + it('falls back to local execution for non-Bash spawns even with the terminal capability', async () => { + let created = 0; + const connection = makeConnection({ + terminalEnabled: true, + createTerminal: () => { + created += 1; + return makeTerminalHandle(); + }, + }); + const { local, calls } = makeLocalProcessService(); + const runtime = await bindRuntime(makeEnvironment(), { connection, local }); + + await runtime.process!.spawn('rg', ['--files', '--hidden']); + + expect(created).toBe(0); + expect(calls).toHaveLength(1); + expect(calls[0]).toMatchObject({ + command: 'rg', + args: ['--files', '--hidden'], + options: { cwd: '/repo' }, + }); + }); +}); diff --git a/packages/acp-server/test/convert.test.ts b/packages/acp-server/test/convert.test.ts index 0e15835b2..6f1ffa7f8 100644 --- a/packages/acp-server/test/convert.test.ts +++ b/packages/acp-server/test/convert.test.ts @@ -19,7 +19,7 @@ describe('acpMcpServersToConfigRecord', () => { expect(acpMcpServersToConfigRecord([])).toBeUndefined(); }); - it('rejects stdio servers that cannot declare a runtime identity', () => { + it('maps stdio servers (no type field) to local stdio configs', () => { const servers: McpServer[] = [ { name: 'fs', @@ -31,9 +31,15 @@ describe('acpMcpServersToConfigRecord', () => { ], }, ]; - expect(() => acpMcpServersToConfigRecord(servers)).toThrow( - 'ACP stdio MCP server fs does not declare a runtime identity', - ); + expect(acpMcpServersToConfigRecord(servers)).toEqual({ + fs: { + transport: 'stdio', + command: '/usr/local/bin/mcp-fs', + args: ['--root', '/tmp'], + env: { API_KEY: 'secret', DEBUG: '1' }, + runtime_id: 'local', + }, + }); }); it('maps http and sse servers with header pairs as a record', () => { diff --git a/packages/acp-server/test/e2e-turn.test.ts b/packages/acp-server/test/e2e-turn.test.ts index 8bd00860a..2e37c981c 100644 --- a/packages/acp-server/test/e2e-turn.test.ts +++ b/packages/acp-server/test/e2e-turn.test.ts @@ -991,7 +991,7 @@ describe('acp-server terminal reverse-RPC (clientCapabilities.terminal)', () => expect(JSON.stringify(secondCall)).toContain('hello_from_terminal'); }, 30_000); - it('rejects Bash without falling back when the client does not advertise terminal capability', async () => { + it('falls back to local execution when the client does not advertise the capability', async () => { const c = await boot({}); const terminals = fakeTerminalClient(c, 'should_not_be_used\n'); scriptBashTurn('echo hello_from_bash'); @@ -999,7 +999,7 @@ describe('acp-server terminal reverse-RPC (clientCapabilities.terminal)', () => const { stopReason } = await runPrompt(c); expect(stopReason).toBe('end_turn'); - // No terminal reverse-RPC at all — behavior identical to today. + // No terminal reverse-RPC at all — the command ran locally. expect(terminals).toHaveLength(0); const terminalRpcs = c.received.filter( (m) => typeof m.method === 'string' && m.method.startsWith('terminal/'), @@ -1009,7 +1009,6 @@ describe('acp-server terminal reverse-RPC (clientCapabilities.terminal)', () => // The tool card carries the textual output, exactly as before. const completed = toolCallUpdates(c).find((u) => u.status === 'completed'); const text = completed?.content?.map((entry) => entry.content?.text ?? '').join('\n') ?? ''; - expect(text).not.toContain('hello_from_bash'); - expect(JSON.stringify(scripted!.callHistory()[1])).toContain('ACP terminal capability is unavailable'); + expect(text).toContain('hello_from_bash'); }, 30_000); }); diff --git a/packages/acp-server/test/lifecycle.test.ts b/packages/acp-server/test/lifecycle.test.ts index 8fd638d88..9e9a837b3 100644 --- a/packages/acp-server/test/lifecycle.test.ts +++ b/packages/acp-server/test/lifecycle.test.ts @@ -291,10 +291,10 @@ describe('acp-server session lifecycle', () => { ); it( - 'session/new rejects stdio MCP servers without runtime identity', + 'session/new connects ACP mcpServers as ephemeral session servers', async () => { const c = await boot(); - await expect(c.send('session/new', { + const created = (await c.send('session/new', { cwd: homeDir, mcpServers: [ { @@ -304,16 +304,19 @@ describe('acp-server session lifecycle', () => { env: [{ name: 'PYTHINKER_TEST_MCP_START_DELAY_MS', value: '0' }], }, ], - })).rejects.toThrow('ACP stdio MCP server mock does not declare a runtime identity'); + })) as { sessionId: string }; + expect(created.sessionId).toMatch(/^session_/); // Engine-side assertion: the session scope's MCP handle is the overlay // view and the converted server ended up connected under its ACP name. + const entries = await sessionMcpEntries(c, created.sessionId); + expect(entries.find((e) => e.name === 'mock')?.status).toBe('connected'); }, 30_000, ); it( - 'session/load rejects stdio MCP servers without runtime identity', + 'session/load forwards mcpServers to the re-materialized session', async () => { const c = await boot(); const created = (await c.send('session/new', { cwd: homeDir, mcpServers: [] })) as { @@ -321,13 +324,16 @@ describe('acp-server session lifecycle', () => { }; await c.send('session/close', { sessionId: created.sessionId }); - await expect(c.send('session/load', { + await c.send('session/load', { sessionId: created.sessionId, cwd: homeDir, mcpServers: [ { name: 'mock', command: process.execPath, args: [STDIO_MCP_FIXTURE], env: [] }, ], - })).rejects.toThrow('ACP stdio MCP server mock does not declare a runtime identity'); + }); + + const entries = await sessionMcpEntries(c, created.sessionId); + expect(entries.find((e) => e.name === 'mock')?.status).toBe('connected'); }, 30_000, ); diff --git a/packages/agent-core-v2/src/agent/profile/profileService.ts b/packages/agent-core-v2/src/agent/profile/profileService.ts index 2b504afa0..4e887ccbf 100644 --- a/packages/agent-core-v2/src/agent/profile/profileService.ts +++ b/packages/agent-core-v2/src/agent/profile/profileService.ts @@ -182,11 +182,6 @@ export class AgentProfileService extends Disposable implements IAgentProfileServ event.waitUntil(this.refreshSystemPrompt()); }), ); - this._register( - this.instructions.onDidChange(() => { - void this.refreshSystemPrompt(); - }), - ); this._register( this.config.onDidSectionChange(({ domain }) => { if (domain === TOOLS_SECTION) { diff --git a/packages/agent-core-v2/src/agent/tools/ask-user-question/askUserQuestionTool.ts b/packages/agent-core-v2/src/agent/tools/ask-user-question/askUserQuestionTool.ts index 47129f7ee..38375003c 100644 --- a/packages/agent-core-v2/src/agent/tools/ask-user-question/askUserQuestionTool.ts +++ b/packages/agent-core-v2/src/agent/tools/ask-user-question/askUserQuestionTool.ts @@ -1,11 +1,10 @@ -import { z } from 'zod'; - import { CoreErrors } from '#/_base/errors/codes'; import { Error2 } from '#/_base/errors/errors'; import { toInputJsonSchema } from '#/tool/input-schema'; import { isAbortError } from '#/_base/utils/abort'; import { IAgentTaskService } from '#/agent/task/task'; import { IAgentScopeContext } from '#/agent/scopeContext/scopeContext'; +import { IAgentToolPolicyService } from '#/agent/toolPolicy/toolPolicy'; import { ITelemetryService } from '#/app/telemetry/telemetry'; import type { QuestionAnsweredEvent, QuestionDismissedEvent } from '#/app/telemetry/events'; import type { @@ -23,6 +22,7 @@ import type { QuestionResult, } from '#/session/question/question'; import { + AskUserQuestionInputSchema, AskUserQuestionInputSchemaWithBackground, IAskUserQuestionTool, questionUniquenessError, @@ -36,20 +36,33 @@ const QUESTION_DISMISSED_MESSAGE = 'User dismissed the question without answerin const QUESTION_UNSUPPORTED_FAILURE_MESSAGE = 'The connected client does not support interactive questions. Do NOT call this tool again. Ask the user directly in your text response instead.'; +const BACKGROUND_DESCRIPTION = + '- Set background=true when you can keep working without the answer. This starts a background question task and returns a task_id immediately. The answer arrives automatically in a later turn — you do not need to poll, sleep, or check on it. Continue with other work; never fabricate or predict the answer.'; + +const BACKGROUND_UNAVAILABLE_MESSAGE = + 'Background questions are not available for this agent because TaskList, TaskOutput, and TaskStop are not enabled.'; + +const PARAMETERS_WITH_BACKGROUND = toInputJsonSchema(AskUserQuestionInputSchemaWithBackground); +const PARAMETERS_FOREGROUND_ONLY = toInputJsonSchema(AskUserQuestionInputSchema); + export class AskUserQuestionTool implements IAskUserQuestionTool { declare readonly _serviceBrand: undefined; readonly name = 'AskUserQuestion' as const; - readonly description: string; - readonly parameters: Record; constructor( @ISessionQuestionService private readonly question: ISessionQuestionService, @ITelemetryService private readonly telemetry: ITelemetryService, @IAgentTaskService private readonly tasks: IAgentTaskService, @IAgentScopeContext private readonly scopeContext: IAgentScopeContext, - ) { - this.description = `${DESCRIPTION}- Set background=true when you can keep working without the answer. This starts a background question task and returns a task_id immediately. The answer arrives automatically in a later turn — you do not need to poll, sleep, or check on it. Continue with other work; never fabricate or predict the answer.`; - this.parameters = toInputJsonSchema(this.inputSchema()); + @IAgentToolPolicyService private readonly toolPolicy: IAgentToolPolicyService, + ) {} + + get description(): string { + return `${DESCRIPTION}${this.allowBackground() ? BACKGROUND_DESCRIPTION : ''}`; + } + + get parameters(): Record { + return this.allowBackground() ? PARAMETERS_WITH_BACKGROUND : PARAMETERS_FOREGROUND_ONLY; } resolveExecution(args: AskUserQuestionInput): ToolExecution { @@ -67,6 +80,10 @@ export class AskUserQuestionTool implements IAskUserQuestionTool { args: AskUserQuestionInput, { toolCallId, signal, turnId, trace }: ExecutableToolContext, ): Promise { + if (args.background === true && !this.allowBackground()) { + return { isError: true, output: BACKGROUND_UNAVAILABLE_MESSAGE }; + } + const uniquenessError = questionUniquenessError(args.questions); if (uniquenessError !== null) { return { isError: true, output: uniquenessError }; @@ -79,8 +96,12 @@ export class AskUserQuestionTool implements IAskUserQuestionTool { return this.executeQuestion(args, { toolCallId, turnId, signal, trace }); } - private inputSchema(): z.ZodType { - return AskUserQuestionInputSchemaWithBackground; + private allowBackground(): boolean { + return ( + this.toolPolicy.isToolActive('TaskList') && + this.toolPolicy.isToolActive('TaskOutput') && + this.toolPolicy.isToolActive('TaskStop') + ); } private executeInBackground( diff --git a/packages/agent-core-v2/src/runtime/runtimeUnitHost.ts b/packages/agent-core-v2/src/runtime/runtimeUnitHost.ts index 9554fbff4..be0a88736 100644 --- a/packages/agent-core-v2/src/runtime/runtimeUnitHost.ts +++ b/packages/agent-core-v2/src/runtime/runtimeUnitHost.ts @@ -201,7 +201,7 @@ class SharedRuntimeUnitHost implements RuntimeUnitHost { await this.enqueue(async () => { let failure: unknown; let failed = false; - for (const record of [...this.records].reverse()) { + for (const record of [...this.records].toReversed()) { if (!record.active) continue; record.active = false; try { @@ -304,7 +304,14 @@ class SharedRuntimeUnitHost implements RuntimeUnitHost { const handle: RuntimeProviderRuntimeHandle = { runtimeId: runtime.identity.runtimeId, update: (replacement) => this.updateRuntime(staged, replacement), - remove: () => this.removeRuntime(staged), + remove: async () => { + try { + await this.removeRuntime(staged); + } finally { + const index = runtimes.indexOf(staged); + if (index >= 0) runtimes.splice(index, 1); + } + }, }; return handle; }, @@ -360,7 +367,7 @@ class SharedRuntimeUnitHost implements RuntimeUnitHost { active = false; let failure: unknown; let failed = false; - for (const staged of runtimes.reverse()) { + for (const staged of runtimes.toReversed()) { if (!staged.active) continue; staged.active = false; try { @@ -371,10 +378,10 @@ class SharedRuntimeUnitHost implements RuntimeUnitHost { failed = true; } } - for (const registration of local.reverse()) { + for (const registration of local.toReversed()) { if (this.locals.get(registration.id) === registration) this.locals.delete(registration.id); } - for (const unit of units.reverse()) { + for (const unit of units.toReversed()) { try { await unit.dispose(); } catch (error) { diff --git a/packages/agent-core-v2/test/agent/profile/binding.test.ts b/packages/agent-core-v2/test/agent/profile/binding.test.ts index 0ba8136e3..5b13be24c 100644 --- a/packages/agent-core-v2/test/agent/profile/binding.test.ts +++ b/packages/agent-core-v2/test/agent/profile/binding.test.ts @@ -4,7 +4,7 @@ import { join, normalize } from 'pathe'; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; -import { Event } from '#/_base/event'; +import { Emitter, Event } from '#/_base/event'; import { InstantiationService } from '#/_base/di/instantiationService'; import { ServiceCollection } from '#/_base/di/serviceCollection'; import { ConfigTarget, IConfigService } from '#/app/config/config'; @@ -25,6 +25,7 @@ import { IAgentToolRegistryService } from '#/agent/toolRegistry/toolRegistry'; import { SELECT_TOOLS_TOOL_NAME } from '#/agent/toolSelect/toolSelect'; import { IAtomicDocumentStore, type IAtomicDocumentStore as AtomicDocumentStore } from '#/persistence/interface/atomicDocumentStore'; import { ISessionAgentProfileCatalog } from '#/session/sessionAgentProfileCatalog/sessionAgentProfileCatalog'; +import { ISessionInstructionsProvider } from '#/session/sessionInstructions/instructionsProvider'; import { ISessionSkillCatalog } from '#/session/sessionSkillCatalog/skillCatalog'; import { ISessionToolPolicy } from '#/session/sessionToolPolicy/sessionToolPolicy'; import { ISessionToolPolicyGate } from '#/session/sessionToolPolicyGate/sessionToolPolicyGate'; @@ -265,6 +266,45 @@ describe('AgentProfileService.bind', () => { } }); + it('freezes the system prompt when the session instructions change', async () => { + const persistence = new InMemoryWireRecordPersistence(); + const emitter = new Emitter(); + let agentsMd = 'v1 instructions'; + ctx = createTestAgent( + { persistence }, + hostEnvironmentServices(homeDir), + sessionService(ISessionInstructionsProvider, { + _serviceBrand: undefined, + ready: Promise.resolve(), + get agentsMd() { + return agentsMd; + }, + agentsMdWarning: undefined, + agentsMdPaths: [], + onDidChange: emitter.event, + } satisfies ISessionInstructionsProvider), + ); + const svc = ctx.get(IAgentProfileService); + await svc.bind({ profile: DEFAULT_AGENT_PROFILE_NAME, model: MOCK_MODEL }); + const before = svc.getSystemPrompt(); + expect(before).toContain('v1 instructions'); + await ctx.get(IWireService).flush(); + const configUpdates = () => + persistence.records.filter( + (record) => record.type === 'config.update' && 'systemPrompt' in record, + ); + const configUpdateCount = configUpdates().length; + + const refreshSpy = vi.spyOn(svc, 'refreshSystemPrompt'); + agentsMd = 'v2 instructions'; + emitter.fire(); + await ctx.get(IWireService).flush(); + + expect(refreshSpy).not.toHaveBeenCalled(); + expect(svc.getSystemPrompt()).toBe(before); + expect(configUpdates()).toHaveLength(configUpdateCount); + }); + it('setModel applies the default profile when none is bound yet', async () => { const { profile: svc } = buildContext(); diff --git a/packages/agent-core-v2/test/agent/questionTools/tools/ask-user.test.ts b/packages/agent-core-v2/test/agent/questionTools/tools/ask-user.test.ts index a4ebcc7b3..fa8165b89 100644 --- a/packages/agent-core-v2/test/agent/questionTools/tools/ask-user.test.ts +++ b/packages/agent-core-v2/test/agent/questionTools/tools/ask-user.test.ts @@ -1,24 +1,34 @@ -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { DisposableStore } from '#/_base/di/lifecycle'; +import { createServices } from '#/_base/di/test'; import { CoreErrors } from '#/_base/errors/codes'; import { Error2 } from '#/_base/errors/errors'; import { AskUserQuestionInputSchema, + IAskUserQuestionTool, type AskUserQuestionInput, } from '#/agent/tools/ask-user-question/ask-user-question'; import { AskUserQuestionTool } from '#/agent/tools/ask-user-question/askUserQuestionTool'; import { ITelemetryService } from '#/app/telemetry/telemetry'; import { IAgentTaskService } from '#/agent/task/task'; import { IAgentScopeContext } from '#/agent/scopeContext/scopeContext'; -import type { +import { IAgentToolPolicyService } from '#/agent/toolPolicy/toolPolicy'; +import { ISessionQuestionService, - QuestionRequest, - QuestionResult, + type QuestionRequest, + type QuestionResult, } from '#/session/question/question'; -import type { QuestionBackgroundTask } from '#/agent/tools/ask-user-question/question-background-task'; +import type { + QuestionBackgroundTask, + QuestionTaskInfo, +} from '#/agent/tools/ask-user-question/question-background-task'; import { executeTool } from '../../../tools/fixtures/execute-tool'; const signal = new AbortController().signal; +const TASK_TOOLS = new Set(['TaskList', 'TaskOutput', 'TaskStop']); + +let disposables: DisposableStore; function input( overrides: Partial = {}, @@ -41,13 +51,14 @@ function input( function makeTool( options: { + readonly activeTaskTools?: ReadonlySet; readonly request?: ( req: QuestionRequest, requestOptions?: { readonly signal?: AbortSignal }, ) => Promise; } = {}, ): { - readonly tool: AskUserQuestionTool; + readonly tool: IAskUserQuestionTool; readonly request: ReturnType; readonly telemetryTrack: ReturnType; readonly registerTask: ReturnType; @@ -56,23 +67,54 @@ function makeTool( } { const request = vi.fn(options.request ?? (async () => ({ Postgres: true }) as QuestionResult)); const telemetryTrack = vi.fn(); - const question = { request } as unknown as ISessionQuestionService; - const telemetry = { track2: telemetryTrack } as unknown as ITelemetryService; let lastTask: QuestionBackgroundTask | undefined; const registerTask = vi.fn((task: QuestionBackgroundTask) => { lastTask = task; return 'q_test_task_id'; }); - const getTask = vi.fn((id: string) => - id === 'q_test_task_id' ? { status: 'running' } : undefined, + const getTask = vi.fn( + (id: string): QuestionTaskInfo | undefined => + id === 'q_test_task_id' + ? { + taskId: id, + description: 'Which database?', + status: 'running', + detached: true, + startedAt: 0, + endedAt: null, + kind: 'question', + questionCount: 1, + toolCallId: 'call_bg', + } + : undefined, ); - const tasks = { registerTask, getTask } as unknown as IAgentTaskService; - const scopeContext = { agentId: 'main' } as unknown as IAgentScopeContext; - const tool = new AskUserQuestionTool(question, telemetry, tasks, scopeContext); + const activeTaskTools = options.activeTaskTools ?? TASK_TOOLS; + const ix = createServices(disposables, { + additionalServices: (reg) => { + reg.definePartialInstance(ISessionQuestionService, { request }); + reg.definePartialInstance(ITelemetryService, { track2: telemetryTrack }); + reg.definePartialInstance(IAgentTaskService, { registerTask, getTask }); + reg.definePartialInstance(IAgentScopeContext, { agentId: 'main' }); + reg.definePartialInstance(IAgentToolPolicyService, { + isToolActive: (name: string) => activeTaskTools.has(name), + }); + reg.define(IAskUserQuestionTool, AskUserQuestionTool); + }, + strict: true, + }); + const tool = ix.get(IAskUserQuestionTool); return { tool, request, telemetryTrack, registerTask, getTask, lastRegisteredTask: () => lastTask }; } describe('AskUserQuestionTool', () => { + beforeEach(() => { + disposables = new DisposableStore(); + }); + + afterEach(() => { + disposables.dispose(); + }); + it('exposes current metadata and schema', () => { const { tool } = makeTool(); @@ -167,7 +209,7 @@ describe('AskUserQuestionTool', () => { expect(request).toHaveBeenCalledOnce(); }); - it('builds the v1-aligned schema including an optional background flag', () => { + it('exposes background mode when all task controls are active', () => { const { tool } = makeTool(); const params = tool.parameters as { properties: { background?: { type?: string; default?: boolean } }; @@ -175,6 +217,99 @@ describe('AskUserQuestionTool', () => { expect(params.properties.background?.type).toBe('boolean'); expect(params.properties.background?.default).toBe(false); + expect(tool.description).toContain('background=true'); + expect(tool.description).toContain('task_id'); + }); + + it('hides and rejects background mode after a task control becomes inactive', async () => { + const activeTaskTools = new Set(TASK_TOOLS); + const { tool, request, registerTask } = makeTool({ activeTaskTools }); + + expect(tool.parameters).toHaveProperty('properties.background'); + activeTaskTools.delete('TaskStop'); + + const params = tool.parameters as { properties: Record }; + + expect(params.properties).not.toHaveProperty('background'); + expect(tool.description.toLowerCase()).not.toContain('background'); + expect(tool.description).not.toContain('task_id'); + expect(tool.description).not.toContain('TaskOutput'); + + const result = await executeTool(tool, { + turnId: 0, + toolCallId: 'call_bg_disabled', + args: { ...input(), background: true }, + signal, + }); + + expect(result).toEqual({ + isError: true, + output: + 'Background questions are not available for this agent because TaskList, TaskOutput, and TaskStop are not enabled.', + }); + expect(registerTask).not.toHaveBeenCalled(); + expect(request).not.toHaveBeenCalled(); + }); + + it('preserves foreground answers when background mode is unavailable', async () => { + const { tool, request } = makeTool({ activeTaskTools: new Set() }); + + const result = await executeTool(tool, { + turnId: 0, + toolCallId: 'call_fg_disabled', + args: input(), + signal, + }); + + expect(result).toEqual({ + isError: false, + output: JSON.stringify({ answers: { Postgres: true } }), + }); + expect(request).toHaveBeenCalledOnce(); + }); + + it('preserves foreground dismissal when background mode is unavailable', async () => { + const { tool } = makeTool({ + activeTaskTools: new Set(), + request: async () => null, + }); + + const result = await executeTool(tool, { + turnId: 0, + toolCallId: 'call_fg_dismissed', + args: input(), + signal, + }); + + expect(result).toEqual({ + isError: false, + output: JSON.stringify({ + answers: {}, + note: 'User dismissed the question without answering.', + }), + }); + }); + + it('preserves foreground errors when background mode is unavailable', async () => { + const { tool } = makeTool({ + activeTaskTools: new Set(), + request: async () => { + throw new Error2(CoreErrors.codes.NOT_IMPLEMENTED, 'Client does not support questions'); + }, + }); + + const result = await executeTool(tool, { + turnId: 0, + toolCallId: 'call_fg_unsupported', + args: input(), + signal, + }); + + expect(result).toEqual({ + isError: true, + output: + 'The connected client does not support interactive questions. Do NOT call this tool again. Ask the user directly in your text response instead.', + }); }); it('dispatches questions through the session question service', async () => { diff --git a/packages/agent-core-v2/test/runtime/runtimeUnitHost.test.ts b/packages/agent-core-v2/test/runtime/runtimeUnitHost.test.ts index 9b9eec0e4..9bcbbc542 100644 --- a/packages/agent-core-v2/test/runtime/runtimeUnitHost.test.ts +++ b/packages/agent-core-v2/test/runtime/runtimeUnitHost.test.ts @@ -225,6 +225,56 @@ describe('RuntimeUnitHost', () => { disposables.dispose(); }); + it('re-registers the same runtime id after its registration was removed', async () => { + const { disposables, host, registry } = setup(); + let providerHost!: RuntimeProviderHost; + const handle = await host.provide(emptyImports(), async (provider) => { + providerHost = provider; + return { dispose: () => {} }; + }); + + const first = runtime('one'); + const registration = providerHost.registerRuntime(first); + await registration.remove(); + expect(registry.current('local')).toBeUndefined(); + + const second = runtime('two'); + providerHost.registerRuntime(second); + expect(registry.current('local')).toBe(second); + + await handle.remove(); + expect(registry.current('local')).toBeUndefined(); + expect(second.disposed).toBe(true); + await host.dispose(); + disposables.dispose(); + }); + + it('re-registers the same runtime id even when removal teardown fails', async () => { + const { disposables, host, registry } = setup(); + let providerHost!: RuntimeProviderHost; + const handle = await host.provide(emptyImports(), async (provider) => { + providerHost = provider; + return { dispose: () => {} }; + }); + + const failing = runtime('one'); + failing.dispose = () => { + throw new Error('boom'); + }; + const registration = providerHost.registerRuntime(failing); + await expect(registration.remove()).rejects.toThrow('boom'); + expect(registry.current('local')).toBeUndefined(); + + const second = runtime('two'); + providerHost.registerRuntime(second); + expect(registry.current('local')).toBe(second); + + await handle.remove(); + expect(registry.current('local')).toBeUndefined(); + await host.dispose(); + disposables.dispose(); + }); + it('waits for in-flight prepare, rejects new transactions, and tears down in reverse order', async () => { const { disposables, host } = setup(); const order: string[] = []; diff --git a/packages/agent-core/src/agent/tool/index.ts b/packages/agent-core/src/agent/tool/index.ts index 5cc605571..65de3e2bd 100644 --- a/packages/agent-core/src/agent/tool/index.ts +++ b/packages/agent-core/src/agent/tool/index.ts @@ -787,10 +787,11 @@ export class ToolManager { }, this.agent.skills?.registry.getSkillRoots() ?? [], ); - const allowBackground = + const canRunInBackground = () => this.isExactToolEnabled('TaskList') && this.isExactToolEnabled('TaskOutput') && this.isExactToolEnabled('TaskStop'); + const allowBackground = canRunInBackground(); const goalToolsEnabled = this.agent.type === 'main'; this.builtinTools = new Map( [ @@ -828,7 +829,8 @@ export class ToolManager { goalToolsEnabled && new b.GetGoalTool(this.agent), goalToolsEnabled && new b.SetGoalBudgetTool(this.agent), goalToolsEnabled && new b.UpdateGoalTool(this.agent), - this.agent.rpc?.requestQuestion && new b.AskUserQuestionTool(this.agent), + this.agent.rpc?.requestQuestion && + new b.AskUserQuestionTool(this.agent, { allowBackground: canRunInBackground }), new b.TodoListTool(this.toolStore), new b.TaskListTool(background), new b.TaskOutputTool(background), diff --git a/packages/agent-core/src/tools/builtin/collaboration/ask-user.ts b/packages/agent-core/src/tools/builtin/collaboration/ask-user.ts index c6466d08a..fa104cb32 100644 --- a/packages/agent-core/src/tools/builtin/collaboration/ask-user.ts +++ b/packages/agent-core/src/tools/builtin/collaboration/ask-user.ts @@ -128,16 +128,37 @@ const QUESTION_DISMISSED_MESSAGE = 'User dismissed the question without answerin const QUESTION_UNSUPPORTED_FAILURE_MESSAGE = 'The connected client does not support interactive questions. Do NOT call this tool again. Ask the user directly in your text response instead.'; +const BACKGROUND_DESCRIPTION = + '- Set background=true when you can keep working without the answer. This starts a background question task and returns a task_id immediately. The answer arrives automatically in a later turn — you do not need to poll, sleep, or check on it. Continue with other work; never fabricate or predict the answer.'; + +const BACKGROUND_UNAVAILABLE_MESSAGE = + 'Background questions are not available for this agent because TaskList, TaskOutput, and TaskStop are not enabled.'; + +const PARAMETERS_WITH_BACKGROUND = toInputJsonSchema(AskUserQuestionInputSchemaWithBackground); +const PARAMETERS_FOREGROUND_ONLY = toInputJsonSchema(AskUserQuestionInputSchema); + // ── Implementation ─────────────────────────────────────────────────── export class AskUserQuestionTool implements BuiltinTool { readonly name = 'AskUserQuestion' as const; - readonly description: string; - readonly parameters: Record; - constructor(private readonly agent: Agent) { - this.description = `${DESCRIPTION}- Set background=true when you can keep working without the answer. This starts a background question task and returns a task_id immediately. The answer arrives automatically in a later turn — you do not need to poll, sleep, or check on it. Continue with other work; never fabricate or predict the answer.`; - this.parameters = toInputJsonSchema(this.inputSchema()); + private readonly canRunInBackground: () => boolean; + + constructor( + private readonly agent: Agent, + options?: { allowBackground?: boolean | (() => boolean) }, + ) { + const allowBackground = options?.allowBackground ?? true; + this.canRunInBackground = + typeof allowBackground === 'function' ? allowBackground : () => allowBackground; + } + + get description(): string { + return `${DESCRIPTION}${this.canRunInBackground() ? BACKGROUND_DESCRIPTION : ''}`; + } + + get parameters(): Record { + return this.canRunInBackground() ? PARAMETERS_WITH_BACKGROUND : PARAMETERS_FOREGROUND_ONLY; } resolveExecution(args: AskUserQuestionInput): ToolExecution { @@ -160,6 +181,10 @@ export class AskUserQuestionTool implements BuiltinTool { turnId, }: ExecutableToolContext, ): Promise { + if (args.background === true && !this.canRunInBackground()) { + return { isError: true, output: BACKGROUND_UNAVAILABLE_MESSAGE }; + } + // AJV (the runtime arg validator) cannot express the uniqueness refine, // so enforce it here before any UI interaction or task registration. const uniquenessError = questionUniquenessError(args.questions); @@ -174,10 +199,6 @@ export class AskUserQuestionTool implements BuiltinTool { return this.executeQuestion(args, { toolCallId, turnId, signal, traceId }); } - private inputSchema(): z.ZodType { - return AskUserQuestionInputSchemaWithBackground; - } - private async executeQuestion( args: AskUserQuestionInput, { diff --git a/packages/agent-core/test/agent/tool.test.ts b/packages/agent-core/test/agent/tool.test.ts index 80d7b4235..82d5aa3db 100644 --- a/packages/agent-core/test/agent/tool.test.ts +++ b/packages/agent-core/test/agent/tool.test.ts @@ -326,6 +326,51 @@ describe('Agent tools', () => { expect(subagentHost.spawn).not.toHaveBeenCalled(); }); + it('rechecks AskUserQuestion background mode after the task policy changes', async () => { + const ctx = testAgent(); + ctx.configure(); + ctx.agent.tools.setActiveTools(['AskUserQuestion', 'TaskList', 'TaskOutput', 'TaskStop']); + + const retainedTool = ctx.agent.tools.loopTools.find( + (tool) => tool.name === 'AskUserQuestion', + ); + expect(retainedTool).toBeDefined(); + expect(retainedTool!.parameters).toHaveProperty('properties.background'); + expect(retainedTool!.description).toContain('background=true'); + + const registerTask = vi.spyOn(ctx.agent.background, 'registerTask'); + ctx.agent.tools.setActiveTools(['AskUserQuestion', 'TaskList', 'TaskOutput']); + + expect(retainedTool!.parameters).not.toHaveProperty('properties.background'); + expect(retainedTool!.description.toLowerCase()).not.toContain('background'); + await expect( + executeTool(retainedTool!, { + turnId: '0', + toolCallId: 'call_question', + args: { + background: true, + questions: [ + { + question: 'Which database?', + header: 'Storage', + options: [ + { label: 'Postgres', description: 'Relational storage' }, + { label: 'SQLite', description: 'Embedded storage' }, + ], + multi_select: false, + }, + ], + }, + signal, + }), + ).resolves.toEqual({ + isError: true, + output: + 'Background questions are not available for this agent because TaskList, TaskOutput, and TaskStop are not enabled.', + }); + expect(registerTask).not.toHaveBeenCalled(); + }); + it('removes denied exact tool names from the active set', () => { const ctx = testAgent(); ctx.configure(); diff --git a/packages/agent-core/test/tools/ask-user.test.ts b/packages/agent-core/test/tools/ask-user.test.ts index 6993d3ef3..64723dc0e 100644 --- a/packages/agent-core/test/tools/ask-user.test.ts +++ b/packages/agent-core/test/tools/ask-user.test.ts @@ -35,6 +35,7 @@ function input( function makeTool( options: { + readonly allowBackground?: boolean; readonly mode?: PermissionMode; readonly requestQuestion?: ( request: QuestionRequest, @@ -58,7 +59,11 @@ function makeTool( rpc: { requestQuestion }, telemetry: { track: telemetryTrack }, } as unknown as Agent; - return { tool: new AskUserQuestionTool(agent), requestQuestion, telemetryTrack }; + return { + tool: new AskUserQuestionTool(agent, { allowBackground: options.allowBackground }), + requestQuestion, + telemetryTrack, + }; } describe('AskUserQuestionTool', () => { @@ -185,16 +190,111 @@ describe('AskUserQuestionTool', () => { expect(requestQuestion).not.toHaveBeenCalled(); }); - it('always builds the background-question schema', () => { + it('keeps the background schema and description when background questions are allowed', () => { const agent = { rpc: { requestQuestion: vi.fn() }, telemetry: { track: vi.fn() }, background: createBackgroundManager().manager, } as unknown as Agent; - const tool = new AskUserQuestionTool(agent); + const tool = new AskUserQuestionTool(agent, { allowBackground: true }); expect(JSON.stringify(tool.parameters)).toContain('background'); + expect(tool.description).toContain('background=true'); + expect(tool.description).toContain('task_id'); + }); + + it('hides and rejects background questions when background is not allowed', async () => { + const { manager } = createBackgroundManager(); + const registerTask = vi.spyOn(manager, 'registerTask'); + const requestQuestion = vi.fn(); + const agent = { + rpc: { requestQuestion }, + telemetry: { track: vi.fn() }, + background: manager, + } as unknown as Agent; + const tool = new AskUserQuestionTool(agent, { allowBackground: false }); + + expect(JSON.stringify(tool.parameters)).not.toContain('background'); + expect(tool.description.toLowerCase()).not.toContain('background'); + expect(tool.description).not.toContain('task_id'); + expect(tool.description).not.toContain('TaskOutput'); + + const result = await executeTool(tool, { + turnId: '0', + toolCallId: 'call_bg_disabled', + args: { ...input(), background: true }, + signal, + }); + + expect(result).toEqual({ + isError: true, + output: + 'Background questions are not available for this agent because TaskList, TaskOutput, and TaskStop are not enabled.', + }); + expect(registerTask).not.toHaveBeenCalled(); + expect(requestQuestion).not.toHaveBeenCalled(); + }); + + it('preserves foreground answers when background questions are disabled', async () => { + const { tool, requestQuestion } = makeTool({ allowBackground: false }); + + const result = await executeTool(tool, { + turnId: '0', + toolCallId: 'call_fg_disabled', + args: input(), + signal, + }); + + expect(result).toEqual({ + isError: false, + output: JSON.stringify({ answers: { Postgres: true } }), + }); + expect(requestQuestion).toHaveBeenCalledOnce(); + }); + + it('preserves foreground dismissal semantics when background questions are disabled', async () => { + const { tool } = makeTool({ + allowBackground: false, + requestQuestion: async () => null, + }); + + const result = await executeTool(tool, { + turnId: '0', + toolCallId: 'call_fg_dismissed', + args: input(), + signal, + }); + + expect(result).toEqual({ + isError: false, + output: JSON.stringify({ + answers: {}, + note: 'User dismissed the question without answering.', + }), + }); + }); + + it('preserves foreground error semantics when background questions are disabled', async () => { + const { tool } = makeTool({ + allowBackground: false, + requestQuestion: async () => { + throw new PythinkerError(ErrorCodes.NOT_IMPLEMENTED, 'Client does not support questions'); + }, + }); + + const result = await executeTool(tool, { + turnId: '0', + toolCallId: 'call_fg_unsupported', + args: input(), + signal, + }); + + expect(result).toEqual({ + isError: true, + output: + 'The connected client does not support interactive questions. Do NOT call this tool again. Ask the user directly in your text response instead.', + }); }); it.each(['manual', 'yolo'] as const)(