From 0d3b1fa3e817a297a3f95643442211665240e523 Mon Sep 17 00:00:00 2001 From: widat Date: Tue, 18 Aug 2026 09:51:46 +0700 Subject: [PATCH] feat: add ability to support multi-root workspace --- README.md | 4 + src/acp/agent.ts | 28 +++-- src/acp/session.ts | 14 ++- src/acp/workspace-roots.ts | 34 ++++++ src/pi-rpc/process.ts | 52 +++++++++ ...new-session-additional-directories.test.ts | 109 ++++++++++++++++++ test/unit/session-restore.test.ts | 6 +- test/unit/workspace-roots.test.ts | 54 +++++++++ 8 files changed, 289 insertions(+), 12 deletions(-) create mode 100644 src/acp/workspace-roots.ts create mode 100644 test/unit/new-session-additional-directories.test.ts create mode 100644 test/unit/workspace-roots.test.ts diff --git a/README.md b/README.md index 75925f45..d74f94aa 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,9 @@ Expect some minor breaking changes. - Session persistence - pi stores its own sessions in `~/.pi/agent/sessions/...` - `pi-acp` stores a small mapping file at `~/.pi/pi-acp/session-map.json` so `session/load` can reattach to a previous pi session file +- Multi-workspace support (`sessionCapabilities.additionalDirectories`) + - ACP clients can pass additional workspace roots on `session/new` / `session/load` (e.g. Zed multi-root workspaces) + - `cwd` stays the primary working directory; the additional roots are communicated to pi via `--append-system-prompt`, since pi has no native multi-root workspace concept - Slash commands - Loads file-based slash commands compatible with pi’s conventions - Adds a small set of built-in commands for headless/editor usage @@ -196,6 +199,7 @@ Project layout: - No ACP filesystem delegation (`fs/*`) and no ACP terminal delegation (`terminal/*`). pi reads/writes and executes locally. - MCP servers are accepted in ACP params and stored in session state, but not wired through to pi in this adapter. If you use [pi MCP adapter](https://github.com/nicobailon/pi-mcp-adapter) it will be available in the ACP client. +- Additional workspace roots are not a hard filesystem boundary: pi can operate outside them. They are communicated to the model (workspace awareness), not enforced as a sandbox. - Assistant streaming is currently sent as `agent_message_chunk` (no separate thought stream). - Queue is implemented client-side and should work like pi's `one-at-a-time` - ~~ACP clients don't yet suport session history, but ACP sessions from `pi-acp` can be `/resume`d in pi directly~~ diff --git a/src/acp/agent.ts b/src/acp/agent.ts index 381b04d8..f9f63004 100644 --- a/src/acp/agent.ts +++ b/src/acp/agent.ts @@ -50,6 +50,7 @@ import { existsSync, readFileSync, realpathSync, readdirSync, statSync, unlinkSy import type { AvailableCommand } from '@agentclientprotocol/sdk' import { join, dirname, basename } from 'node:path' import { spawnSync } from 'node:child_process' +import { normalizeAdditionalDirectories } from './workspace-roots.js' type ThinkingLevel = 'off' | 'minimal' | 'low' | 'medium' | 'high' | 'xhigh' type AdvertisedModel = { @@ -180,7 +181,7 @@ export class PiAcpAgent implements ACPAgent { private async restoreSession( sessionId: string, - opts?: { cwd?: string; mcpServers?: LoadSessionRequest['mcpServers'] } + opts?: { cwd?: string; mcpServers?: LoadSessionRequest['mcpServers']; additionalDirectories?: string[] } ): Promise { const existing = this.sessions.maybeGet(sessionId) if (existing) return existing @@ -201,7 +202,8 @@ export class PiAcpAgent implements ACPAgent { proc = await PiRpcProcess.spawn({ cwd, sessionPath: stored.sessionFile, - piCommand: process.env.PI_ACP_PI_COMMAND + piCommand: process.env.PI_ACP_PI_COMMAND, + additionalDirectories: opts?.additionalDirectories }) } catch (e: any) { if (e?.name === 'PiRpcSpawnError') { @@ -216,7 +218,8 @@ export class PiAcpAgent implements ACPAgent { mcpServers: opts?.mcpServers ?? [], conn: this.conn, proc, - fileCommands + fileCommands, + additionalDirectories: opts?.additionalDirectories }) this.lastSessionCwd = cwd @@ -263,7 +266,8 @@ export class PiAcpAgent implements ACPAgent { // **UNSTABLE** ACP capability used by Zed's codex-acp adapter. // Enables a native session picker in clients that support it. list: {}, - delete: {} + delete: {}, + additionalDirectories: {} } } } @@ -274,6 +278,8 @@ export class PiAcpAgent implements ACPAgent { throw RequestError.invalidParams(`cwd must be an absolute path: ${params.cwd}`) } + const additionalDirectories = normalizeAdditionalDirectories(params.additionalDirectories, params.cwd) + this.lastSessionCwd = params.cwd const fileCommands = loadSlashCommands(params.cwd) @@ -285,7 +291,8 @@ export class PiAcpAgent implements ACPAgent { mcpServers: params.mcpServers, conn: this.conn, fileCommands, - piCommand: process.env.PI_ACP_PI_COMMAND + piCommand: process.env.PI_ACP_PI_COMMAND, + additionalDirectories }) // Fetch state + models once (parallel) to reduce startup latency. @@ -363,7 +370,8 @@ export class PiAcpAgent implements ACPAgent { : buildStartupInfo({ cwd: params.cwd, fileCommands, - updateNotice + updateNotice, + additionalDirectories }) if (preludeText) @@ -945,9 +953,11 @@ export class PiAcpAgent implements ACPAgent { } const enableSkillCommands = getEnableSkillCommands(params.cwd) + const additionalDirectories = normalizeAdditionalDirectories(params.additionalDirectories, params.cwd) const session = await this.restoreSession(params.sessionId, { cwd: params.cwd, - mcpServers: params.mcpServers + mcpServers: params.mcpServers, + additionalDirectories }) const proc = session.proc const fileCommands = loadSlashCommands(params.cwd) @@ -1489,6 +1499,7 @@ function buildStartupInfo(opts: { cwd: string fileCommands: ReturnType updateNotice: string | null + additionalDirectories?: string[] }): string { void opts.fileCommands @@ -1525,6 +1536,9 @@ function buildStartupInfo(opts: { if (existsSync(contextPath)) contextItems.push(contextPath) addSection('Context', contextItems) + // Additional workspace roots (ACP additionalDirectories) + addSection('Additional workspace roots', opts.additionalDirectories ?? []) + // Skills const skillsItems: string[] = [] diff --git a/src/acp/session.ts b/src/acp/session.ts index d2fae685..59ca51ae 100644 --- a/src/acp/session.ts +++ b/src/acp/session.ts @@ -34,6 +34,8 @@ type SessionCreateParams = { conn: AgentSideConnection fileCommands?: import('./slash-commands.js').FileSlashCommand[] piCommand?: string + /** ACP additionalDirectories: extra workspace roots beyond cwd (absolute paths). */ + additionalDirectories?: string[] } export type StopReason = 'end_turn' | 'cancelled' | 'error' @@ -191,7 +193,8 @@ export class SessionManager { try { proc = await PiRpcProcess.spawn({ cwd: params.cwd, - piCommand: params.piCommand + piCommand: params.piCommand, + additionalDirectories: params.additionalDirectories }) } catch (e) { if (e instanceof PiRpcSpawnError) { @@ -220,7 +223,8 @@ export class SessionManager { mcpServers: params.mcpServers, proc, conn: params.conn, - fileCommands: params.fileCommands ?? [] + fileCommands: params.fileCommands ?? [], + additionalDirectories: params.additionalDirectories ?? [] }) this.sessions.set(sessionId, session) @@ -247,7 +251,8 @@ export class SessionManager { mcpServers: params.mcpServers, proc: params.proc, conn: params.conn, - fileCommands: params.fileCommands ?? [] + fileCommands: params.fileCommands ?? [], + additionalDirectories: params.additionalDirectories ?? [] }) this.sessions.set(sessionId, session) @@ -259,6 +264,7 @@ export class PiAcpSession { readonly sessionId: string readonly cwd: string readonly mcpServers: McpServer[] + readonly additionalDirectories: string[] private startupInfo: string | null = null private startupInfoSent = false @@ -303,10 +309,12 @@ export class PiAcpSession { proc: PiRpcProcess conn: AgentSideConnection fileCommands?: FileSlashCommand[] + additionalDirectories?: string[] }) { this.sessionId = opts.sessionId this.cwd = opts.cwd this.mcpServers = opts.mcpServers + this.additionalDirectories = opts.additionalDirectories ?? [] this.proc = opts.proc this.conn = opts.conn this.fileCommands = opts.fileCommands ?? [] diff --git a/src/acp/workspace-roots.ts b/src/acp/workspace-roots.ts new file mode 100644 index 00000000..5708b5e8 --- /dev/null +++ b/src/acp/workspace-roots.ts @@ -0,0 +1,34 @@ +import { RequestError } from '@agentclientprotocol/sdk' +import { isAbsolute, resolve as resolvePath } from 'node:path' + +/** + * Validate and normalize the ACP `additionalDirectories` request field + * (https://agentclientprotocol.com/protocol/v1/session-setup#additional-workspace-roots). + * + * Every entry MUST be an absolute path. `cwd` stays the primary root, so an entry + * equal to it is dropped. Order is preserved and duplicates are removed. + */ +export function normalizeAdditionalDirectories(input: readonly string[] | null | undefined, cwd: string): string[] { + if (!input || input.length === 0) return [] + + const resolvedCwd = resolvePath(cwd) + const out: string[] = [] + + for (const entry of input) { + if (typeof entry !== 'string') { + throw RequestError.invalidParams(`additionalDirectories entries must be absolute paths: ${String(entry)}`) + } + + const dir = entry.trim() + if (!dir) continue + + if (!isAbsolute(dir)) { + throw RequestError.invalidParams(`additionalDirectories entries must be absolute paths: ${entry}`) + } + + if (resolvePath(dir) === resolvedCwd) continue + if (!out.includes(dir)) out.push(dir) + } + + return out +} diff --git a/src/pi-rpc/process.ts b/src/pi-rpc/process.ts index 92f7959c..7ff4cdc3 100644 --- a/src/pi-rpc/process.ts +++ b/src/pi-rpc/process.ts @@ -1,4 +1,7 @@ import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import * as readline from 'node:readline' import { getPiCommand, shouldUseShellForPiCommand } from './command.js' @@ -74,6 +77,27 @@ type SpawnParams = { piCommand?: string /** If set, pi will persist the session to this exact file (via `--session `). */ sessionPath?: string + /** + * Additional workspace roots (ACP `additionalDirectories`). Communicated to pi via + * `--append-system-prompt` since pi has no native multi-root workspace support. + */ + additionalDirectories?: readonly string[] +} + +/** + * Build the system-prompt text describing the session's workspace root set. + * pi appends this to its system prompt so the model treats the additional + * directories as part of the user's workspace. + */ +export function buildWorkspaceRootsPrompt(cwd: string, dirs: readonly string[]): string { + return [ + '', + `Primary working directory: ${cwd}`, + "Additional workspace roots (also part of the user's workspace):", + ...dirs.map(dir => `- ${dir}`), + '', + "The user's workspace spans the primary working directory and the additional workspace roots listed above. Treat files under the additional roots as part of the workspace: read, search, and edit them using absolute paths. Relative paths still resolve against the primary working directory." + ].join('\n') } export class PiRpcProcess { @@ -137,6 +161,28 @@ export class PiRpcProcess { const args = ['--mode', 'rpc', '--no-themes'] if (params.sessionPath) args.push('--session', params.sessionPath) + // Workspace roots go through `--append-system-prompt`, which pi resolves from a + // file when the argument is an existing path. Using a temp file (instead of inline + // text) sidesteps shell-quoting issues for long multi-line text, which matters + // because pi.cmd on Windows must be spawned with shell mode. + let workspaceRootsDir: string | null = null + if (params.additionalDirectories && params.additionalDirectories.length > 0) { + workspaceRootsDir = mkdtempSync(join(tmpdir(), 'pi-acp-')) + const promptFile = join(workspaceRootsDir, 'workspace-roots.txt') + writeFileSync(promptFile, buildWorkspaceRootsPrompt(params.cwd, params.additionalDirectories), 'utf-8') + args.push('--append-system-prompt', shouldUseShellForPiCommand(cmd) ? `"${promptFile}"` : promptFile) + } + + const removeWorkspaceRootsDir = () => { + if (!workspaceRootsDir) return + try { + rmSync(workspaceRootsDir, { recursive: true, force: true }) + } catch { + // best effort; the file lives in the OS temp dir + } + workspaceRootsDir = null + } + const child = spawn(cmd, args, { cwd: params.cwd, stdio: 'pipe', @@ -144,6 +190,11 @@ export class PiRpcProcess { shell: shouldUseShellForPiCommand(cmd) }) + // Remove the temp file when the subprocess goes away; pi may re-read it on + // resource reloads, so it must live exactly as long as the process. + child.on('exit', removeWorkspaceRootsDir) + child.on('error', removeWorkspaceRootsDir) + // Ensure spawn failures (e.g. ENOENT when pi isn't installed) are surfaced as a // deterministic error instead of later EPIPE/internal-error noise. try { @@ -165,6 +216,7 @@ export class PiRpcProcess { child.once('error', onError) }) } catch (e: any) { + removeWorkspaceRootsDir() const code = typeof e?.code === 'string' ? e.code : undefined if (code === 'ENOENT') { throw new PiRpcSpawnError( diff --git a/test/unit/new-session-additional-directories.test.ts b/test/unit/new-session-additional-directories.test.ts new file mode 100644 index 00000000..8bda106e --- /dev/null +++ b/test/unit/new-session-additional-directories.test.ts @@ -0,0 +1,109 @@ +import test from 'node:test' +import assert from 'node:assert/strict' +import { resolve } from 'node:path' +import { PiAcpAgent } from '../../src/acp/agent.js' +import { FakeAgentSideConnection, asAgentConn } from '../helpers/fakes.js' + +class FakeSessions { + createCalls: any[] = [] + + constructor(private readonly session: any) {} + + async create(params: any) { + this.createCalls.push(params) + return this.session + } + + maybeGet(sessionId: string) { + if (sessionId !== this.session.sessionId) return undefined + return this.session + } + + get(sessionId: string) { + if (sessionId !== this.session.sessionId) { + throw new Error(`Unknown sessionId: ${sessionId}`) + } + return this.session + } +} + +function makeSession(cwd: string) { + return { + sessionId: 's1', + cwd, + setStartupInfo() {}, + sendStartupInfoIfPending() {}, + proc: { + async getAvailableModels() { + return { models: [{ provider: 'test', id: 'alpha', name: 'Alpha' }] } + }, + async getState() { + return { thinkingLevel: 'medium', model: null } + } + } + } +} + +test('PiAcpAgent: newSession forwards normalized additionalDirectories to the session', async () => { + const realSetTimeout = globalThis.setTimeout + ;(globalThis as any).setTimeout = () => 0 as any + + try { + const conn = new FakeAgentSideConnection() + const cwd = process.cwd() + const extra = resolve(cwd, 'lib') + + const session = makeSession(cwd) + const sessions = new FakeSessions(session) + const agent = new PiAcpAgent(asAgentConn(conn), {} as any) + ;(agent as any).sessions = sessions as any + + await agent.newSession({ cwd, mcpServers: [], additionalDirectories: [extra, extra, cwd] } as any) + + assert.equal(sessions.createCalls.length, 1) + assert.deepEqual(sessions.createCalls[0].additionalDirectories, [extra]) + } finally { + ;(globalThis as any).setTimeout = realSetTimeout + } +}) + +test('PiAcpAgent: newSession accepts omitted additionalDirectories', async () => { + const realSetTimeout = globalThis.setTimeout + ;(globalThis as any).setTimeout = () => 0 as any + + try { + const conn = new FakeAgentSideConnection() + const cwd = process.cwd() + + const session = makeSession(cwd) + const sessions = new FakeSessions(session) + const agent = new PiAcpAgent(asAgentConn(conn), {} as any) + ;(agent as any).sessions = sessions as any + + await agent.newSession({ cwd, mcpServers: [] } as any) + + assert.equal(sessions.createCalls.length, 1) + assert.deepEqual(sessions.createCalls[0].additionalDirectories, []) + } finally { + ;(globalThis as any).setTimeout = realSetTimeout + } +}) + +test('PiAcpAgent: newSession rejects relative additionalDirectories', async () => { + const conn = new FakeAgentSideConnection() + const agent = new PiAcpAgent(asAgentConn(conn), {} as any) + + await assert.rejects( + () => agent.newSession({ cwd: process.cwd(), mcpServers: [], additionalDirectories: ['relative/path'] } as any), + (e: any) => e?.code === -32602 + ) +}) + +test('PiAcpAgent: initialize advertises sessionCapabilities.additionalDirectories', async () => { + const conn = new FakeAgentSideConnection() + const agent = new PiAcpAgent(asAgentConn(conn), {} as any) + + const res = await agent.initialize({ protocolVersion: 1 } as any) + + assert.deepEqual((res as any).agentCapabilities.sessionCapabilities.additionalDirectories, {}) +}) diff --git a/test/unit/session-restore.test.ts b/test/unit/session-restore.test.ts index 46abe337..7d03bfb7 100644 --- a/test/unit/session-restore.test.ts +++ b/test/unit/session-restore.test.ts @@ -80,7 +80,8 @@ test('PiAcpAgent: prompt auto-restores a missing session from SessionStore', asy { cwd: '/tmp/store-project', sessionPath: '/tmp/store-project/session.jsonl', - piCommand: process.env.PI_ACP_PI_COMMAND + piCommand: process.env.PI_ACP_PI_COMMAND, + additionalDirectories: undefined } ]) assert.deepEqual(promptCalls, [{ message: 'hello again', images: [] }]) @@ -173,7 +174,8 @@ test('PiAcpAgent: setSessionConfigOption auto-restores via pi session discovery { cwd: '/tmp/fallback-project', sessionPath: sessionFile, - piCommand: process.env.PI_ACP_PI_COMMAND + piCommand: process.env.PI_ACP_PI_COMMAND, + additionalDirectories: undefined } ]) assert.deepEqual(setModelCalls, [{ provider: 'test', modelId: 'beta' }]) diff --git a/test/unit/workspace-roots.test.ts b/test/unit/workspace-roots.test.ts new file mode 100644 index 00000000..0f21e3fa --- /dev/null +++ b/test/unit/workspace-roots.test.ts @@ -0,0 +1,54 @@ +import test from 'node:test' +import assert from 'node:assert/strict' +import { resolve } from 'node:path' +import { normalizeAdditionalDirectories } from '../../src/acp/workspace-roots.js' +import { buildWorkspaceRootsPrompt } from '../../src/pi-rpc/process.js' + +const cwd = resolve('/work') + +test('normalizeAdditionalDirectories: empty input activates no additional roots', () => { + assert.deepEqual(normalizeAdditionalDirectories(undefined, cwd), []) + assert.deepEqual(normalizeAdditionalDirectories(null, cwd), []) + assert.deepEqual(normalizeAdditionalDirectories([], cwd), []) +}) + +test('normalizeAdditionalDirectories: keeps absolute paths and preserves order', () => { + const a = resolve('/work/libs/a') + const b = resolve('/work/docs/b') + + assert.deepEqual(normalizeAdditionalDirectories([a, b], cwd), [a, b]) +}) + +test('normalizeAdditionalDirectories: drops duplicates and cwd itself', () => { + const a = resolve('/work/libs/a') + + assert.deepEqual(normalizeAdditionalDirectories([a, a, cwd, resolve('/work')], cwd), [a]) +}) + +test('normalizeAdditionalDirectories: rejects non-absolute entries', () => { + assert.throws( + () => normalizeAdditionalDirectories(['relative/path'], cwd), + (e: any) => e?.code === -32602 + ) +}) + +test('normalizeAdditionalDirectories: rejects non-string entries', () => { + assert.throws( + () => normalizeAdditionalDirectories([42] as any, cwd), + (e: any) => e?.code === -32602 + ) +}) + +test('buildWorkspaceRootsPrompt: lists cwd and additional roots with guidance', () => { + const a = resolve('/work/libs/a') + const b = resolve('/work/docs/b') + + const prompt = buildWorkspaceRootsPrompt(cwd, [a, b]) + + assert.ok(prompt.includes('')) + assert.ok(prompt.includes(`Primary working directory: ${cwd}`)) + assert.ok(prompt.includes(`- ${a}`)) + assert.ok(prompt.includes(`- ${b}`)) + assert.ok(prompt.includes('')) + assert.ok(prompt.includes('Relative paths still resolve against the primary working directory.')) +})