Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/acp-local-execution-and-stdio-mcp.md
Original file line numberDiff line numberDiff line change
@@ -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.
5 changes: 5 additions & 0 deletions .changeset/guard-background-questions.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
"@pymodel/pythinker-code": patch
---

Prevent AskUserQuestion from starting background tasks when task controls are unavailable.
5 changes: 5 additions & 0 deletions .changeset/stable-session-system-prompt.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
"@pymodel/pythinker-code": patch
---

Keep the system prompt unchanged for the rest of a session when AGENTS.md is edited.
51 changes: 41 additions & 10 deletions packages/acp-server/src/acp-terminal/acpTerminalRunner.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' &&
Expand All@@ -47,18 +64,16 @@ class AcpProcessService implements IHostProcessService {
private readonly sessionId: string,
private readonly cwd: string,
private readonly connection: IAcpConnection,
private readonly local: IHostProcessService,
) {}

async spawn(
command: string,
args: readonly string[] = [],
options?: HostProcessOptions,
): Promise<IHostProcess> {
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({
Expand DownExpand Up@@ -178,6 +193,7 @@ class AcpSessionRuntime implements Runtime {
cwd: string,
connection: IAcpConnection,
environment: IHostEnvironment,
local: IHostProcessService,
) {
this.identity = {
workspaceId,
Expand DownExpand Up@@ -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 {}
Expand All@@ -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;
Expand All@@ -241,7 +265,7 @@ class AcpWorkspaceRuntimeAttachment implements RuntimeProviderAttachment {
async dispose(): Promise<void> {
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();
}
}

Expand All@@ -253,14 +277,21 @@ export class AcpRuntimeProviderFactory implements RuntimeProviderFactory {
constructor(
private readonly connection: IAcpConnection,
private readonly environment: IHostEnvironment,
private readonly local: IHostProcessService,
) {}

static runtimeId(sessionId: string): string {
return `acp:${sessionId}`;
}

async attach(workspace: RuntimeProviderContext, host: RuntimeProviderHost): Promise<RuntimeProviderAttachment> {
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 () => {
Expand Down
9 changes: 8 additions & 1 deletion packages/acp-server/src/convert.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -176,7 +176,14 @@ export function acpMcpServersToConfigRecord(
const out: Record<string, McpServerConfig> = {};
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] = {
Expand Down
7 changes: 6 additions & 1 deletion packages/acp-server/src/start.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,7 @@ import {
IAgentRuntimeBindingService,
IAppendLogStore,
IHostEnvironment,
IHostProcessService,
ISessionContext,
ISessionIndexMirror,
IWorkspaceInstanceManager,
Expand DownExpand Up@@ -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<string, string>();
server = new AcpServer(client, klient, acpConnection, {
Expand Down
153 changes: 146 additions & 7 deletions packages/acp-server/test/acp-terminal.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,54 @@
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: () => {},
onTerminalCreated: () => () => {},
};
}

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> = {}): IHostEnvironment {
return {
_serviceBrand: undefined,
Expand All@@ -41,15 +64,22 @@ function makeEnvironment(overrides: Partial<IHostEnvironment> = {}): IHostEnviro
} as IHostEnvironment;
}

async function bindRuntime(environment: IHostEnvironment): Promise<Runtime> {
async function bindRuntime(
environment: IHostEnvironment,
options: { connection?: IAcpConnection; local?: IHostProcessService } = {},
): Promise<Runtime> {
const runtimes: Runtime[] = [];
const host = {
registerRuntime: (runtime: Runtime) => {
runtimes.push(runtime);
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];
Expand All@@ -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',
Expand DownExpand Up@@ -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' },
});
});
});
Loading
Loading