diff --git a/.gitignore b/.gitignore index c5961eeddd..93276ae80f 100644 --- a/.gitignore +++ b/.gitignore @@ -103,3 +103,6 @@ evals/logs/ # Git worktrees (ad-hoc local checkouts; keep out of git and test discovery) .worktrees/ + +# ACP test logs (from scripts/test-acp-zed-bugs.mjs) +.acp-test-logs/ diff --git a/packages/agents/src/core/agenticLoop/AgenticLoop.ts b/packages/agents/src/core/agenticLoop/AgenticLoop.ts index 7bacc67d6d..4836ca5882 100644 --- a/packages/agents/src/core/agenticLoop/AgenticLoop.ts +++ b/packages/agents/src/core/agenticLoop/AgenticLoop.ts @@ -51,6 +51,7 @@ import { buildToolResponses, classifyCompletedTools, recordCancelledToolHistory, + recordCompletedToolHistory, } from './loopHelpers.js'; const logger = new DebugLogger('llxprt:agents:agentic-loop'); @@ -639,6 +640,17 @@ export class AgenticLoop { await recordCancelledToolHistory(agentTools, this.agentClient); return { continueLoop: false, nextMessage: [] }; } + // A successful pause request (the pause tool) is a terminal signal: the + // loop must stop so the agent returns control to the user. Without this, + // the loop feeds the pause response back to the model and continues + // indefinitely (issue #2653). In the CLI this is masked by the React UI + // continuation gate, but ACP/Zed and other headless consumers have none. + if (hasSuccessfulTodoPause(agentTools)) { + // Eagerly persist tool history since there will be no next model turn + // to carry the response parts naturally. + await recordCompletedToolHistory(agentTools, this.agentClient); + return { continueLoop: false, nextMessage: [] }; + } const responseParts = buildToolResponses(agentTools); if (responseParts.length === 0) { return { continueLoop: false, nextMessage: [] }; @@ -655,3 +667,18 @@ function* flushBuffered(queue: EventQueue): Generator { event = queue.popBuffered(); } } + +/** + * Returns true if any of the completed tool calls is a successful pause + * (case-insensitive, no error). A failed pause must NOT terminate the loop, + * matching TodoContinuationService.isSuccessfulTodoPauseResponse semantics. + */ +function hasSuccessfulTodoPause(tools: CompletedToolCall[]): boolean { + return tools.some( + (tc) => + tc.request.name.toLowerCase() === 'todo_pause' && + tc.status === 'success' && + tc.response.error === undefined && + tc.response.errorType === undefined, + ); +} diff --git a/packages/agents/src/core/agenticLoop/__tests__/agenticLoop.todoPause.test.ts b/packages/agents/src/core/agenticLoop/__tests__/agenticLoop.todoPause.test.ts new file mode 100644 index 0000000000..0cd197e537 --- /dev/null +++ b/packages/agents/src/core/agenticLoop/__tests__/agenticLoop.todoPause.test.ts @@ -0,0 +1,243 @@ +/** + * @license + * Copyright 2025 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Behavioral tests for AgenticLoop pause-loop-break (issue #2653). + * + * These tests verify that when the model calls the pause tool and the tool + * completes successfully, the AgenticLoop stops — it does NOT feed the + * pause response back into another model turn. This is critical for + * ACP/Zed and other headless consumers that lack the CLI's React UI + * continuation gate. + * + * The test approach follows dev-docs/RULES.md: real AgenticLoop, real + * CoreToolScheduler, real MessageBus. The only mock boundary is the + * provider stream (scripted ServerAgentStreamEvents). + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { AgenticLoop } from '../AgenticLoop.js'; +import { MockTool } from '@vybestack/llxprt-code-core/test-utils/mock-tool.js'; +import { clearAllSchedulers } from '@vybestack/llxprt-code-core/config/schedulerSingleton.js'; +import { MessageBus } from '@vybestack/llxprt-code-core/confirmation-bus/message-bus.js'; +import { ApprovalMode } from '@vybestack/llxprt-code-core/config/configTypes.js'; +import { + createScriptedAgentClient, + createTestConfig, + createToolRegistryForTest, + createAskPolicyEngine, + collectEvents, + isToolsComplete, + toolCallRequestEvent, + finishedEvent, +} from './agenticLoop-test-helpers.js'; + +describe('AgenticLoop pause loop-break (issue #2653)', () => { + beforeEach(() => { + clearAllSchedulers(); + }); + afterEach(() => { + clearAllSchedulers(); + }); + + it('stops the loop after a successful pause tool call (no extra model turn)', async () => { + const pauseTool = new MockTool({ + name: 'todo_pause', + execute: async () => ({ + llmContent: 'paused', + returnDisplay: 'paused', + }), + }); + + const toolRegistry = createToolRegistryForTest([pauseTool]); + const messageBus = new MessageBus(createAskPolicyEngine(), false); + const config = createTestConfig({ + messageBus, + toolRegistry, + policyEngine: createAskPolicyEngine(), + interactive: true, + approvalMode: ApprovalMode.YOLO, + }); + + // Turn 1: model requests the pause tool, then finishes the turn. + // No Turn 2 script is provided — if the loop tries to continue, the + // scripted client will return an empty stream. + const { client, turnMessages } = createScriptedAgentClient([ + [ + toolCallRequestEvent('todo_pause', 'pause-1', { + reason: 'testing pause', + }), + finishedEvent(), + ], + ]); + + const loop = new AgenticLoop({ + agentClient: client, + config, + messageBus, + interactiveMode: true, + }); + + const events = await collectEvents( + loop, + [{ type: 'text', text: 'pause please' }], + new AbortController().signal, + ); + + expect(events.some(isToolsComplete)).toBe(true); + // CRITICAL: only ONE model turn should have happened. If the loop + // continued after the pause, turnMessages would have 2 entries. + expect(turnMessages).toHaveLength(1); + + const errorEvents = events.filter((e) => e.kind === 'error'); + expect(errorEvents).toHaveLength(0); + }); + + it('eagerly records pause tool history so the response is durable', async () => { + const pauseTool = new MockTool({ + name: 'todo_pause', + execute: async () => ({ + llmContent: 'paused with reason', + returnDisplay: 'paused', + }), + }); + + const toolRegistry = createToolRegistryForTest([pauseTool]); + const messageBus = new MessageBus(createAskPolicyEngine(), false); + const config = createTestConfig({ + messageBus, + toolRegistry, + policyEngine: createAskPolicyEngine(), + interactive: true, + approvalMode: ApprovalMode.YOLO, + }); + + const { client, history } = createScriptedAgentClient([ + [ + toolCallRequestEvent('todo_pause', 'pause-2', { + reason: 'need to stop', + }), + finishedEvent(), + ], + ]); + + const loop = new AgenticLoop({ + agentClient: client, + config, + messageBus, + interactiveMode: true, + }); + + await collectEvents( + loop, + [{ type: 'text', text: 'pause' }], + new AbortController().signal, + ); + + // History must contain the tool response blocks, even though there was + // no next model turn to carry them. This proves the eager history + // recording in buildNextMessage works. + const allBlocks = history.flatMap((h) => h.blocks); + const hasToolResponse = allBlocks.some((b) => b.type === 'tool_response'); + expect(hasToolResponse).toBe(true); + }); + + it('does NOT stop the loop when the pause tool fails (error response)', async () => { + const pauseTool = new MockTool({ + name: 'todo_pause', + execute: async () => { + throw new Error('pause failed'); + }, + }); + + const echoTool = new MockTool({ + name: 'echo', + execute: async () => ({ + llmContent: 'echoed', + returnDisplay: 'echoed', + }), + }); + + const toolRegistry = createToolRegistryForTest([pauseTool, echoTool]); + const messageBus = new MessageBus(createAskPolicyEngine(), false); + const config = createTestConfig({ + messageBus, + toolRegistry, + policyEngine: createAskPolicyEngine(), + interactive: true, + approvalMode: ApprovalMode.YOLO, + }); + + const { client, turnMessages } = createScriptedAgentClient([ + [ + toolCallRequestEvent('todo_pause', 'pause-err', { + reason: 'will fail', + }), + finishedEvent(), + ], + [toolCallRequestEvent('echo', 'echo-1', {}), finishedEvent()], + ]); + + const loop = new AgenticLoop({ + agentClient: client, + config, + messageBus, + interactiveMode: true, + }); + + await collectEvents( + loop, + [{ type: 'text', text: 'try pause then echo' }], + new AbortController().signal, + ); + + // The failed pause must NOT have terminated the loop — the model should + // have received the error and continued with a second turn. + expect(turnMessages.length).toBeGreaterThanOrEqual(2); + }); + + it('continues the loop normally for non-pause tools after the fix', async () => { + // Regression guard: a normal successful tool should still cause the + // loop to continue (feed response, next model turn). + const normalTool = new MockTool({ + name: 'get_info', + execute: async () => ({ + llmContent: 'info result', + returnDisplay: 'info result', + }), + }); + + const toolRegistry = createToolRegistryForTest([normalTool]); + const messageBus = new MessageBus(createAskPolicyEngine(), false); + const config = createTestConfig({ + messageBus, + toolRegistry, + policyEngine: createAskPolicyEngine(), + interactive: true, + approvalMode: ApprovalMode.YOLO, + }); + + const { client, turnMessages } = createScriptedAgentClient([ + [toolCallRequestEvent('get_info', 'info-1', {}), finishedEvent()], + [finishedEvent()], + ]); + + const loop = new AgenticLoop({ + agentClient: client, + config, + messageBus, + interactiveMode: true, + }); + + await collectEvents( + loop, + [{ type: 'text', text: 'get info' }], + new AbortController().signal, + ); + + expect(turnMessages).toHaveLength(2); + }); +}); diff --git a/packages/agents/src/core/agenticLoop/loopHelpers.ts b/packages/agents/src/core/agenticLoop/loopHelpers.ts index 9915456232..9927624726 100644 --- a/packages/agents/src/core/agenticLoop/loopHelpers.ts +++ b/packages/agents/src/core/agenticLoop/loopHelpers.ts @@ -96,7 +96,7 @@ export function buildToolResponses( * Awaits both writes so callers that continue or exit the loop immediately * afterwards can guarantee the tool history is durable before the next turn. */ -async function recordCompletedToolHistory( +export async function recordCompletedToolHistory( tools: CompletedToolCall[], agentClient: AgentClientContract, ): Promise { diff --git a/packages/agents/src/core/subagentExecution.scheduler-receiver.test.ts b/packages/agents/src/core/subagentExecution.scheduler-receiver.test.ts new file mode 100644 index 0000000000..37e3cac706 --- /dev/null +++ b/packages/agents/src/core/subagentExecution.scheduler-receiver.test.ts @@ -0,0 +1,131 @@ +/** + * @license + * Copyright 2025 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Behavioral test for the scheduler receiver-preservation fix (issue #2653). + * + * When no schedulerFactory is provided (the ACP/Zed fallback path), + * initInteractiveScheduler must preserve the scheduler's `this` context. + * Previously it copied `schedule: scheduler.schedule` directly, losing the + * receiver and causing `this.isRunning is not a function` when + * CoreToolScheduler.schedule() called this.isRunning(). + * + * This test creates a receiver-sensitive scheduler and verifies that calling + * the facade's `schedule()` invokes the original scheduler's method with the + * correct `this`. + */ + +import { describe, it, expect, vi } from 'vitest'; +// vi is used for mocking ctx components below +import { + initInteractiveScheduler, + type InitSchedulerContext, +} from './subagentExecution.js'; +import { DebugLogger } from '@vybestack/llxprt-code-core/debug/DebugLogger.js'; +import type { MessageBus } from '@vybestack/llxprt-code-core/confirmation-bus/message-bus.js'; + +/** + * A scheduler whose schedule() method depends on `this` — it calls + * this.isRunning() (just like CoreToolScheduler). If the receiver is lost, + * this throws "this.isRunning is not a function". + * + * IMPORTANT: schedule() MUST be a regular prototype method, NOT an arrow + * function property. Arrow functions lexically bind `this` to the instance, + * which would make receiver-loss impossible to detect — the test would pass + * even with the bug present. + */ +class ReceiverSensitiveScheduler { + private running = false; + scheduleCallCount = 0; + + private isRunning(): boolean { + return this.running; + } + + async schedule(_req: unknown, _signal: unknown): Promise { + this.scheduleCallCount++; + // This call will crash if `this` is wrong (e.g. copied without binding). + if (this.isRunning()) { + throw new Error('should not be running'); + } + } + + async dispose(): Promise {} +} + +function makeCtx( + overrides?: Partial, +): InitSchedulerContext { + return { + schedulerConfig: { + getSessionId: () => 'test-session', + disposeScheduler: vi.fn(), + } as unknown as InitSchedulerContext['schedulerConfig'], + messageBus: { + subscribe: vi.fn(() => () => {}), + respondToConfirmation: vi.fn(), + } as unknown as MessageBus, + subagentId: 'test-subagent', + logger: new DebugLogger('test'), + ...overrides, + }; +} + +describe('initInteractiveScheduler — scheduler receiver preservation (issue #2653)', () => { + it('preserves the scheduler receiver when no schedulerFactory is provided (ACP fallback)', async () => { + const realScheduler = new ReceiverSensitiveScheduler(); + + // Override getOrCreateScheduler to return our receiver-sensitive scheduler. + const ctx = makeCtx({ + schedulerConfig: { + getSessionId: () => 'test-session', + disposeScheduler: vi.fn(), + getOrCreateScheduler: vi.fn(async () => realScheduler), + } as unknown as InitSchedulerContext['schedulerConfig'], + }); + + // No schedulerFactory → exercises the fallback path. + const result = await initInteractiveScheduler(undefined, ctx); + + // Call schedule through the facade. Before the fix, this crashed with + // "this.isRunning is not a function" because the facade copied + // `schedule: scheduler.schedule` without binding, so `this` inside + // schedule() was the facade (which has no isRunning method). + const signal = new AbortController().signal; + await expect( + result.scheduler.schedule([], signal), + ).resolves.toBeUndefined(); + + // Verify the original scheduler's schedule was actually called. + expect(realScheduler.scheduleCallCount).toBe(1); + }); + + it('preserves the scheduler receiver when schedulerFactory IS provided', async () => { + const realScheduler = new ReceiverSensitiveScheduler(); + + const ctx = makeCtx(); + + const result = await initInteractiveScheduler( + { + schedulerFactory: async () => ({ + schedule: async (req: unknown, sig: unknown) => + realScheduler.schedule(req, sig), + dispose: () => { + void realScheduler.dispose(); + }, + }), + }, + ctx, + ); + + const signal = new AbortController().signal; + await expect( + result.scheduler.schedule([], signal), + ).resolves.toBeUndefined(); + + expect(realScheduler.scheduleCallCount).toBe(1); + }); +}); diff --git a/packages/agents/src/core/subagentExecution.ts b/packages/agents/src/core/subagentExecution.ts index 94aca0564c..093740edea 100644 --- a/packages/agents/src/core/subagentExecution.ts +++ b/packages/agents/src/core/subagentExecution.ts @@ -530,7 +530,15 @@ export async function initInteractiveScheduler( return { scheduler: { - schedule: scheduler.schedule, + // Preserve the scheduler receiver: copying scheduler.schedule directly + // loses `this`, causing CoreToolScheduler.schedule() to crash with + // "this.isRunning is not a function" (issue #2653). A closure keeps + // the original instance as the receiver, matching the pattern in + // interactiveToolScheduler.ts. + schedule: ( + request: Parameters[0], + signal: Parameters[1], + ) => scheduler.schedule(request, signal), awaitCompletedCalls: channel.awaitCompletedCalls, }, schedulerDispose, diff --git a/scripts/acp-logging-proxy.mjs b/scripts/acp-logging-proxy.mjs new file mode 100644 index 0000000000..0bf0b36428 --- /dev/null +++ b/scripts/acp-logging-proxy.mjs @@ -0,0 +1,148 @@ +#!/usr/bin/env node +/** + * ACP logging proxy: sits between Zed and the real llxprt ACP agent. + * + * Zed spawns THIS script as its agent server. This script then spawns the + * real llxprt ACP agent as a child process, forwarding all stdin/stdout + * bidirectionally while logging every ACP message to a log file. + * + * This lets us observe the full ACP protocol exchange when Zed interacts + * with the agent, including any errors from the `task` tool or the pause tool. + * + * Usage: Set this as the Zed agent server command instead of bun directly. + * The real agent binary and args are passed after a -- separator. + * + * Example Zed config: + * "command": "node", + * "args": ["scripts/acp-logging-proxy.mjs", "--", "bun", "packages/cli/index.ts", "--experimental-acp", ...] + */ + +import { spawn } from 'node:child_process'; +import { createWriteStream, mkdirSync, existsSync } from 'node:fs'; +import { join, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +// Parse args after "--" to find the real agent command +const sepIndex = process.argv.indexOf('--'); +const agentArgs = sepIndex >= 0 ? process.argv.slice(sepIndex + 1) : []; +const agentCmd = agentArgs[0] || 'bun'; +const agentCmdArgs = agentArgs.slice(1); + +if (agentArgs.length === 0) { + process.stderr.write('acp-logging-proxy: no agent command after "--"\n'); + process.exit(1); +} + +// Log directory +const logDir = + process.env.ACP_PROXY_LOG_DIR || join(__dirname, '..', '.acp-proxy-logs'); +if (!existsSync(logDir)) { + mkdirSync(logDir, { recursive: true }); +} + +const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); +const logFile = join(logDir, `acp-${timestamp}.jsonl`); +const logStream = createWriteStream(logFile, { flags: 'a' }); + +function logEntry(direction, data) { + const entry = { + ts: new Date().toISOString(), + direction, // 'zed->agent' or 'agent->zed' + raw: typeof data === 'string' ? data : data.toString(), + }; + // Try to parse as JSON for prettier logging + try { + entry.parsed = JSON.parse(entry.raw); + delete entry.raw; + } catch { + // Keep raw if not JSON + } + logStream.write(JSON.stringify(entry) + '\n'); +} + +logStream.write( + JSON.stringify({ + ts: new Date().toISOString(), + event: 'proxy-start', + agentCmd, + agentCmdArgs, + pid: process.pid, + }) + '\n', +); + +// Spawn the real agent +const child = spawn(agentCmd, agentCmdArgs, { + stdio: ['pipe', 'pipe', 'pipe'], + env: { ...process.env }, +}); + +// zed -> agent (stdin forwarding with logging) +process.stdin.on('data', (chunk) => { + logEntry('zed->agent', chunk); + child.stdin.write(chunk); +}); + +process.stdin.on('end', () => { + child.stdin.end(); +}); + +// agent -> zed (stdout forwarding with logging) +child.stdout.on('data', (chunk) => { + logEntry('agent->zed', chunk); + process.stdout.write(chunk); +}); + +// agent stderr -> our stderr (for debug logs) +child.stderr.on('data', (chunk) => { + const text = chunk.toString(); + // Log errors specially + if ( + text.includes('isRunning') || + text.includes('not a function') || + text.includes('ERROR') + ) { + logStream.write( + JSON.stringify({ + ts: new Date().toISOString(), + event: 'STDERR_ERROR', + text: text.trim(), + }) + '\n', + ); + } + process.stderr.write(chunk); +}); + +// Handle process lifecycle +child.on('error', (err) => { + logStream.write( + JSON.stringify({ + ts: new Date().toISOString(), + event: 'child-error', + error: err.message, + }) + '\n', + ); + process.exit(1); +}); + +child.on('exit', (code, signal) => { + logStream.write( + JSON.stringify({ + ts: new Date().toISOString(), + event: 'child-exit', + code, + signal, + }) + '\n', + ); + logStream.end(); + process.exit(code ?? 1); +}); + +process.on('SIGTERM', () => { + child.kill('SIGTERM'); +}); +process.on('SIGINT', () => { + child.kill('SIGINT'); +}); diff --git a/scripts/setup-zed-agent.mjs b/scripts/setup-zed-agent.mjs new file mode 100644 index 0000000000..ecd8b5cf77 --- /dev/null +++ b/scripts/setup-zed-agent.mjs @@ -0,0 +1,313 @@ +#!/usr/bin/env node +/** + * Cross-platform Zed agent configuration script. + * + * Configures the Zed editor's agent_servers setting to point to a specific + * llxprt-code build (e.g., a local development branch). This is useful for + * testing fixes in Zed without needing to reinstall the global npm package. + * + * Usage: + * node scripts/setup-zed-agent.mjs [--profile ] [--entry ] [--binary ] + * + * Without arguments, it auto-detects: + * - Binary: local node_modules bun (or global bun on PATH) + * - Entry: packages/cli/index.ts relative to project root + * - Profile: gpt56high (override with --profile) + * + * Works on Windows, macOS, and Linux. + */ + +import { readFileSync, writeFileSync, existsSync, mkdirSync } from 'node:fs'; +import { join, dirname, resolve } from 'node:path'; +import { homedir, platform } from 'node:os'; +import { fileURLToPath } from 'node:url'; + +const __filename = fileURLToPath(import.meta.url); +// --------------------------------------------------------------------------- +// JSONC comment stripping (string-aware state machine) +// +// A naive regex like /\/\/.*$/gm corrupts Windows paths (e.g., "C:\\Users" +// becomes "C:"). This state machine tracks whether we're inside a string +// literal and only strips comments outside strings. +// --------------------------------------------------------------------------- + +/** + * Strip // line comments and block comments from JSONC, respecting + * string literals. Handles escaped quotes inside strings. + */ +function stripJsoncComments(text) { + let result = ''; + let i = 0; + let inString = false; + + while (i < text.length) { + const ch = text[i]; + const next = text[i + 1]; + + if (inString) { + const { advanced, stillInString } = consumeStringChar(ch, next); + result += advanced.char; + if (advanced.char !== '') result += advanced.extra || ''; + inString = stillInString; + i += advanced.step; + } else if (ch === '"') { + inString = true; + result += ch; + i++; + } else if (ch === '/' && next === '/') { + i = skipLineComment(text, i); + } else if (ch === '/' && next === '*') { + i = skipBlockComment(text, i); + } else { + result += ch; + i++; + } + } + + return result; +} + +function skipLineComment(text, i) { + i += 2; + while (i < text.length && text[i] !== '\n') { + i++; + } + return i; +} + +function skipBlockComment(text, i) { + i += 2; + while (i < text.length && !(text[i] === '*' && text[i + 1] === '/')) { + i++; + } + return i + 2; +} + +function consumeStringChar(ch, next) { + if (ch === '\\' && next !== undefined) { + return { + advanced: { char: ch, extra: next, step: 2 }, + stillInString: true, + }; + } + return { + advanced: { char: ch, extra: '', step: 1 }, + stillInString: ch !== '"', + }; +} + +const __dirname = dirname(__filename); +const projectRoot = resolve(__dirname, '..'); + +// --------------------------------------------------------------------------- +// Zed settings path resolution (cross-platform) +// --------------------------------------------------------------------------- + +function getZedSettingsPath() { + const home = homedir(); + const isWin = platform() === 'win32'; + const isMac = platform() === 'darwin'; + + if (isMac) { + return join(home, 'Library', 'Application Support', 'Zed', 'settings.json'); + } + + if (isWin) { + return join(home, 'AppData', 'Roaming', 'Zed', 'settings.json'); + } + + // Linux and others — follow XDG + const xdgConfig = process.env.XDG_CONFIG_HOME || join(home, '.config'); + return join(xdgConfig, 'zed', 'settings.json'); +} + +// --------------------------------------------------------------------------- +// Binary resolution +// --------------------------------------------------------------------------- + +function findBunBinary() { + const isWin = platform() === 'win32'; + const exe = isWin ? 'bun.exe' : 'bun'; + + // Local node_modules bun + const localBun = join(projectRoot, 'node_modules', 'bun', 'bin', exe); + if (existsSync(localBun)) { + return localBun; + } + + // Fallback + return 'bun'; +} + +function findEntryPoint() { + return join(projectRoot, 'packages', 'cli', 'index.ts'); +} + +// --------------------------------------------------------------------------- +// Argument parsing +// --------------------------------------------------------------------------- + +function parseArgs() { + const args = process.argv.slice(2); + const opts = { + profile: 'gpt56high', + entry: null, + binary: null, + agentName: 'llxprt', + yolo: true, + debug: true, + }; + + for (let i = 0; i < args.length; i++) { + switch (args[i]) { + case '--profile': + opts.profile = args[++i]; + break; + case '--entry': + opts.entry = args[++i]; + break; + case '--binary': + opts.binary = args[++i]; + break; + case '--agent-name': + opts.agentName = args[++i]; + break; + case '--no-yolo': + opts.yolo = false; + break; + case '--no-debug': + opts.debug = false; + break; + case '--help': + case '-h': + printHelp(); + process.exit(0); + break; + } + } + + opts.binary = opts.binary || findBunBinary(); + opts.entry = opts.entry || findEntryPoint(); + + return opts; +} + +function printHelp() { + console.log(` +Usage: node scripts/setup-zed-agent.mjs [options] + +Configures Zed's agent_servers to use a local llxprt-code build. + +Options: + --profile Profile to load (default: gpt56high) + --entry Override the index.ts entry point + --binary Override the bun binary path + --agent-name Agent server name in Zed (default: llxprt) + --no-yolo Disable auto-approve (--yolo flag) + --no-debug Disable debug logging + -h, --help Show this help + +The script reads the existing Zed settings.json, adds or updates the +agent_servers entry, and writes it back. It preserves all other settings. +`); +} + +// --------------------------------------------------------------------------- +// Main +// --------------------------------------------------------------------------- + +function main() { + const opts = parseArgs(); + const settingsPath = getZedSettingsPath(); + + console.log(`Zed settings path: ${settingsPath}`); + console.log(`Binary: ${opts.binary}`); + console.log(`Entry: ${opts.entry}`); + console.log(`Profile: ${opts.profile}`); + console.log(`Agent name: ${opts.agentName}`); + + // Read existing settings or start fresh + let settings = {}; + if (existsSync(settingsPath)) { + try { + const raw = readFileSync(settingsPath, 'utf-8'); + settings = JSON.parse(stripJsoncComments(raw)); + } catch (e) { + console.error( + `Error: could not parse existing settings.json: ${e.message}`, + ); + // Save a backup so the user can recover + try { + writeFileSync( + `${settingsPath}.bak`, + readFileSync(settingsPath, 'utf-8'), + ); + console.error(`A backup was saved to ${settingsPath}.bak`); + } catch { + // If backup also fails, nothing more we can do + } + console.error('Please fix the file manually and re-run this script.'); + process.exit(1); + } + } else { + // Ensure directory exists + const dir = dirname(settingsPath); + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true }); + } + } + + // Build the agent server config + const args = [ + opts.entry, + '--experimental-acp', + '--profile-load', + opts.profile, + ]; + if (opts.yolo) { + args.push('--yolo'); + } + + const env = {}; + if (opts.debug) { + env.LLXPRT_DEBUG = 'llxprt:*'; + } + + if (!settings.agent_servers) { + settings.agent_servers = {}; + } + + settings.agent_servers[opts.agentName] = { + type: 'custom', + command: opts.binary, + args, + }; + + if (Object.keys(env).length > 0) { + settings.agent_servers[opts.agentName].env = env; + } + + // Backup before write (in case of disk error mid-write) + if (existsSync(settingsPath)) { + try { + writeFileSync(`${settingsPath}.bak`, readFileSync(settingsPath, 'utf-8')); + } catch { + // Non-fatal: best-effort backup + } + } + + // Write back + writeFileSync( + settingsPath, + JSON.stringify(settings, null, 2) + '\n', + 'utf-8', + ); + + console.log(`\n✅ Updated Zed settings: ${settingsPath}`); + console.log(` Agent "${opts.agentName}" now points to your local build.`); + console.log( + `\n📌 Restart Zed (or reconnect the agent) for changes to take effect.`, + ); +} + +main(); diff --git a/scripts/test-acp-zed-bugs.mjs b/scripts/test-acp-zed-bugs.mjs new file mode 100644 index 0000000000..2fb6dae3e5 --- /dev/null +++ b/scripts/test-acp-zed-bugs.mjs @@ -0,0 +1,533 @@ +#!/usr/bin/env node +/** + * Cross-platform ACP integration test for Zed/ACP-mode bugs. + * + * Reproduces two issues that only manifest in ACP/Zed mode: + * + * Bug 1: `this.isRunning is not a function` when the `task` tool is invoked. + * Bug 2: The pause tool does not stop the agent continuation loop. + */ + +import { spawn } from 'node:child_process'; +import { platform } from 'node:os'; +import { resolve, join } from 'node:path'; +import { existsSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { dirname } from 'node:path'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); +const projectRoot = resolve(__dirname, '..'); + +// --------------------------------------------------------------------------- +// Argument parsing +// --------------------------------------------------------------------------- + +function parseArgs() { + const args = process.argv.slice(2); + const opts = { + profile: process.env.LLXPRT_PROFILE || 'gpt56high', + prompt: null, + timeout: 120_000, + }; + for (let i = 0; i < args.length; i++) { + switch (args[i]) { + case '--profile': + opts.profile = args[++i]; + break; + case '--prompt': + opts.prompt = args[++i]; + break; + case '--timeout': + opts.timeout = Number(args[++i]); + break; + case '--help': + case '-h': + printHelp(); + process.exit(0); + break; + } + } + if (!opts.prompt) { + opts.prompt = + 'Use the todo_write tool to create a single todo item that says "test". ' + + 'Then immediately call todo_pause with reason "testing pause in ACP mode". ' + + 'Do not do anything else.'; + } + return opts; +} + +function printHelp() { + console.log(` +Usage: node scripts/test-acp-zed-bugs.mjs [options] + +Options: + --profile Profile to load (default: gpt56high, env: LLXPRT_PROFILE) + --prompt Custom prompt to send (default: triggers todo_pause) + --timeout Timeout in milliseconds (default: 120000) + -h, --help Show this help + +Environment: + LLXPRT_BINARY Override bun binary path + LLXPRT_ENTRY Override index.ts entry point + DEBUG Set to 'llxprt:*' for verbose logs +`); +} + +// --------------------------------------------------------------------------- +// Binary resolution (cross-platform) +// --------------------------------------------------------------------------- + +function findBunBinary() { + if (process.env.LLXPRT_BINARY) { + return process.env.LLXPRT_BINARY; + } + + const cwd = process.cwd(); + const isWin = platform() === 'win32'; + const exe = isWin ? 'bun.exe' : 'bun'; + + // 1. Local node_modules bun (project devDependency) + const localBun = join(cwd, 'node_modules', 'bun', 'bin', exe); + if (existsSync(localBun)) { + return localBun; + } + + // 2. Global npm install path (common on Windows) + if (isWin) { + const globalBun = join( + process.env.APPDATA || '', + 'npm', + 'node_modules', + '@vybestack', + 'llxprt-code', + 'node_modules', + 'bun', + 'bin', + exe, + ); + if (existsSync(globalBun)) { + return globalBun; + } + } + + // 3. Fallback: just "bun" on PATH + return 'bun'; +} + +function findEntryPoint() { + if (process.env.LLXPRT_ENTRY) { + return process.env.LLXPRT_ENTRY; + } + // Local dev entry point + return join(projectRoot, 'packages', 'cli', 'index.ts'); +} + +// --------------------------------------------------------------------------- +// ACP JSON-RPC client +// --------------------------------------------------------------------------- + +class AcpClient { + constructor(process) { + this.proc = process; + this.buffer = ''; + this.nextId = 1; + this.pending = new Map(); // id -> {resolve, reject} + this.sessionUpdates = []; // all sessionUpdate notifications + this.toolCalls = []; + this.agentMessages = []; + this.errors = []; + + this.proc.stdout.on('data', (chunk) => this._onData(chunk)); + } + + _onData(chunk) { + this.buffer += chunk.toString(); + const lines = this.buffer.split('\n'); + this.buffer = lines.pop() || ''; + + for (const line of lines) { + if (!line.trim()) continue; + try { + const msg = JSON.parse(line); + this._handleMessage(msg); + } catch { + // Non-JSON line (debug logs etc.) — ignore + } + } + } + + _handleMessage(msg) { + // Response to a request + if (msg.id !== undefined && this.pending.has(msg.id)) { + const { resolve, reject } = this.pending.get(msg.id); + this.pending.delete(msg.id); + if (msg.error) { + reject(new Error(msg.error.message || JSON.stringify(msg.error))); + } else { + resolve(msg.result); + } + return; + } + + // Notification (no id) — session/update + if (msg.method === 'session/update') { + const update = msg.params?.update; + if (update) { + this.sessionUpdates.push(update); + this._categorizeUpdate(update); + } + } + } + + _categorizeUpdate(update) { + switch (update.sessionUpdate) { + case 'agent_message_chunk': + if (update.content?.text) { + this.agentMessages.push(update.content.text); + } + break; + case 'tool_call': + this.toolCalls.push({ + phase: 'start', + name: update.toolCallId, + rawTitle: update.title, + rawKind: update.kind, + content: update.content, + status: update.status, + }); + break; + case 'tool_call_update': + this.toolCalls.push({ + phase: 'update', + rawStatus: update.status, + content: update.content, + }); + break; + } + } + + async send(method, params) { + const id = this.nextId++; + const request = { jsonrpc: '2.0', id, method, params }; + + return new Promise((resolve, reject) => { + this.pending.set(id, { resolve, reject }); + this.proc.stdin.write(JSON.stringify(request) + '\n'); + + // Per-request timeout + setTimeout(() => { + if (this.pending.has(id)) { + this.pending.delete(id); + reject(new Error(`Request ${method} (id=${id}) timed out`)); + } + }, 30_000); + }); + } + + waitForCompletion(totalTimeout) { + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + reject(new Error(`Prompt did not complete within ${totalTimeout}ms`)); + }, totalTimeout); + + this.proc.on('exit', (code, signal) => { + clearTimeout(timer); + resolve({ exited: true, code, signal }); + }); + }); + } + + /** Drain stderr for error detection. */ + attachStderr() { + this.proc.stderr.on('data', (chunk) => { + const text = chunk.toString(); + if ( + text.includes('isRunning') || + text.includes('not a function') || + text.includes('ERROR') + ) { + this.errors.push(text.trim()); + } + }); + } +} + +// --------------------------------------------------------------------------- +// Test scenarios +// --------------------------------------------------------------------------- + +/** + * Performs ACP handshake: initialize → authenticate → session/new. + * Returns the session ID or null on failure. + */ +async function performAcpHandshake(client, opts, results) { + // Step 1: Initialize + console.log('\n[1/4] Sending initialize...'); + const initResult = await client.send('initialize', { + protocolVersion: 1, + clientCapabilities: { + fs: { readTextFile: false, writeTextFile: false }, + }, + }); + console.log(' [OK] Initialize succeeded'); + results.details.protocolVersion = initResult.protocolVersion; + + // Step 2: Authenticate (load profile) + console.log('\n[2/4] Sending authenticate...'); + try { + await client.send('session/authenticate', { methodId: opts.profile }); + console.log(' [OK] Authenticated'); + } catch (e) { + console.log(` Auth step skipped/failed: ${e.message}`); + } + + // Step 3: Create session + console.log('\n[3/4] Sending session/new...'); + const sessionResult = await client.send('session/new', { + cwd: projectRoot, + mcpServers: [], + }); + console.log(` [OK] Session created: ${sessionResult.sessionId}`); + results.details.sessionId = sessionResult.sessionId; + + return sessionResult.sessionId; +} + +function collectAcpMetrics(client, results) { + results.details.toolCallCount = client.toolCalls.length; + results.details.messageCount = client.agentMessages.length; + results.details.stderrErrors = client.errors.length; + results.details.stderrErrorSnippets = client.errors.slice(0, 3); +} + +function evaluateExpectations(expectations, client, results) { + for (const expectation of expectations) { + const check = expectation.check(client, results.details); + results.details[expectation.name] = check; + if (!check.passed) { + results.errors.push(check.message); + } + console.log( + ` ${check.passed ? 'PASS' : 'FAIL'} ${expectation.name}: ${check.message}`, + ); + } +} + +async function runScenario(opts, label, prompt, expectations) { + const binary = findBunBinary(); + const entry = findEntryPoint(); + + const args = [ + entry, + '--experimental-acp', + '--profile-load', + opts.profile, + '--yolo', + ]; + const env = { ...process.env }; + if (!env.DEBUG) { + env.DEBUG = 'llxprt:*'; + } + env.LLXPRT_LOG_HOME = + env.LLXPRT_LOG_HOME || join(projectRoot, '.acp-test-logs'); + + console.log(`\n${'='.repeat(70)}`); + console.log(`Scenario: ${label}`); + console.log(`${'='.repeat(70)}`); + console.log(`Binary: ${binary}`); + console.log(`Entry: ${entry}`); + console.log(`Profile: ${opts.profile}`); + console.log(`Prompt: ${prompt.substring(0, 100)}...`); + + const child = spawn(binary, args, { + stdio: ['pipe', 'pipe', 'pipe'], + cwd: projectRoot, + env, + }); + + const client = new AcpClient(child); + client.attachStderr(); + + const results = { + label, + passed: false, + errors: [], + details: {}, + }; + + try { + // Wait for process to be ready + await sleep(1000); + + const sessionId = await performAcpHandshake(client, opts, results); + if (!sessionId) return results; + + // Step 4: Prompt + console.log('\n[4/4] Sending session/prompt...'); + const promptPromise = client.send('session/prompt', { + sessionId, + prompt: [{ type: 'text', text: prompt }], + }); + + // Wait for prompt to complete (or timeout) + // Attach a no-op catch to prevent unhandled rejection if the race + // resolves via the timeout/process-exit path first. + promptPromise.catch(() => {}); + + const promptResult = await Promise.race([ + promptPromise, + client.waitForCompletion(opts.timeout), + ]); + + console.log( + ` ✓ Prompt completed: stopReason=${promptResult.stopReason || 'N/A'}`, + ); + results.details.stopReason = promptResult.stopReason; + + // Analyze results + await sleep(500); + collectAcpMetrics(client, results); + evaluateExpectations(expectations, client, results); + + results.passed = results.errors.length === 0; + } catch (error) { + results.errors.push(error.message); + results.details.fatalError = error.message; + console.error(`\n ✗ FATAL: ${error.message}`); + } finally { + try { + child.kill('SIGTERM'); + } catch { + // Process may have already exited + } + await sleep(500); + try { + child.kill('SIGKILL'); + } catch { + // Process may have already exited + } + } + + return results; +} + +// --------------------------------------------------------------------------- +// Expectation helpers +// --------------------------------------------------------------------------- + +function expectNoStderrErrors() { + return { + name: 'No stderr "isRunning" / "not a function" errors', + check: (client) => { + const hasError = client.errors.some( + (e) => e.includes('isRunning') || e.includes('not a function'), + ); + return { + passed: !hasError, + message: hasError + ? `Found error in stderr: ${client.errors.find((e) => e.includes('isRunning'))?.substring(0, 200)}` + : 'No isRunning/not-a-function errors detected', + }; + }, + }; +} + +function expectTodoPauseStopsLoop() { + return { + name: 'pause tool stops continuation (stopReason is end_turn, not loop-detected)', + check: (client, details) => { + // After the pause tool, the agent should stop — not continue the loop. + // We check: the stopReason should be 'end_turn' and there should + // NOT be excessive tool calls (which would indicate looping). + const stopReason = details.stopReason; + const timedOut = details.timedOut === true; + // A timeout means the loop never stopped — that's the bug. + // Otherwise, a normal end_turn/cancelled is correct behavior. + const validStopReason = + stopReason === 'end_turn' || stopReason === 'cancelled'; + const passed = + !timedOut && + (validStopReason || + (stopReason === undefined && details.toolCallCount < 10)); + return { + passed, + message: timedOut + ? `TIMEOUT after ${details.timeout || 'N/A'}ms — loop did not stop (BUG REPRODUCED)` + : `stopReason=${stopReason}, toolCalls=${details.toolCallCount}`, + }; + }, + }; +} + +// --------------------------------------------------------------------------- +// Utilities +// --------------------------------------------------------------------------- + +function sleep(ms) { + return new Promise((r) => setTimeout(r, ms)); +} + +function summarizeResults(allResults) { + console.log(`\n${'='.repeat(70)}`); + console.log('SUMMARY'); + console.log(`${'='.repeat(70)}`); + + let allPassed = true; + for (const r of allResults) { + const icon = r.passed ? '✅' : '❌'; + console.log(`${icon} ${r.label}`); + if (r.errors.length > 0) { + for (const e of r.errors) { + console.log(` → ${e}`); + } + allPassed = false; + } + } + + console.log( + `\n${allPassed ? '✅ ALL TESTS PASSED' : '❌ SOME TESTS FAILED'}`, + ); + return allPassed; +} + +// --------------------------------------------------------------------------- +// Main +// --------------------------------------------------------------------------- + +async function main() { + const opts = parseArgs(); + + const results = []; + + // Scenario 1: pause tool should stop the loop + results.push( + await runScenario( + opts, + 'pause tool stops continuation in ACP mode', + 'Use the todo_write tool to create a single todo item that says "test". ' + + 'Then immediately call todo_pause with reason "testing pause in ACP mode". ' + + 'Do not do anything else. Do not continue working.', + [expectTodoPauseStopsLoop(), expectNoStderrErrors()], + ), + ); + + // Scenario 2: task tool should not crash with isRunning error + results.push( + await runScenario( + opts, + 'task tool does not crash with isRunning error in ACP mode', + 'Use the task tool to launch the deepthinker subagent with the goal "write a single haiku". ' + + 'Do not use any other tools. Return the haiku.', + [expectNoStderrErrors()], + ), + ); + + const allPassed = summarizeResults(results); + process.exit(allPassed ? 0 : 1); +} + +main().catch((err) => { + console.error('Unhandled error:', err); + process.exit(1); +});