From 0fde06b6294761ea52583a6d4d7deb00931badb1 Mon Sep 17 00:00:00 2001 From: Suho Han Date: Thu, 13 Aug 2026 15:28:28 +0900 Subject: [PATCH 01/15] feat(app): add gjc ACP provider catalog entry --- packages/app/src/assets/acp-provider-icons.ts | 1 + packages/app/src/assets/acp-provider-icons/gjc.svg | 3 +++ packages/app/src/components/provider-icon-name.test.ts | 1 + packages/app/src/data/acp-provider-catalog.ts | 10 ++++++++++ .../app/src/hooks/use-acp-provider-catalog.test.ts | 7 +++++++ packages/protocol/src/provider-icon-names.ts | 1 + 6 files changed, 23 insertions(+) create mode 100644 packages/app/src/assets/acp-provider-icons/gjc.svg diff --git a/packages/app/src/assets/acp-provider-icons.ts b/packages/app/src/assets/acp-provider-icons.ts index c31f51d46e..3d14b04623 100644 --- a/packages/app/src/assets/acp-provider-icons.ts +++ b/packages/app/src/assets/acp-provider-icons.ts @@ -64,6 +64,7 @@ export const ACP_PROVIDER_ICON_SVGS = { '\n \n\n', "qwen-code": '\n', + gjc: '\n 🦞\n\n', sigit: '\n\n\n\n\n\n\n\n\n\n\n\n\n', stakpak: diff --git a/packages/app/src/assets/acp-provider-icons/gjc.svg b/packages/app/src/assets/acp-provider-icons/gjc.svg new file mode 100644 index 0000000000..0f8c3031a7 --- /dev/null +++ b/packages/app/src/assets/acp-provider-icons/gjc.svg @@ -0,0 +1,3 @@ + + 🦞 + diff --git a/packages/app/src/components/provider-icon-name.test.ts b/packages/app/src/components/provider-icon-name.test.ts index fabfbd25e4..a0b25d63d7 100644 --- a/packages/app/src/components/provider-icon-name.test.ts +++ b/packages/app/src/components/provider-icon-name.test.ts @@ -18,6 +18,7 @@ describe("resolveProviderIconName", () => { it("returns the catalog identifier for ACP catalog provider ids that ship an icon", () => { expect(resolveProviderIconName("amp-acp")).toEqual({ kind: "catalog", id: "amp-acp" }); expect(resolveProviderIconName("gemini")).toEqual({ kind: "catalog", id: "gemini" }); + expect(resolveProviderIconName("gjc")).toEqual({ kind: "catalog", id: "gjc" }); expect(resolveProviderIconName("traecli")).toEqual({ kind: "catalog", id: "traecli" }); }); diff --git a/packages/app/src/data/acp-provider-catalog.ts b/packages/app/src/data/acp-provider-catalog.ts index eab70de701..a7068798bc 100644 --- a/packages/app/src/data/acp-provider-catalog.ts +++ b/packages/app/src/data/acp-provider-catalog.ts @@ -187,6 +187,16 @@ const CATALOG_DATA = [ installLink: "https://geminicli.com", command: ["npx", "-y", "@google/gemini-cli@0.52.0", "--acp"], }, + { + id: "gjc", + title: "Gajae Code", + description: + "External coding-agent harness with structured planning, persistent evidence, tmux-backed workers, and ACP support", + version: "manual", + iconId: "gjc", + installLink: "https://github.com/Yeachan-Heo/gajae-code", + command: ["gjc", "acp"], + }, { id: "glm-acp-agent", title: "GLM Agent", diff --git a/packages/app/src/hooks/use-acp-provider-catalog.test.ts b/packages/app/src/hooks/use-acp-provider-catalog.test.ts index 87e6d3c866..b697e8b58c 100644 --- a/packages/app/src/hooks/use-acp-provider-catalog.test.ts +++ b/packages/app/src/hooks/use-acp-provider-catalog.test.ts @@ -34,6 +34,12 @@ describe("ACP provider catalog", () => { } }); + it("uses the lobster emoji for the Gajae Code catalog icon", () => { + const iconSvg = findProvider("gjc").iconSvg; + expect(iconSvg).toContain("🦞"); + expect(iconSvg).not.toContain("🦞"); + }); + it("does not offer Pi's unsupported ACP adapter", () => { expect(ACP_PROVIDER_CATALOG.some((entry) => entry.id === "pi-acp")).toBe(false); }); @@ -43,6 +49,7 @@ describe("ACP provider catalog", () => { expect(findProvider("cursor").command).toEqual(["cursor-agent", "acp"]); expect(findProvider("codewhale").command).toEqual(["codewhale", "serve", "--acp"]); expect(findProvider("devin").command).toEqual(["devin", "acp"]); + expect(findProvider("gjc").command).toEqual(["gjc", "acp"]); expect(findProvider("goose").command).toEqual(["goose", "acp"]); expect(findProvider("junie").command).toEqual(["junie", "--acp", "true"]); expect(findProvider("kiro").command).toEqual(["kiro-cli", "acp"]); diff --git a/packages/protocol/src/provider-icon-names.ts b/packages/protocol/src/provider-icon-names.ts index c471a68c6f..a859fb4962 100644 --- a/packages/protocol/src/provider-icon-names.ts +++ b/packages/protocol/src/provider-icon-names.ts @@ -27,6 +27,7 @@ export const ACP_PROVIDER_ICON_NAMES = [ "factory-droid", "fast-agent", "gemini", + "gjc", "glm-acp-agent", "goose", "grok", From 881843213bef34b788cec9ea1d96ef18fc515009 Mon Sep 17 00:00:00 2001 From: Suho Han Date: Thu, 13 Aug 2026 16:44:33 +0900 Subject: [PATCH 02/15] feat(server): wire GJC ACP permission handling --- .../server/agent/provider-registry.test.ts | 93 +++++++++++++++++++ .../src/server/agent/provider-registry.ts | 4 + .../server/agent/providers/acp-agent.test.ts | 22 +++++ .../agent/providers/generic-acp-agent.test.ts | 33 ++++++- .../agent/providers/generic-acp-agent.ts | 53 +++++++++-- .../agent/providers/gjc-acp-agent.test.ts | 64 +++++++++++++ .../server/agent/providers/gjc-acp-agent.ts | 38 ++++++++ 7 files changed, 299 insertions(+), 8 deletions(-) create mode 100644 packages/server/src/server/agent/providers/gjc-acp-agent.test.ts create mode 100644 packages/server/src/server/agent/providers/gjc-acp-agent.ts diff --git a/packages/server/src/server/agent/provider-registry.test.ts b/packages/server/src/server/agent/provider-registry.test.ts index 656349e10b..f0dde1d4b1 100644 --- a/packages/server/src/server/agent/provider-registry.test.ts +++ b/packages/server/src/server/agent/provider-registry.test.ts @@ -38,6 +38,11 @@ const mockState = vi.hoisted(() => { env?: Record; providerParams?: unknown; }>, + gjc: [] as Array<{ + command: string[]; + env?: Record; + providerParams?: unknown; + }>, trae: [] as Array<{ command: string[]; env?: Record; @@ -65,6 +70,7 @@ const mockState = vi.hoisted(() => { this.constructorArgs.codex = []; this.constructorArgs.copilot = []; this.constructorArgs.cursor = []; + this.constructorArgs.gjc = []; this.constructorArgs.trae = []; this.constructorArgs.kimi = []; this.constructorArgs.pi = []; @@ -356,6 +362,59 @@ vi.mock("./providers/generic-acp-agent.js", () => ({ }, })); +vi.mock("./providers/gjc-acp-agent.js", () => ({ + GjcACPAgentClient: class GjcACPAgentClient { + readonly capabilities = { + supportsStreaming: true, + supportsSessionPersistence: true, + supportsDynamicModes: true, + supportsMcpServers: true, + supportsReasoningStream: true, + supportsToolInvocations: true, + }; + readonly provider = "acp"; + readonly runtimeSettings?: unknown; + + constructor(options: { + command: string[]; + env?: Record; + providerParams?: unknown; + }) { + this.runtimeSettings = { + command: { + mode: "replace", + argv: options.command, + }, + env: options.env, + }; + mockState.constructorArgs.gjc.push({ + command: options.command, + env: options.env, + providerParams: options.providerParams, + }); + } + + async createSession(): Promise { + throw new Error("not implemented"); + } + + async resumeSession(): Promise { + throw new Error("not implemented"); + } + + async fetchCatalog(): Promise { + return { + models: mockState.runtimeModels.get(this.provider) ?? [], + modes: [], + }; + } + + async isAvailable(): Promise { + return true; + } + }, +})); + vi.mock("./providers/cursor-acp-agent.js", () => ({ CursorACPAgentClient: class CursorACPAgentClient { readonly capabilities = { @@ -825,6 +884,40 @@ test("ACP provider params can disable MCP support", () => { ]); }); +test("gjc provider extending acp uses GjcACPAgentClient", () => { + const registry = buildProviderRegistry(logger, { + providerOverrides: { + gjc: { + extends: "acp", + label: "Gajae Code", + command: ["gjc", "acp"], + env: { + GJC_LOG: "debug", + }, + }, + }, + }); + + expect(registry.gjc.createClient(logger).provider).toBe("gjc"); + expect(mockState.constructorArgs.gjc).toEqual([ + { + command: ["gjc", "acp"], + env: { + GJC_LOG: "debug", + }, + providerParams: undefined, + }, + { + command: ["gjc", "acp"], + env: { + GJC_LOG: "debug", + }, + providerParams: undefined, + }, + ]); + expect(mockState.constructorArgs.genericAcp).toEqual([]); +}); + test("cursor provider extending acp uses CursorACPAgentClient", () => { const registry = buildProviderRegistry(logger, { providerOverrides: { diff --git a/packages/server/src/server/agent/provider-registry.ts b/packages/server/src/server/agent/provider-registry.ts index b673e2bd75..7b9a9606a9 100644 --- a/packages/server/src/server/agent/provider-registry.ts +++ b/packages/server/src/server/agent/provider-registry.ts @@ -37,6 +37,7 @@ import { CodexAppServerAgentClient } from "./providers/codex-app-server-agent.js import { CopilotACPAgentClient } from "./providers/copilot-acp-agent.js"; import { CursorACPAgentClient } from "./providers/cursor-acp-agent.js"; import { GenericACPAgentClient } from "./providers/generic-acp-agent.js"; +import { GjcACPAgentClient } from "./providers/gjc-acp-agent.js"; import { KimiACPAgentClient } from "./providers/kimi-acp-agent.js"; import { KiroACPAgentClient } from "./providers/kiro-acp-agent.js"; import { OpenCodeAgentClient } from "./providers/opencode-agent.js"; @@ -776,6 +777,9 @@ function addDerivedProviders( if (providerId === "cursor") { return new CursorACPAgentClient(acpOptions); } + if (providerId === "gjc") { + return new GjcACPAgentClient(acpOptions); + } if (providerId === "kimi") { return new KimiACPAgentClient(acpOptions); } diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index 74cd504c28..b7c0253703 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -82,6 +82,28 @@ describe("buildACPClientCapabilities", () => { _meta: { source: "provider" }, }); }); + + test("can delegate terminal execution while keeping filesystem execution with the agent", () => { + expect( + buildACPClientCapabilities( + { gjc: { permissionHandling: "prompt" } }, + { + terminal: true, + }, + ), + ).toEqual({ + fs: { + readTextFile: false, + writeTextFile: false, + }, + terminal: true, + _meta: { + gjc: { + permissionHandling: "prompt", + }, + }, + }); + }); }); interface ACPSessionInternals { diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.test.ts b/packages/server/src/server/agent/providers/generic-acp-agent.test.ts index 19a1e9f8ee..a48dab1fa3 100644 --- a/packages/server/src/server/agent/providers/generic-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/generic-acp-agent.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test, vi } from "vitest"; +import { beforeEach, describe, expect, test, vi } from "vitest"; import { createTestLogger } from "../../../test-utils/test-logger.js"; @@ -31,6 +31,10 @@ vi.mock("./acp-agent.js", () => ({ import { GenericACPAgentClient } from "./generic-acp-agent.js"; describe("GenericACPAgentClient", () => { + beforeEach(() => { + mockState.superConstructorOptions = []; + }); + test("passes the custom command only as defaultCommand", () => { const _client = new GenericACPAgentClient({ logger: createTestLogger(), @@ -82,4 +86,31 @@ describe("GenericACPAgentClient", () => { }, }); }); + + test("merges wrapper client capability defaults with provider params", () => { + const _client = new GenericACPAgentClient({ + logger: createTestLogger(), + command: ["capable-acp", "serve"], + clientCapabilities: { + terminal: true, + }, + providerParams: { + clientCapabilities: { + fs: { + readTextFile: true, + }, + }, + }, + }); + void _client; + + expect(mockState.superConstructorOptions.at(-1)).toMatchObject({ + clientCapabilities: { + fs: { + readTextFile: true, + }, + terminal: true, + }, + }); + }); }); diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.ts b/packages/server/src/server/agent/providers/generic-acp-agent.ts index 095d4e10a8..f1eb0499d0 100644 --- a/packages/server/src/server/agent/providers/generic-acp-agent.ts +++ b/packages/server/src/server/agent/providers/generic-acp-agent.ts @@ -47,6 +47,7 @@ interface GenericACPAgentClientOptions { waitForInitialCommands?: boolean; initialCommandsWaitTimeoutMs?: number; diagnosticPhaseTimeoutMs?: number; + clientCapabilities?: GenericACPProviderParams["clientCapabilities"]; clientCapabilityMeta?: ACPClientCapabilityMeta; configFeatureOptions?: ACPConfigFeatureOption[]; extensionCommandsParser?: ACPExtensionCommandsParser; @@ -61,6 +62,10 @@ export class GenericACPAgentClient extends ACPAgentClient { constructor(options: GenericACPAgentClientOptions) { const providerParams = parseGenericACPProviderParams(options.providerParams); + const clientCapabilities = mergeGenericACPClientCapabilities( + options.clientCapabilities, + providerParams.clientCapabilities, + ); super({ provider: "acp", logger: options.logger, @@ -69,13 +74,25 @@ export class GenericACPAgentClient extends ACPAgentClient { }, defaultCommand: options.command, capabilities: buildGenericACPCapabilities(providerParams), - waitForInitialCommands: options.waitForInitialCommands, - initialCommandsWaitTimeoutMs: options.initialCommandsWaitTimeoutMs, - clientCapabilities: providerParams.clientCapabilities, - clientCapabilityMeta: options.clientCapabilityMeta, - configFeatureOptions: options.configFeatureOptions, - extensionCommandsParser: options.extensionCommandsParser, - catalogModelResolver: options.catalogModelResolver, + ...(options.waitForInitialCommands !== undefined + ? { waitForInitialCommands: options.waitForInitialCommands } + : {}), + ...(options.initialCommandsWaitTimeoutMs !== undefined + ? { initialCommandsWaitTimeoutMs: options.initialCommandsWaitTimeoutMs } + : {}), + ...(clientCapabilities ? { clientCapabilities } : {}), + ...(options.clientCapabilityMeta + ? { clientCapabilityMeta: options.clientCapabilityMeta } + : {}), + ...(options.configFeatureOptions + ? { configFeatureOptions: options.configFeatureOptions } + : {}), + ...(options.extensionCommandsParser + ? { extensionCommandsParser: options.extensionCommandsParser } + : {}), + ...(options.catalogModelResolver + ? { catalogModelResolver: options.catalogModelResolver } + : {}), }); this.command = options.command; @@ -168,6 +185,28 @@ function buildGenericACPCapabilities(params: GenericACPProviderParams): AgentCap }; } +function mergeGenericACPClientCapabilities( + defaults: GenericACPProviderParams["clientCapabilities"], + overrides: GenericACPProviderParams["clientCapabilities"], +): GenericACPProviderParams["clientCapabilities"] { + if (!defaults && !overrides) { + return undefined; + } + + return { + ...defaults, + ...overrides, + ...(defaults?.fs || overrides?.fs + ? { + fs: { + ...defaults?.fs, + ...overrides?.fs, + }, + } + : {}), + }; +} + function parseGenericACPProviderParams(params: unknown): GenericACPProviderParams { return GenericACPProviderParamsSchema.parse(params ?? {}); } diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts new file mode 100644 index 0000000000..dab84df374 --- /dev/null +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts @@ -0,0 +1,64 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; + +import { createTestLogger } from "../../../test-utils/test-logger.js"; + +const mockState = vi.hoisted(() => ({ + genericConstructorOptions: [] as unknown[], +})); + +vi.mock("./generic-acp-agent.js", () => ({ + GenericACPAgentClient: class GenericACPAgentClient { + readonly provider = "acp"; + + constructor(options: unknown) { + mockState.genericConstructorOptions.push(options); + } + }, +})); + +import { GjcACPAgentClient } from "./gjc-acp-agent.js"; + +describe("GjcACPAgentClient", () => { + beforeEach(() => { + mockState.genericConstructorOptions = []; + }); + + test("enables Paseo terminal execution and prompt permission handling for GJC ACP", () => { + const _client = new GjcACPAgentClient({ + logger: createTestLogger(), + command: ["gjc", "acp"], + env: { + GJC_LOG: "debug", + }, + providerId: "gjc", + label: "Gajae Code", + providerParams: { + supportsMcpServers: false, + }, + }); + void _client; + + expect(mockState.genericConstructorOptions).toEqual([ + { + logger: expect.any(Object), + command: ["gjc", "acp"], + env: { + GJC_LOG: "debug", + }, + providerId: "gjc", + label: "Gajae Code", + providerParams: { + supportsMcpServers: false, + }, + clientCapabilities: { + terminal: true, + }, + clientCapabilityMeta: { + gjc: { + permissionHandling: "prompt", + }, + }, + }, + ]); + }); +}); diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.ts new file mode 100644 index 0000000000..6cee3fc2b3 --- /dev/null +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.ts @@ -0,0 +1,38 @@ +import type { Logger } from "pino"; + +import type { ACPClientCapabilityMeta } from "./acp-agent.js"; +import { GenericACPAgentClient } from "./generic-acp-agent.js"; + +interface GjcACPAgentClientOptions { + logger: Logger; + command: [string, ...string[]]; + env?: Record; + providerId?: string; + label?: string; + providerParams?: unknown; +} + +const GJC_CLIENT_CAPABILITIES = { + terminal: true, +}; + +const GJC_CLIENT_CAPABILITY_META = { + gjc: { + permissionHandling: "prompt", + }, +} satisfies ACPClientCapabilityMeta; + +export class GjcACPAgentClient extends GenericACPAgentClient { + constructor(options: GjcACPAgentClientOptions) { + super({ + logger: options.logger, + command: options.command, + env: options.env, + providerId: options.providerId, + label: options.label, + providerParams: options.providerParams, + clientCapabilities: GJC_CLIENT_CAPABILITIES, + clientCapabilityMeta: GJC_CLIENT_CAPABILITY_META, + }); + } +} From c8888ca28125abcf18a35a071a70a00599de21cb Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 19:18:22 +0900 Subject: [PATCH 03/15] fix(gjc): create ACP sessions through lifecycle API --- .../provider-selection.test.ts | 18 + .../provider-selection/provider-selection.ts | 11 +- .../server/agent/providers/acp-agent.test.ts | 129 ++++++ .../src/server/agent/providers/acp-agent.ts | 226 ++++++--- .../agent/providers/generic-acp-agent.ts | 18 + .../agent/providers/gjc-acp-agent.test.ts | 327 ++++++++++++- .../server/agent/providers/gjc-acp-agent.ts | 429 +++++++++++++++++- 7 files changed, 1100 insertions(+), 58 deletions(-) diff --git a/packages/app/src/provider-selection/provider-selection.test.ts b/packages/app/src/provider-selection/provider-selection.test.ts index 690bd439af..c1620e0498 100644 --- a/packages/app/src/provider-selection/provider-selection.test.ts +++ b/packages/app/src/provider-selection/provider-selection.test.ts @@ -215,6 +215,24 @@ describe("combined model selector data", () => { expect(matchesModelSearch(row, "kimi gemini")).toBe(false); }); + it("matches model ids qualified by a thinking option suffix", () => { + const row = { + favoriteKey: "gjc:openai-codex/gpt-5.5", + provider: "gjc", + providerLabel: "Gajae Code", + modelId: "openai-codex/gpt-5.5", + modelLabel: "GPT-5.5", + description: "openai-codex/gpt-5.5", + thinkingOptions: [ + { id: "high", label: "High" }, + { id: "xhigh", label: "Extra high" }, + ], + }; + + expect(matchesModelSearch(row, "openai-codex/gpt-5.5:xhigh")).toBe(true); + expect(matchesModelSearch(row, "openai-codex/gpt-5.5:max")).toBe(false); + }); + it("ranks model search results by fuzzy match quality", () => { const rows = [ { diff --git a/packages/app/src/provider-selection/provider-selection.ts b/packages/app/src/provider-selection/provider-selection.ts index ca8be17b85..d15e667a8f 100644 --- a/packages/app/src/provider-selection/provider-selection.ts +++ b/packages/app/src/provider-selection/provider-selection.ts @@ -23,6 +23,7 @@ export interface ProviderSelectionModelRow { modelLabel: string; description?: string; isDefault?: boolean; + thinkingOptions?: AgentModelDefinition["thinkingOptions"]; } function buildModelRowKey(provider: string, modelId: string): string { @@ -67,6 +68,7 @@ function buildModelRows( modelLabel: model.label, description: model.description ?? model.id, isDefault: model.isDefault, + ...(model.thinkingOptions?.length ? { thinkingOptions: model.thinkingOptions } : {}), })); } @@ -230,7 +232,14 @@ export function matchesModelSearch( } function getModelRowSearchFields(row: ProviderSelectionModelRow): string[] { - return [row.modelLabel, row.modelId, row.providerLabel, row.description ?? ""]; + const fields = [row.modelLabel, row.modelId, row.providerLabel, row.description ?? ""]; + for (const option of row.thinkingOptions ?? []) { + fields.push(`${row.modelId}:${option.id}`); + fields.push(`${row.modelLabel}:${option.id}`); + fields.push(`${row.modelId}:${option.label}`); + fields.push(`${row.modelLabel}:${option.label}`); + } + return fields; } export function scoreModelRow(row: ProviderSelectionModelRow, normalizedQuery: string) { diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index b7c0253703..11f1797a28 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -2032,6 +2032,68 @@ describe("ACPAgentClient config features", () => { }), ]); }); + + test("uses the custom starter and closer for feature probe sessions", async () => { + const newSession = vi.fn().mockRejectedValue(new Error("newSession should not be called")); + const newSessionStarter = vi.fn(async () => ({ + sessionId: "probe-session-1", + configOptions: [copilotAgentConfigOption("Probe Agent")], + })); + const probeSessionCloser = vi.fn(async () => undefined); + + class TestACPAgentClient extends ACPAgentClient { + protected override async spawnProcess(): Promise { + return { + child: { kill: vi.fn(), exitCode: 0, signalCode: null, once: vi.fn() }, + connection: { + newSession, + } as unknown as ClientSideConnection, + initialize: { agentCapabilities: {} }, + } as SpawnedACPProcess; + } + + protected override async closeProbe(): Promise {} + } + + const client = new TestACPAgentClient({ + provider: "copilot", + logger: createTestLogger(), + defaultCommand: ["copilot", "--acp"], + configFeatureOptions: [COPILOT_AGENT_FEATURE_OPTION], + newSessionStarter, + probeSessionCloser, + }); + + await expect( + client.listFeatures({ + provider: "copilot", + cwd: "/tmp/acp-features", + }), + ).resolves.toEqual([ + expect.objectContaining({ + type: "toggle", + id: "auto_accept", + }), + expect.objectContaining({ + type: "select", + id: "agent", + value: "Probe Agent", + }), + ]); + + expect(newSession).not.toHaveBeenCalled(); + expect(probeSessionCloser).toHaveBeenCalledWith({ + response: { + sessionId: "probe-session-1", + configOptions: [copilotAgentConfigOption("Probe Agent")], + }, + config: { + provider: "copilot", + cwd: "/tmp/acp-features", + }, + mcpServers: [], + }); + }); }); describe("ACPAgentClient sessionResponseTransformer", () => { @@ -2120,6 +2182,73 @@ describe("ACPAgentClient fetchCatalog", () => { }); }); + test("uses the custom starter and closer for catalog probe sessions", async () => { + const newSession = vi.fn().mockRejectedValue(new Error("newSession should not be called")); + const loadSession = vi.fn(); + const newSessionStarter = vi.fn(async () => ({ + sessionId: "probe-session-1", + modes: null, + models: null, + configOptions: [], + })); + const probeSessionCloser = vi.fn(async () => undefined); + + class TestACPAgentClient extends ACPAgentClient { + protected override async spawnProcess(): Promise { + return { + child: { kill: vi.fn(), exitCode: 0, signalCode: null, once: vi.fn() }, + connection: { + newSession, + loadSession, + } as unknown as ClientSideConnection, + initialize: { agentCapabilities: {} }, + } as SpawnedACPProcess; + } + + protected override async closeProbe(): Promise {} + } + + const client = new TestACPAgentClient({ + provider: "gjc", + logger: createTestLogger(), + defaultCommand: ["gjc", "acp"], + defaultModes: [], + newSessionStarter, + probeSessionCloser, + }); + + await expect( + client.fetchCatalog({ scope: "workspace", cwd: "/tmp/acp-catalog-cwd", force: false }), + ).resolves.toEqual({ models: [], modes: [] }); + + expect(newSession).not.toHaveBeenCalled(); + expect(newSessionStarter).toHaveBeenCalledWith({ + connection: expect.objectContaining({ + newSession, + loadSession, + }), + config: { + provider: "gjc", + cwd: "/tmp/acp-catalog-cwd", + }, + mcpServers: [], + runRequest: expect.any(Function), + }); + expect(probeSessionCloser).toHaveBeenCalledWith({ + response: { + sessionId: "probe-session-1", + modes: null, + models: null, + configOptions: [], + }, + config: { + provider: "gjc", + cwd: "/tmp/acp-catalog-cwd", + }, + mcpServers: [], + }); + }); + test("returns an empty modes array when no ACP modes are reported and fallback modes are empty", async () => { class TestACPAgentClient extends ACPAgentClient { protected override async spawnProcess(): Promise { diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 2296aa3960..b97b89d93b 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -402,6 +402,25 @@ export type ACPCatalogModelResolver = ( context: ACPCatalogModelResolverContext, ) => Promise; +export interface ACPNewSessionStarterContext { + connection: ClientSideConnection; + config: AgentSessionConfig; + mcpServers: McpServer[]; + runRequest: (request: () => Promise) => Promise; +} + +export type ACPNewSessionStarter = ( + context: ACPNewSessionStarterContext, +) => Promise; + +export interface ACPProbeSessionCloserContext { + response: SessionStateResponse; + config: AgentSessionConfig; + mcpServers: McpServer[]; +} + +export type ACPProbeSessionCloser = (context: ACPProbeSessionCloserContext) => Promise; + interface ACPAgentClientOptions { provider: string; logger: Logger; @@ -409,6 +428,8 @@ interface ACPAgentClientOptions { defaultCommand: [string, ...string[]]; defaultModes?: AgentMode[]; catalogModelResolver?: ACPCatalogModelResolver; + newSessionStarter?: ACPNewSessionStarter; + probeSessionCloser?: ACPProbeSessionCloser; modelTransformer?: (models: AgentModelDefinition[]) => AgentModelDefinition[]; sessionResponseTransformer?: (response: SessionStateResponse) => SessionStateResponse; configOptionsTransformer?: (configOptions: SessionConfigOption[]) => SessionConfigOption[]; @@ -439,6 +460,7 @@ interface ACPAgentSessionOptions { runtimeSettings?: ProviderRuntimeSettings; defaultCommand: [string, ...string[]]; defaultModes: AgentMode[]; + newSessionStarter?: ACPNewSessionStarter; modelTransformer?: (models: AgentModelDefinition[]) => AgentModelDefinition[]; sessionResponseTransformer?: (response: SessionStateResponse) => SessionStateResponse; configOptionsTransformer?: (configOptions: SessionConfigOption[]) => SessionConfigOption[]; @@ -792,6 +814,8 @@ export class ACPAgentClient implements AgentClient { protected readonly defaultCommand: [string, ...string[]]; protected readonly defaultModes: AgentMode[]; private readonly catalogModelResolver?: ACPCatalogModelResolver; + private readonly newSessionStarter?: ACPNewSessionStarter; + private readonly probeSessionCloser?: ACPProbeSessionCloser; private readonly modelTransformer?: (models: AgentModelDefinition[]) => AgentModelDefinition[]; private readonly sessionResponseTransformer?: ( response: SessionStateResponse, @@ -832,6 +856,8 @@ export class ACPAgentClient implements AgentClient { this.defaultCommand = options.defaultCommand; this.defaultModes = options.defaultModes ?? []; this.catalogModelResolver = options.catalogModelResolver; + this.newSessionStarter = options.newSessionStarter; + this.probeSessionCloser = options.probeSessionCloser; this.modelTransformer = options.modelTransformer; this.sessionResponseTransformer = options.sessionResponseTransformer; this.configOptionsTransformer = options.configOptionsTransformer; @@ -861,6 +887,7 @@ export class ACPAgentClient implements AgentClient { runtimeSettings: this.runtimeSettings, defaultCommand: this.defaultCommand, defaultModes: this.defaultModes, + newSessionStarter: this.newSessionStarter, modelTransformer: this.modelTransformer, sessionResponseTransformer: this.sessionResponseTransformer, configOptionsTransformer: this.configOptionsTransformer, @@ -947,42 +974,50 @@ export class ACPAgentClient implements AgentClient { }, }); probe = initializedProbe; - const response = await this.runACPRequest(() => - initializedProbe.connection.newSession({ - cwd, - mcpServers: [], - }), - ); - const transformed = this.transformSessionResponse(response); - const derivedModels = deriveModelDefinitionsFromACP( - this.provider, - transformed.models, - transformed.configOptions, - ); - const models = this.catalogModelResolver - ? await this.catalogModelResolver({ - connection: initializedProbe.connection, - sessionId: response.sessionId, - models: derivedModels, - configOptions: transformed.configOptions, - runRequest: (request) => this.runACPRequest(request), - transformConfigOptions: (configOptions) => - this.configOptionsTransformer - ? this.configOptionsTransformer(configOptions) - : configOptions, - logger: this.logger, - provider: this.provider, - }) - : derivedModels; - const modeInfo = deriveModesFromACP( - this.defaultModes, - transformed.modes, - transformed.configOptions, - ); - return { - models: this.modelTransformer ? this.modelTransformer(models) : models, - modes: modeInfo.modes, - }; + const config: AgentSessionConfig = { provider: this.provider, cwd }; + const mcpServers: McpServer[] = []; + let response: SessionStateResponse | null = null; + try { + response = await this.startProbeSession({ + connection: initializedProbe.connection, + config, + mcpServers, + }); + const transformed = this.transformSessionResponse(response); + const derivedModels = deriveModelDefinitionsFromACP( + this.provider, + transformed.models, + transformed.configOptions, + ); + const models = this.catalogModelResolver + ? await this.catalogModelResolver({ + connection: initializedProbe.connection, + sessionId: requireACPResponseSessionId(response, this.provider, "catalog probe"), + models: derivedModels, + configOptions: transformed.configOptions, + runRequest: (request) => this.runACPRequest(request), + transformConfigOptions: (configOptions) => + this.configOptionsTransformer + ? this.configOptionsTransformer(configOptions) + : configOptions, + logger: this.logger, + provider: this.provider, + }) + : derivedModels; + const modeInfo = deriveModesFromACP( + this.defaultModes, + transformed.modes, + transformed.configOptions, + ); + return { + models: this.modelTransformer ? this.modelTransformer(models) : models, + modes: modeInfo.modes, + }; + } finally { + if (response) { + await this.closeProbeSession({ response, config, mcpServers }); + } + } })(); return await withTimeout( @@ -1005,19 +1040,27 @@ export class ACPAgentClient implements AgentClient { this.assertProvider(config); const probe = await this.spawnProcess(PROBE_ENV); + const mcpServers: McpServer[] = []; + let response: SessionStateResponse | null = null; try { - const response = await this.runACPRequest(() => - probe.connection.newSession({ - cwd: config.cwd, - mcpServers: [], - }), - ); + response = await this.startProbeSession({ + connection: probe.connection, + config: { ...config, provider: this.provider }, + mcpServers, + }); const transformed = this.transformSessionResponse(response); return [ autoAcceptFeature, ...deriveFeaturesFromACP(transformed.configOptions, this.configFeatureOptions), ]; } finally { + if (response) { + await this.closeProbeSession({ + response, + config: { ...config, provider: this.provider }, + mcpServers, + }); + } await this.closeProbe(probe); } } @@ -1279,14 +1322,16 @@ export class ACPAgentClient implements AgentClient { } const sessionStartedAt = Date.now(); + const config: AgentSessionConfig = { provider: this.provider, cwd }; + const mcpServers: McpServer[] = []; + let response: SessionStateResponse | null = null; try { - const response = await withTimeout( - this.runACPRequest(() => - activeTransport.connection.newSession({ - cwd, - mcpServers: [], - }), - ), + response = await withTimeout( + this.startProbeSession({ + connection: activeTransport.connection, + config, + mcpServers, + }), phaseTimeoutMs, `ACP session/new timed out after ${phaseTimeoutMs}ms`, ); @@ -1314,6 +1359,10 @@ export class ACPAgentClient implements AgentClient { }); pushACPStderrRow(rows, activeTransport.stderrChunks); return rows; + } finally { + if (response) { + await this.closeProbeSession({ response, config, mcpServers }); + } } pushACPStderrRow(rows, activeTransport.stderrChunks); @@ -1370,6 +1419,63 @@ export class ACPAgentClient implements AgentClient { configOptions: this.configOptionsTransformer(transformed.configOptions), }; } + + private async startProbeSession(context: { + connection: ClientSideConnection; + config: AgentSessionConfig; + mcpServers: McpServer[]; + }): Promise { + if (this.newSessionStarter) { + return await this.newSessionStarter({ + ...context, + runRequest: (request) => this.runACPRequest(request), + }); + } + + return await this.runACPRequest(() => + context.connection.newSession({ + cwd: context.config.cwd, + mcpServers: context.mcpServers, + }), + ); + } + + private async closeProbeSession(context: ACPProbeSessionCloserContext): Promise { + if (!this.probeSessionCloser) { + return; + } + + try { + await this.probeSessionCloser(context); + } catch (error) { + this.logger.warn( + { + err: error, + sessionId: getACPResponseSessionId(context.response) ?? undefined, + cwd: context.config.cwd, + }, + "Failed to close ACP probe session", + ); + } + } +} + +function getACPResponseSessionId(response: SessionStateResponse): string | null { + return "sessionId" in response && typeof response.sessionId === "string" + ? response.sessionId + : null; +} + +function requireACPResponseSessionId( + response: SessionStateResponse, + provider: string, + context: string, +): string { + const sessionId = getACPResponseSessionId(response); + if (!sessionId) { + throw new Error(`${provider} ACP ${context} did not expose a session id`); + } + return sessionId; } export class ACPAgentSession implements AgentSession, ACPClient { @@ -1380,6 +1486,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { private readonly runtimeSettings?: ProviderRuntimeSettings; private readonly defaultCommand: [string, ...string[]]; private readonly defaultModes: AgentMode[]; + private readonly newSessionStarter?: ACPNewSessionStarter; protected readonly modelTransformer?: (models: AgentModelDefinition[]) => AgentModelDefinition[]; private readonly sessionResponseTransformer?: ( response: SessionStateResponse, @@ -1450,6 +1557,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { this.runtimeSettings = options.runtimeSettings; this.defaultCommand = options.defaultCommand; this.defaultModes = options.defaultModes; + this.newSessionStarter = options.newSessionStarter; this.modelTransformer = options.modelTransformer; this.sessionResponseTransformer = options.sessionResponseTransformer; this.configOptionsTransformer = options.configOptionsTransformer; @@ -1486,12 +1594,20 @@ export class ACPAgentSession implements AgentSession, ACPClient { this.connection = spawned.connection; this.agentCapabilities = spawned.initialize.agentCapabilities ?? null; - const response = await this.runACPRequest(() => - this.connection!.newSession({ - cwd: this.config.cwd, - mcpServers: this.acpMcpServers(), - }), - ); + const mcpServers = this.acpMcpServers(); + const response = this.newSessionStarter + ? await this.newSessionStarter({ + connection: spawned.connection, + config: this.config, + mcpServers, + runRequest: (request) => this.runACPRequest(request), + }) + : await this.runACPRequest(() => + this.connection!.newSession({ + cwd: this.config.cwd, + mcpServers, + }), + ); this.sessionId = response.sessionId; this.bootstrapThreadEventPending = true; this.applySessionState(response); diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.ts b/packages/server/src/server/agent/providers/generic-acp-agent.ts index f1eb0499d0..a9a53f2887 100644 --- a/packages/server/src/server/agent/providers/generic-acp-agent.ts +++ b/packages/server/src/server/agent/providers/generic-acp-agent.ts @@ -1,5 +1,6 @@ import type { Logger } from "pino"; import { z } from "zod"; +import type { SessionConfigOption } from "@agentclientprotocol/sdk"; import type { AgentCapabilityFlags } from "../agent-sdk-types.js"; import { checkProviderLaunchAvailable, resolveProviderLaunch } from "../provider-launch-config.js"; @@ -10,6 +11,9 @@ import { type ACPConfigFeatureOption, DEFAULT_ACP_CAPABILITIES, type ACPExtensionCommandsParser, + type ACPNewSessionStarter, + type ACPProbeSessionCloser, + type SessionStateResponse, } from "./acp-agent.js"; import { buildBinaryDiagnosticRows, @@ -52,6 +56,11 @@ interface GenericACPAgentClientOptions { configFeatureOptions?: ACPConfigFeatureOption[]; extensionCommandsParser?: ACPExtensionCommandsParser; catalogModelResolver?: ACPCatalogModelResolver; + newSessionStarter?: ACPNewSessionStarter; + probeSessionCloser?: ACPProbeSessionCloser; + sessionResponseTransformer?: (response: SessionStateResponse) => SessionStateResponse; + configOptionsTransformer?: (configOptions: SessionConfigOption[]) => SessionConfigOption[]; + modeIdTransformer?: (modeId: string) => string | null; } export class GenericACPAgentClient extends ACPAgentClient { @@ -93,6 +102,15 @@ export class GenericACPAgentClient extends ACPAgentClient { ...(options.catalogModelResolver ? { catalogModelResolver: options.catalogModelResolver } : {}), + ...(options.sessionResponseTransformer + ? { sessionResponseTransformer: options.sessionResponseTransformer } + : {}), + ...(options.configOptionsTransformer + ? { configOptionsTransformer: options.configOptionsTransformer } + : {}), + ...(options.modeIdTransformer ? { modeIdTransformer: options.modeIdTransformer } : {}), + ...(options.newSessionStarter ? { newSessionStarter: options.newSessionStarter } : {}), + ...(options.probeSessionCloser ? { probeSessionCloser: options.probeSessionCloser } : {}), }); this.command = options.command; diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts index dab84df374..b1a7e70b1a 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts @@ -1,5 +1,7 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; +import type { ClientSideConnection, LoadSessionResponse } from "@agentclientprotocol/sdk"; + import { createTestLogger } from "../../../test-utils/test-logger.js"; const mockState = vi.hoisted(() => ({ @@ -16,7 +18,16 @@ vi.mock("./generic-acp-agent.js", () => ({ }, })); -import { GjcACPAgentClient } from "./gjc-acp-agent.js"; +import { + buildGjcLifecycleCloseCommand, + buildGjcLifecycleCreateCommand, + createGjcACPNewSessionStarter, + createGjcACPProbeSessionCloser, + GjcACPAgentClient, + transformGjcConfigOptions, + transformGjcModeId, + transformGjcSessionResponse, +} from "./gjc-acp-agent.js"; describe("GjcACPAgentClient", () => { beforeEach(() => { @@ -58,7 +69,321 @@ describe("GjcACPAgentClient", () => { permissionHandling: "prompt", }, }, + sessionResponseTransformer: expect.any(Function), + configOptionsTransformer: expect.any(Function), + modeIdTransformer: expect.any(Function), + newSessionStarter: expect.any(Function), + probeSessionCloser: expect.any(Function), + }, + ]); + }); + + test("filters GJC host-lifecycle plan mode from ACP mode state", () => { + const transformed = transformGjcSessionResponse({ + sessionId: "session-1", + modes: { + currentModeId: "plan", + availableModes: [ + { id: "default", name: "Default" }, + { id: "plan", name: "Plan" }, + { + id: "https://agentclientprotocol.com/protocol/session-modes#plan", + name: "Plan", + }, + ], + }, + configOptions: [], + }); + + expect(transformed.modes).toEqual({ + currentModeId: "default", + availableModes: [{ id: "default", name: "Default" }], + }); + }); + + test("filters GJC host-lifecycle plan mode from config mode options", () => { + const transformed = transformGjcConfigOptions([ + { + id: "mode", + name: "Mode", + category: "mode", + type: "select", + currentValue: "plan", + options: [ + { value: "default", name: "Default" }, + { value: "plan", name: "Plan" }, + { + value: "https://agentclientprotocol.com/protocol/session-modes#plan", + name: "Plan", + }, + ], + }, + { + id: "thought_level", + name: "Thinking", + category: "thought_level", + type: "select", + currentValue: "xhigh", + options: [{ value: "xhigh", name: "Extra high" }], + }, + ]); + + expect(transformed).toEqual([ + { + id: "mode", + name: "Mode", + category: "mode", + type: "select", + currentValue: "default", + options: [{ value: "default", name: "Default" }], + }, + { + id: "thought_level", + name: "Thinking", + category: "thought_level", + type: "select", + currentValue: "xhigh", + options: [{ value: "xhigh", name: "Extra high" }], }, ]); }); + + test("maps unsupported GJC mode updates to null", () => { + expect(transformGjcModeId("plan")).toBeNull(); + expect(transformGjcModeId("default")).toBe("default"); + }); + + test("builds a lifecycle create command from a wrapped gjc acp command", () => { + const input = { + cwd: "/repo", + target: { + path: "/repo", + }, + readinessTimeoutMs: 60_000, + }; + + const command = buildGjcLifecycleCreateCommand(["bun", "x", "gjc", "acp"], "/repo", input); + + expect(command.command).toBe("bun"); + expect(command.args.slice(0, 6)).toEqual(["x", "gjc", "sdk", "session", "raw", "global"]); + expect(command.args).toContain("session.create"); + const jsonInputIndex = command.args.indexOf("--json-input"); + expect(JSON.parse(command.args[jsonInputIndex + 1]!)).toEqual(input); + expect(command.args.slice(-2)).toEqual(["--repo", "/repo"]); + }); + + test("builds a lifecycle close command from a wrapped gjc acp command", () => { + const command = buildGjcLifecycleCloseCommand( + ["bun", "x", "gjc", "acp"], + "/repo", + "gjc-session-1", + ); + + expect(command.command).toBe("bun"); + expect(command.args).toEqual([ + "x", + "gjc", + "sdk", + "session", + "raw", + "control", + "gjc-session-1", + "--op", + "session.close", + "--json-input", + "{}", + "--confirm", + "--json", + "--repo", + "/repo", + ]); + }); + + test("creates a gjc lifecycle session with extended readiness before loading ACP state", async () => { + const execFile = vi.fn(async () => ({ + stdout: JSON.stringify({ + type: "broker_response", + ok: true, + result: { + sessionId: "gjc-session-1", + endpoint: { + token: "secret-token", + }, + }, + }), + stderr: "", + })); + const loadResponse = {} as LoadSessionResponse; + const loadSession = vi.fn(async () => loadResponse); + const runRequest = vi.fn(async (request: () => Promise) => await request()); + const starter = createGjcACPNewSessionStarter({ + command: ["gjc", "acp"], + env: { + GJC_LOG: "debug", + }, + execFile, + }); + + const response = await starter({ + connection: { + loadSession, + } as unknown as ClientSideConnection, + config: { + provider: "gjc", + cwd: "/repo", + }, + mcpServers: [], + runRequest, + }); + + expect(response).toEqual({ + sessionId: "gjc-session-1", + }); + expect(execFile).toHaveBeenCalledWith( + "gjc", + expect.arrayContaining(["sdk", "session", "raw", "global"]), + expect.objectContaining({ + cwd: "/repo", + env: expect.objectContaining({ + GJC_LOG: "debug", + }), + timeout: 130_000, + maxBuffer: 1024 * 1024, + encoding: "utf8", + }), + ); + const args = execFile.mock.calls[0]![1]; + const jsonInput = JSON.parse(args[args.indexOf("--json-input") + 1]!); + expect(jsonInput).toEqual({ + cwd: "/repo", + target: { + path: "/repo", + }, + readinessTimeoutMs: 60_000, + }); + expect(loadSession).toHaveBeenCalledWith({ + sessionId: "gjc-session-1", + cwd: "/repo", + mcpServers: [], + }); + expect(runRequest).toHaveBeenCalledTimes(1); + }); + + test("closes a gjc lifecycle session when the ACP load step fails", async () => { + const execFile = vi + .fn() + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + ok: true, + result: { + sessionId: "gjc-session-1", + }, + }), + stderr: "", + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + ok: true, + result: { + closed: true, + }, + }), + stderr: "", + }); + const loadSession = vi.fn().mockRejectedValue(new Error("load failed")); + const runRequest = vi.fn(async (request: () => Promise) => await request()); + const starter = createGjcACPNewSessionStarter({ + command: ["gjc", "acp"], + execFile, + }); + + await expect( + starter({ + connection: { + loadSession, + } as unknown as ClientSideConnection, + config: { + provider: "gjc", + cwd: "/repo", + }, + mcpServers: [], + runRequest, + }), + ).rejects.toThrow("load failed"); + + expect(execFile).toHaveBeenCalledTimes(2); + expect(execFile.mock.calls[1]![1]).toEqual([ + "sdk", + "session", + "raw", + "control", + "gjc-session-1", + "--op", + "session.close", + "--json-input", + "{}", + "--confirm", + "--json", + "--repo", + "/repo", + ]); + }); + + test("closes a gjc probe lifecycle session after catalog use", async () => { + const execFile = vi.fn(async () => ({ + stdout: JSON.stringify({ + ok: true, + result: { + closed: true, + }, + }), + stderr: "", + })); + const closer = createGjcACPProbeSessionCloser({ + command: ["gjc", "acp"], + env: { + GJC_LOG: "debug", + }, + execFile, + }); + + await closer({ + response: { + sessionId: "gjc-session-1", + }, + config: { + provider: "gjc", + cwd: "/repo", + }, + mcpServers: [], + }); + + expect(execFile).toHaveBeenCalledWith( + "gjc", + [ + "sdk", + "session", + "raw", + "control", + "gjc-session-1", + "--op", + "session.close", + "--json-input", + "{}", + "--confirm", + "--json", + "--repo", + "/repo", + ], + expect.objectContaining({ + cwd: "/repo", + env: expect.objectContaining({ + GJC_LOG: "debug", + }), + timeout: 30_000, + maxBuffer: 1024 * 1024, + encoding: "utf8", + }), + ); + }); }); diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.ts index 6cee3fc2b3..09d6c3cba4 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.ts @@ -1,6 +1,21 @@ +import { execFile as execFileCallback } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { promisify } from "node:util"; + +import type { + McpServer, + SessionConfigOption, + SessionConfigSelectGroup, + SessionConfigSelectOption, +} from "@agentclientprotocol/sdk"; import type { Logger } from "pino"; -import type { ACPClientCapabilityMeta } from "./acp-agent.js"; +import type { + ACPClientCapabilityMeta, + ACPNewSessionStarter, + ACPProbeSessionCloser, + SessionStateResponse, +} from "./acp-agent.js"; import { GenericACPAgentClient } from "./generic-acp-agent.js"; interface GjcACPAgentClientOptions { @@ -12,6 +27,27 @@ interface GjcACPAgentClientOptions { providerParams?: unknown; } +interface GjcLifecycleCommand { + command: string; + args: string[]; +} + +interface GjcSessionCreateResult { + sessionId: string; +} + +type GjcExecFile = ( + file: string, + args: string[], + options: { + cwd: string; + env: NodeJS.ProcessEnv; + timeout: number; + maxBuffer: number; + encoding: BufferEncoding; + }, +) => Promise<{ stdout: string; stderr: string }>; + const GJC_CLIENT_CAPABILITIES = { terminal: true, }; @@ -22,6 +58,21 @@ const GJC_CLIENT_CAPABILITY_META = { }, } satisfies ACPClientCapabilityMeta; +const GJC_ACP_READINESS_TIMEOUT_MS = 60_000; +const GJC_ACP_RAW_CREATE_TIMEOUT_MS = 130_000; +const GJC_ACP_RAW_CLOSE_TIMEOUT_MS = 30_000; +const GJC_ACP_RAW_CREATE_MAX_BUFFER_BYTES = 1024 * 1024; +const GJC_DEFAULT_MODE_ID = "default"; +const GJC_UNSUPPORTED_HOST_LIFECYCLE_MODE_IDS = new Set([ + "plan", + "https://agentclientprotocol.com/protocol/session-modes#plan", +]); + +type SelectConfigOption = Extract; +type GjcModeOption = SessionConfigSelectGroup | SessionConfigSelectOption; + +const execFile = promisify(execFileCallback) as GjcExecFile; + export class GjcACPAgentClient extends GenericACPAgentClient { constructor(options: GjcACPAgentClientOptions) { super({ @@ -33,6 +84,382 @@ export class GjcACPAgentClient extends GenericACPAgentClient { providerParams: options.providerParams, clientCapabilities: GJC_CLIENT_CAPABILITIES, clientCapabilityMeta: GJC_CLIENT_CAPABILITY_META, + sessionResponseTransformer: transformGjcSessionResponse, + configOptionsTransformer: transformGjcConfigOptions, + modeIdTransformer: transformGjcModeId, + newSessionStarter: createGjcACPNewSessionStarter({ + command: options.command, + env: options.env, + }), + probeSessionCloser: createGjcACPProbeSessionCloser({ + command: options.command, + env: options.env, + }), + }); + } +} + +export function transformGjcSessionResponse(response: SessionStateResponse): SessionStateResponse { + if (!response.modes) { + return response; + } + const availableModes = response.modes.availableModes.filter( + (mode) => !isGjcUnsupportedHostLifecycleMode(mode.id), + ); + return { + ...response, + modes: { + ...response.modes, + availableModes, + currentModeId: transformGjcModeId(response.modes.currentModeId) ?? GJC_DEFAULT_MODE_ID, + }, + }; +} + +export function transformGjcConfigOptions( + configOptions: SessionConfigOption[], +): SessionConfigOption[] { + return configOptions.flatMap((option) => { + if (option.type !== "select" || option.category !== "mode") { + return [option]; + } + const options = filterGjcModeOptions(option.options); + const currentValue = transformGjcModeId(option.currentValue) ?? firstModeOptionValue(options); + if (!currentValue) { + return []; + } + return { + ...option, + options, + currentValue, + }; + }); +} + +export function transformGjcModeId(modeId: string): string | null { + return isGjcUnsupportedHostLifecycleMode(modeId) ? null : modeId; +} + +export function createGjcACPNewSessionStarter(options: { + command: [string, ...string[]]; + env?: Record; + execFile?: GjcExecFile; +}): ACPNewSessionStarter { + const runExecFile = options.execFile ?? execFile; + + return async ({ connection, config, mcpServers, runRequest }) => { + const lifecycleCommand = buildGjcLifecycleCreateCommand(options.command, config.cwd, { + cwd: config.cwd, + target: { + path: config.cwd, + }, + readinessTimeoutMs: GJC_ACP_READINESS_TIMEOUT_MS, + ...(mcpServers.length > 0 ? { mcpServers } : {}), + }); + + let createResult: GjcSessionCreateResult; + try { + const { stdout } = await runExecFile(lifecycleCommand.command, lifecycleCommand.args, { + cwd: config.cwd, + env: { + ...process.env, + ...options.env, + }, + timeout: GJC_ACP_RAW_CREATE_TIMEOUT_MS, + maxBuffer: GJC_ACP_RAW_CREATE_MAX_BUFFER_BYTES, + encoding: "utf8", + }); + createResult = extractGjcSessionCreateResult(parseGjcJsonOutput(stdout)); + } catch (error) { + throw new Error(`GJC lifecycle session.create failed: ${formatGjcExecError(error)}`, { + cause: error, + }); + } + + let sessionState: SessionStateResponse; + try { + sessionState = await runRequest(() => + connection.loadSession({ + sessionId: createResult.sessionId, + cwd: config.cwd, + mcpServers, + }), + ); + } catch (error) { + await closeGjcLifecycleSession({ + command: options.command, + env: options.env, + execFile: runExecFile, + cwd: config.cwd, + sessionId: createResult.sessionId, + }).catch(() => undefined); + throw error; + } + return { + ...sessionState, + sessionId: createResult.sessionId, + }; + }; +} + +export function createGjcACPProbeSessionCloser(options: { + command: [string, ...string[]]; + env?: Record; + execFile?: GjcExecFile; +}): ACPProbeSessionCloser { + const runExecFile = options.execFile ?? execFile; + + return async ({ response, config }) => { + const sessionId = getSessionStateResponseId(response); + if (!sessionId) { + throw new Error("GJC probe session did not expose a session id"); + } + await closeGjcLifecycleSession({ + command: options.command, + env: options.env, + execFile: runExecFile, + cwd: config.cwd, + sessionId, + }); + }; +} + +export function buildGjcLifecycleCreateCommand( + acpCommand: [string, ...string[]], + cwd: string, + input: { + cwd: string; + target: { path: string }; + readinessTimeoutMs: number; + mcpServers?: McpServer[]; + }, +): GjcLifecycleCommand { + return buildGjcLifecycleCommand(acpCommand, [ + "sdk", + "session", + "raw", + "global", + "--op", + "session.create", + "--json-input", + JSON.stringify(input), + "--idempotency-key", + randomUUID(), + "--json", + "--repo", + cwd, + ]); +} + +export function buildGjcLifecycleCloseCommand( + acpCommand: [string, ...string[]], + cwd: string, + sessionId: string, +): GjcLifecycleCommand { + return buildGjcLifecycleCommand(acpCommand, [ + "sdk", + "session", + "raw", + "control", + sessionId, + "--op", + "session.close", + "--json-input", + "{}", + "--confirm", + "--json", + "--repo", + cwd, + ]); +} + +async function closeGjcLifecycleSession(options: { + command: [string, ...string[]]; + env?: Record; + execFile: GjcExecFile; + cwd: string; + sessionId: string; +}): Promise { + const lifecycleCommand = buildGjcLifecycleCloseCommand( + options.command, + options.cwd, + options.sessionId, + ); + try { + const { stdout } = await options.execFile(lifecycleCommand.command, lifecycleCommand.args, { + cwd: options.cwd, + env: { + ...process.env, + ...options.env, + }, + timeout: GJC_ACP_RAW_CLOSE_TIMEOUT_MS, + maxBuffer: GJC_ACP_RAW_CREATE_MAX_BUFFER_BYTES, + encoding: "utf8", + }); + assertGjcLifecycleCommandSucceeded(stdout); + } catch (error) { + throw new Error(`GJC lifecycle session.close failed: ${formatGjcExecError(error)}`, { + cause: error, }); } } + +function isGjcUnsupportedHostLifecycleMode(modeId: string): boolean { + return GJC_UNSUPPORTED_HOST_LIFECYCLE_MODE_IDS.has(modeId); +} + +function filterGjcModeOptions( + options: SelectConfigOption["options"], +): SelectConfigOption["options"] { + const filtered: GjcModeOption[] = []; + for (const option of options as GjcModeOption[]) { + if ("value" in option) { + if (!isGjcUnsupportedHostLifecycleMode(option.value)) { + filtered.push(option); + } + continue; + } + const groupOptions = option.options.filter( + (choice) => !isGjcUnsupportedHostLifecycleMode(choice.value), + ); + if (groupOptions.length > 0) { + filtered.push({ ...option, options: groupOptions }); + } + } + return filtered as SelectConfigOption["options"]; +} + +function firstModeOptionValue(options: SelectConfigOption["options"]): string | null { + for (const option of options as GjcModeOption[]) { + if ("value" in option) { + return option.value; + } + const firstGroupOption = option.options[0]; + if (firstGroupOption) { + return firstGroupOption.value; + } + } + return null; +} + +function buildGjcLifecycleCommand( + acpCommand: [string, ...string[]], + lifecycleArgs: string[], +): GjcLifecycleCommand { + const acpArgs = acpCommand.slice(1); + const acpArgIndex = acpArgs.findIndex((arg) => arg === "acp"); + const prefixArgs = acpArgIndex === -1 ? acpArgs : acpArgs.slice(0, acpArgIndex); + return { + command: acpCommand[0], + args: [...prefixArgs, ...lifecycleArgs], + }; +} + +function parseGjcJsonOutput(stdout: string): unknown { + const trimmed = stdout.trim(); + if (!trimmed) { + throw new Error("empty JSON response"); + } + try { + return JSON.parse(trimmed); + } catch { + const jsonLine = trimmed + .split(/\r?\n/) + .toReversed() + .find((line) => line.trim().startsWith("{")); + if (!jsonLine) { + throw new Error("non-JSON response"); + } + return JSON.parse(jsonLine); + } +} + +function extractGjcSessionCreateResult(value: unknown): GjcSessionCreateResult { + if (isRecord(value) && value.ok === false) { + throw new Error(formatGjcBrokerError(value)); + } + + const result = isRecord(value) && isRecord(value.result) ? value.result : value; + if (isRecord(result) && result.ok === false) { + throw new Error(formatGjcBrokerError(result)); + } + + const nestedResult = isRecord(result) && isRecord(result.result) ? result.result : result; + if (isRecord(nestedResult) && typeof nestedResult.sessionId === "string") { + return { sessionId: nestedResult.sessionId }; + } + + throw new Error("missing session id"); +} + +function assertGjcLifecycleCommandSucceeded(stdout: string): void { + if (!stdout.trim()) { + return; + } + const value = parseGjcJsonOutput(stdout); + if (isRecord(value) && value.ok === false) { + throw new Error(formatGjcBrokerError(value)); + } + + const result = isRecord(value) && isRecord(value.result) ? value.result : value; + if (isRecord(result) && result.ok === false) { + throw new Error(formatGjcBrokerError(result)); + } +} + +function getSessionStateResponseId(response: SessionStateResponse): string | null { + return "sessionId" in response && typeof response.sessionId === "string" + ? response.sessionId + : null; +} + +function formatGjcBrokerError(value: Record): string { + const error = value.error; + if (isRecord(error)) { + const code = typeof error.code === "string" ? error.code : null; + const message = typeof error.message === "string" ? error.message : null; + return sanitizeGjcDiagnostic([code, message].filter(Boolean).join(": ")); + } + return "broker returned an error"; +} + +function formatGjcExecError(error: unknown): string { + if (error instanceof Error) { + const stdoutMessage = isRecord(error) ? extractGjcStdoutError(error.stdout) : null; + if (stdoutMessage) { + return stdoutMessage; + } + return sanitizeGjcDiagnostic(error.message); + } + return sanitizeGjcDiagnostic(error); +} + +function extractGjcStdoutError(stdout: unknown): string | null { + if (typeof stdout !== "string" || !stdout.trim()) { + return null; + } + try { + const parsed = parseGjcJsonOutput(stdout); + if (isRecord(parsed) && parsed.ok === false) { + return formatGjcBrokerError(parsed); + } + if (isRecord(parsed) && isRecord(parsed.result) && parsed.result.ok === false) { + return formatGjcBrokerError(parsed.result); + } + } catch { + return null; + } + return null; +} + +function sanitizeGjcDiagnostic(value: unknown): string { + const text = typeof value === "string" ? value : JSON.stringify(value); + return (text || "unknown error") + .replace(/("token"\s*:\s*")[^"]+(")/gi, "$1[redacted]$2") + .replace(/(token=)[^\s&]+/gi, "$1[redacted]") + .replace(/(authorization:\s*bearer\s+)[^\s]+/gi, "$1[redacted]"); +} + +function isRecord(value: unknown): value is Record { + return value != null && typeof value === "object" && !Array.isArray(value); +} From 47ad860fc5d8c5a5bbbd4d71930f3bd35224eebe Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 19:53:59 +0900 Subject: [PATCH 04/15] Fix ACP probe cleanup after refresh abort --- .../server/agent/providers/acp-agent.test.ts | 176 +++++++++ .../src/server/agent/providers/acp-agent.ts | 87 ++++- .../agent/providers/gjc-acp-agent.test.ts | 356 ++++++++++++++---- .../server/agent/providers/gjc-acp-agent.ts | 74 +++- 4 files changed, 591 insertions(+), 102 deletions(-) diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index 11f1797a28..ac3e15aed8 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -30,6 +30,7 @@ import { summarizeACPRequestError, } from "./acp-agent.js"; import type { ProcessTerminator, TreeKillTarget } from "../../../utils/tree-kill.js"; +import type { ProviderRefreshContext } from "../agent-sdk-types.js"; import { COPILOT_AGENT_FEATURE_OPTION, COPILOT_ALLOW_ALL_MODE_ID, @@ -195,6 +196,25 @@ class FakeTerminator { } } +interface Deferred { + promise: Promise; + resolve: (value: T | PromiseLike) => void; + reject: (reason?: unknown) => void; +} + +function createDeferred(): Deferred { + let resolve: Deferred["resolve"] | null = null; + let reject: Deferred["reject"] | null = null; + const promise = new Promise((innerResolve, innerReject) => { + resolve = innerResolve; + reject = innerReject; + }); + if (!resolve || !reject) { + throw new Error("Deferred promise executor did not initialize"); + } + return { promise, resolve, reject }; +} + function createSessionWithConfig( config: { provider?: string; @@ -2249,6 +2269,84 @@ describe("ACPAgentClient fetchCatalog", () => { }); }); + test("closes custom catalog probe sessions that finish after refresh abort", async () => { + const started = createDeferred(); + const session = createDeferred(); + const lateResponse: SessionStateResponse = { + sessionId: "late-probe-session", + modes: null, + models: null, + configOptions: [], + }; + const newSessionStarter = vi.fn(() => { + started.resolve(undefined); + return session.promise; + }); + const probeSessionCloser = vi.fn(async () => undefined); + + class TestACPAgentClient extends ACPAgentClient { + protected override async spawnProcess(): Promise { + return { + child: { kill: vi.fn(), exitCode: 0, signalCode: null, once: vi.fn() }, + connection: { + newSession: vi.fn().mockRejectedValue(new Error("newSession should not be called")), + } as unknown as ClientSideConnection, + initialize: { agentCapabilities: {} }, + } as SpawnedACPProcess; + } + + protected override async closeProbe(): Promise {} + } + + const client = new TestACPAgentClient({ + provider: "gjc", + logger: createTestLogger(), + defaultCommand: ["gjc", "acp"], + defaultModes: [], + newSessionStarter, + probeSessionCloser, + }); + const controller = new AbortController(); + const refreshContext: ProviderRefreshContext = { + signal: controller.signal, + runActivity: async (_name, operation) => await operation(), + }; + + const refresh = client.fetchCatalog( + { scope: "workspace", cwd: "/tmp/acp-catalog-cwd", force: false }, + refreshContext, + ); + await started.promise; + + let refreshSettled = false; + void refresh.then( + () => { + refreshSettled = true; + return undefined; + }, + () => { + refreshSettled = true; + return undefined; + }, + ); + controller.abort(new Error("refresh aborted")); + await Promise.resolve(); + expect(refreshSettled).toBe(false); + expect(probeSessionCloser).not.toHaveBeenCalled(); + + session.resolve(lateResponse); + await expect(refresh).rejects.toThrow("refresh aborted"); + + expect(probeSessionCloser).toHaveBeenCalledWith({ + response: lateResponse, + config: { + provider: "gjc", + cwd: "/tmp/acp-catalog-cwd", + }, + mcpServers: [], + }); + }); + test("returns an empty modes array when no ACP modes are reported and fallback modes are empty", async () => { class TestACPAgentClient extends ACPAgentClient { protected override async spawnProcess(): Promise { @@ -2296,6 +2394,84 @@ describe("ACPAgentClient fetchCatalog", () => { }); }); +describe("ACPAgentClient probe diagnostics", () => { + test("closes custom probe sessions that finish after diagnostic session timeout", async () => { + vi.useFakeTimers(); + try { + const started = createDeferred(); + const session = createDeferred(); + const lateResponse: SessionStateResponse = { + sessionId: "late-diagnostic-session", + modes: null, + models: null, + configOptions: [], + }; + const newSessionStarter = vi.fn(() => { + started.resolve(undefined); + return session.promise; + }); + const probeSessionCloser = vi.fn(async () => undefined); + const terminator = new FakeTerminator(); + + class TestACPAgentClient extends ACPAgentClient { + async buildDiagnosticRows() { + return await this.buildACPProbeDiagnosticRows({ + cwd: "/tmp/acp-diagnostic-cwd", + phaseTimeoutMs: 1, + }); + } + + protected override async spawnTransport() { + return { + child: createProbeChildStub(), + connection: { + initialize: vi.fn(async () => ({ agentCapabilities: {} })), + } as unknown as ClientSideConnection, + stderrChunks: [], + spawnReady: Promise.resolve(), + spawnError: new Promise(() => undefined), + }; + } + } + + const client = new TestACPAgentClient({ + provider: "gjc", + logger: createTestLogger(), + defaultCommand: ["gjc", "acp"], + defaultModes: [], + newSessionStarter, + probeSessionCloser, + terminateProcess: terminator.terminate, + }); + + const rowsPromise = client.buildDiagnosticRows(); + await started.promise; + await vi.advanceTimersByTimeAsync(1); + + expect(probeSessionCloser).not.toHaveBeenCalled(); + + session.resolve(lateResponse); + const rows = await rowsPromise; + + expect(rows).toContainEqual({ + label: "ACP session/new", + value: "error: ACP session/new timed out after 1ms", + }); + expect(probeSessionCloser).toHaveBeenCalledWith({ + response: lateResponse, + config: { + provider: "gjc", + cwd: "/tmp/acp-diagnostic-cwd", + }, + mcpServers: [], + }); + expect(terminator.terminated).toHaveLength(1); + } finally { + vi.useRealTimers(); + } + }); +}); + describe("ACPAgentClient listImportableSessions", () => { function makeClient(args: { listSessions: ReturnType; supportsList?: boolean }) { class TestACPAgentClient extends ACPAgentClient { diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 83f9b5a060..497078f7c3 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -511,6 +511,11 @@ interface ACPProcessTransport { spawnError: Promise; } +interface TrackedACPProbeSession { + promise: Promise; + close: () => Promise; +} + export interface ACPToolSnapshot { toolCallId: string; title: string; @@ -982,6 +987,7 @@ export class ACPAgentClient implements AgentClient { const config: AgentSessionConfig = { provider: this.provider, cwd }; const mcpServers: McpServer[] = []; let response: SessionStateResponse | null = null; + let probeSession: TrackedACPProbeSession | null = null; try { const initializedProbe = await runProviderRefreshActivity(context, "initialize", () => @@ -996,15 +1002,14 @@ export class ACPAgentClient implements AgentClient { ), ); probe = initializedProbe; + const activeProbeSession = this.startTrackedProbeSession({ + connection: initializedProbe.connection, + config, + mcpServers, + }); + probeSession = activeProbeSession; response = await runProviderRefreshActivity(context, "session/new", () => - raceProviderRefreshAbort( - context?.signal, - this.startProbeSession({ - connection: initializedProbe.connection, - config, - mcpServers, - }), - ), + raceProviderRefreshAbort(context?.signal, activeProbeSession.promise), ); const transformed = this.transformSessionResponse(response); const derivedModels = deriveModelDefinitionsFromACP( @@ -1048,9 +1053,7 @@ export class ACPAgentClient implements AgentClient { }; } finally { context?.signal.removeEventListener("abort", handleAbort); - if (response) { - await this.closeProbeSession({ response, config, mcpServers }); - } + await probeSession?.close(); await closeProbe(); } } @@ -1348,13 +1351,15 @@ export class ACPAgentClient implements AgentClient { const config: AgentSessionConfig = { provider: this.provider, cwd }; const mcpServers: McpServer[] = []; let response: SessionStateResponse | null = null; + let probeSession: TrackedACPProbeSession | null = null; try { + probeSession = this.startTrackedProbeSession({ + connection: activeTransport.connection, + config, + mcpServers, + }); response = await withTimeout( - this.startProbeSession({ - connection: activeTransport.connection, - config, - mcpServers, - }), + probeSession.promise, phaseTimeoutMs, `ACP session/new timed out after ${phaseTimeoutMs}ms`, ); @@ -1383,9 +1388,7 @@ export class ACPAgentClient implements AgentClient { pushACPStderrRow(rows, activeTransport.stderrChunks); return rows; } finally { - if (response) { - await this.closeProbeSession({ response, config, mcpServers }); - } + await probeSession?.close(); } pushACPStderrRow(rows, activeTransport.stderrChunks); @@ -1463,6 +1466,52 @@ export class ACPAgentClient implements AgentClient { ); } + private startTrackedProbeSession(context: { + connection: ClientSideConnection; + config: AgentSessionConfig; + mcpServers: McpServer[]; + }): TrackedACPProbeSession { + let response: SessionStateResponse | null = null; + let closeRequested = false; + let closePromise: Promise | null = null; + + const closeResponse = (sessionResponse: SessionStateResponse): Promise => { + closePromise ??= this.closeProbeSession({ + response: sessionResponse, + config: context.config, + mcpServers: context.mcpServers, + }); + return closePromise; + }; + + const promise = this.startProbeSession(context).then((sessionResponse) => { + response = sessionResponse; + if (closeRequested) { + void closeResponse(sessionResponse); + } + return sessionResponse; + }); + + return { + promise, + close: async () => { + closeRequested = true; + if (!this.probeSessionCloser) { + return; + } + if (response) { + await closeResponse(response); + return; + } + try { + await closeResponse(await promise); + } catch { + // If session startup failed, there is no provider-owned probe session to close. + } + }, + }; + } + private async closeProbeSession(context: ACPProbeSessionCloserContext): Promise { if (!this.probeSessionCloser) { return; diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts index b1a7e70b1a..d53198e85c 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts @@ -1,23 +1,15 @@ -import { beforeEach, describe, expect, test, vi } from "vitest"; +import type { ChildProcessWithoutNullStreams } from "node:child_process"; +import { access, readFile, stat } from "node:fs/promises"; +import { describe, expect, test, vi } from "vitest"; -import type { ClientSideConnection, LoadSessionResponse } from "@agentclientprotocol/sdk"; +import type { + ClientSideConnection, + LoadSessionResponse, + McpServer, +} from "@agentclientprotocol/sdk"; import { createTestLogger } from "../../../test-utils/test-logger.js"; -const mockState = vi.hoisted(() => ({ - genericConstructorOptions: [] as unknown[], -})); - -vi.mock("./generic-acp-agent.js", () => ({ - GenericACPAgentClient: class GenericACPAgentClient { - readonly provider = "acp"; - - constructor(options: unknown) { - mockState.genericConstructorOptions.push(options); - } - }, -})); - import { buildGjcLifecycleCloseCommand, buildGjcLifecycleCreateCommand, @@ -30,12 +22,79 @@ import { } from "./gjc-acp-agent.js"; describe("GjcACPAgentClient", () => { - beforeEach(() => { - mockState.genericConstructorOptions = []; - }); + test("uses terminal delegation and prompt permission handling during catalog discovery", async () => { + const initialize = vi.fn(async () => ({ agentCapabilities: {} })); + const loadSession = vi.fn( + async (): Promise => ({ + sessionId: "loaded-session", + modes: { + currentModeId: "plan", + availableModes: [ + { id: "default", name: "Default" }, + { id: "plan", name: "Plan" }, + ], + }, + models: { + currentModelId: "openai-codex/gpt-5.5", + availableModels: [ + { + modelId: "openai-codex/gpt-5.5", + name: "GPT-5.5", + description: "GJC model", + }, + ], + }, + configOptions: [], + }), + ); + const execFile = vi.fn(async (_file: string, args: string[]) => { + if (args.includes("session.create")) { + return { + stdout: JSON.stringify({ + ok: true, + result: { + sessionId: "gjc-session-1", + }, + }), + stderr: "", + }; + } + if (args.includes("session.close")) { + return { + stdout: JSON.stringify({ + ok: true, + result: { + closed: true, + }, + }), + stderr: "", + }; + } + throw new Error(`Unexpected GJC command: ${args.join(" ")}`); + }); + + class TestGjcACPAgentClient extends GjcACPAgentClient { + protected override async spawnTransport() { + return { + child: { + kill: vi.fn(), + exitCode: 0, + signalCode: null, + } as unknown as ChildProcessWithoutNullStreams, + connection: { + initialize, + loadSession, + } as unknown as ClientSideConnection, + stderrChunks: [], + spawnReady: Promise.resolve(), + spawnError: new Promise(() => undefined), + }; + } - test("enables Paseo terminal execution and prompt permission handling for GJC ACP", () => { - const _client = new GjcACPAgentClient({ + protected override async closeProbe(): Promise {} + } + + const client = new TestGjcACPAgentClient({ logger: createTestLogger(), command: ["gjc", "acp"], env: { @@ -46,36 +105,177 @@ describe("GjcACPAgentClient", () => { providerParams: { supportsMcpServers: false, }, + execFile, }); - void _client; - expect(mockState.genericConstructorOptions).toEqual([ - { - logger: expect.any(Object), - command: ["gjc", "acp"], - env: { - GJC_LOG: "debug", + await expect( + client.fetchCatalog({ scope: "workspace", cwd: "/repo", force: false }), + ).resolves.toEqual({ + models: [ + { + provider: "acp", + id: "openai-codex/gpt-5.5", + label: "GPT-5.5", + description: "GJC model", + isDefault: true, + thinkingOptions: undefined, + defaultThinkingOptionId: undefined, }, - providerId: "gjc", - label: "Gajae Code", - providerParams: { - supportsMcpServers: false, + ], + modes: [ + { + id: "default", + label: "Default", + description: undefined, }, + ], + }); + + expect(initialize).toHaveBeenCalledWith( + expect.objectContaining({ clientCapabilities: { + fs: { + readTextFile: false, + writeTextFile: false, + }, terminal: true, - }, - clientCapabilityMeta: { - gjc: { - permissionHandling: "prompt", + _meta: { + gjc: { + permissionHandling: "prompt", + }, }, }, - sessionResponseTransformer: expect.any(Function), - configOptionsTransformer: expect.any(Function), - modeIdTransformer: expect.any(Function), - newSessionStarter: expect.any(Function), - probeSessionCloser: expect.any(Function), - }, + }), + ); + expect(loadSession).toHaveBeenCalledWith({ + sessionId: "gjc-session-1", + cwd: "/repo", + mcpServers: [], + }); + expect(execFile).toHaveBeenCalledTimes(2); + expect(execFile.mock.calls[0]![1]).toEqual( + expect.arrayContaining(["sdk", "session", "raw", "global", "--op", "session.create"]), + ); + expect(execFile.mock.calls[1]![1]).toEqual([ + "sdk", + "session", + "raw", + "control", + "gjc-session-1", + "--op", + "session.close", + "--json-input", + "{}", + "--confirm", + "--json", + "--repo", + "/repo", ]); + expect(execFile.mock.calls[0]![2]).toEqual( + expect.objectContaining({ + cwd: "/repo", + env: expect.objectContaining({ + GJC_LOG: "debug", + }), + }), + ); + }); + + test("uses the lifecycle readiness budget for diagnostic session probes", async () => { + vi.useFakeTimers(); + try { + let markStarted: () => void = () => undefined; + const started = new Promise((resolve) => { + markStarted = resolve; + }); + let resolveCreate: (value: { stdout: string; stderr: string }) => void = () => undefined; + const createResult = new Promise<{ stdout: string; stderr: string }>((resolve) => { + resolveCreate = resolve; + }); + const execFile = vi.fn(async (_file: string, args: string[]) => { + if (args.includes("session.create")) { + markStarted(); + return await createResult; + } + if (args.includes("session.close")) { + return { + stdout: JSON.stringify({ + ok: true, + result: { + closed: true, + }, + }), + stderr: "", + }; + } + throw new Error(`Unexpected GJC command: ${args.join(" ")}`); + }); + + class TestGjcACPAgentClient extends GjcACPAgentClient { + protected override async spawnTransport() { + return { + child: { + kill: vi.fn(), + exitCode: 0, + signalCode: null, + } as unknown as ChildProcessWithoutNullStreams, + connection: { + initialize: vi.fn(async () => ({ agentCapabilities: {} })), + loadSession: vi.fn(async () => ({ + sessionId: "gjc-session-1", + configOptions: [], + })), + } as unknown as ClientSideConnection, + stderrChunks: [], + spawnReady: Promise.resolve(), + spawnError: new Promise(() => undefined), + }; + } + } + + const client = new TestGjcACPAgentClient({ + logger: createTestLogger(), + command: ["gjc-test", "acp"], + execFile, + }); + + const diagnostic = client.getDiagnostic(); + let settled = false; + void diagnostic.then( + () => { + settled = true; + return undefined; + }, + () => { + settled = true; + return undefined; + }, + ); + await started; + await Promise.resolve(); + + await vi.advanceTimersByTimeAsync(20_000); + expect(settled).toBe(false); + + await vi.advanceTimersByTimeAsync(40_000); + resolveCreate({ + stdout: JSON.stringify({ + ok: true, + result: { + sessionId: "gjc-session-1", + }, + }), + stderr: "", + }); + await expect(diagnostic).resolves.toEqual({ + diagnostic: expect.stringContaining( + "ACP session/new: error: ACP session/new timed out after 60000ms", + ), + }); + expect(execFile).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } }); test("filters GJC host-lifecycle plan mode from ACP mode state", () => { @@ -199,23 +399,43 @@ describe("GjcACPAgentClient", () => { ]); }); - test("creates a gjc lifecycle session with extended readiness before loading ACP state", async () => { - const execFile = vi.fn(async () => ({ - stdout: JSON.stringify({ - type: "broker_response", - ok: true, - result: { - sessionId: "gjc-session-1", - endpoint: { - token: "secret-token", + test("creates a gjc lifecycle session from a private input file before loading ACP state", async () => { + const lifecycleInputs: unknown[] = []; + let lifecycleInputFilePath: string | null = null; + const execFile = vi.fn(async (_file: string, args: string[]) => { + const jsonInputFileIndex = args.indexOf("--json-input-file"); + expect(jsonInputFileIndex).toBeGreaterThan(-1); + lifecycleInputFilePath = args[jsonInputFileIndex + 1] ?? null; + if (!lifecycleInputFilePath) { + throw new Error("Expected GJC lifecycle input file path"); + } + expect((await stat(lifecycleInputFilePath)).mode & 0o777).toBe(0o600); + lifecycleInputs.push(JSON.parse(await readFile(lifecycleInputFilePath, "utf8"))); + return { + stdout: JSON.stringify({ + type: "broker_response", + ok: true, + result: { + sessionId: "gjc-session-1", + endpoint: { + token: "endpoint-secret", + }, }, - }, - }), - stderr: "", - })); + }), + stderr: "", + }; + }); const loadResponse = {} as LoadSessionResponse; const loadSession = vi.fn(async () => loadResponse); const runRequest = vi.fn(async (request: () => Promise) => await request()); + const mcpServers: McpServer[] = [ + { + type: "http", + name: "hub", + url: "https://hub.test/mcp", + headers: [{ name: "Authorization", value: "Bearer mcp-secret-token" }], + }, + ]; const starter = createGjcACPNewSessionStarter({ command: ["gjc", "acp"], env: { @@ -232,7 +452,7 @@ describe("GjcACPAgentClient", () => { provider: "gjc", cwd: "/repo", }, - mcpServers: [], + mcpServers, runRequest, }); @@ -253,18 +473,26 @@ describe("GjcACPAgentClient", () => { }), ); const args = execFile.mock.calls[0]![1]; - const jsonInput = JSON.parse(args[args.indexOf("--json-input") + 1]!); - expect(jsonInput).toEqual({ - cwd: "/repo", - target: { - path: "/repo", + expect(args).not.toContain("--json-input"); + expect(args.join(" ")).not.toContain("mcp-secret-token"); + expect(lifecycleInputs).toEqual([ + { + cwd: "/repo", + target: { + path: "/repo", + }, + readinessTimeoutMs: 60_000, + mcpServers, }, - readinessTimeoutMs: 60_000, - }); + ]); + if (!lifecycleInputFilePath) { + throw new Error("Expected GJC lifecycle input file path"); + } + await expect(access(lifecycleInputFilePath)).rejects.toThrow(); expect(loadSession).toHaveBeenCalledWith({ sessionId: "gjc-session-1", cwd: "/repo", - mcpServers: [], + mcpServers, }); expect(runRequest).toHaveBeenCalledTimes(1); }); diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.ts index 09d6c3cba4..9676934d8d 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.ts @@ -1,5 +1,8 @@ import { execFile as execFileCallback } from "node:child_process"; import { randomUUID } from "node:crypto"; +import { chmod, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { promisify } from "node:util"; import type { @@ -25,6 +28,7 @@ interface GjcACPAgentClientOptions { providerId?: string; label?: string; providerParams?: unknown; + execFile?: GjcExecFile; } interface GjcLifecycleCommand { @@ -32,6 +36,13 @@ interface GjcLifecycleCommand { args: string[]; } +interface GjcLifecycleCreateInput { + cwd: string; + target: { path: string }; + readinessTimeoutMs: number; + mcpServers?: McpServer[]; +} + interface GjcSessionCreateResult { sessionId: string; } @@ -84,16 +95,19 @@ export class GjcACPAgentClient extends GenericACPAgentClient { providerParams: options.providerParams, clientCapabilities: GJC_CLIENT_CAPABILITIES, clientCapabilityMeta: GJC_CLIENT_CAPABILITY_META, + diagnosticPhaseTimeoutMs: GJC_ACP_READINESS_TIMEOUT_MS, sessionResponseTransformer: transformGjcSessionResponse, configOptionsTransformer: transformGjcConfigOptions, modeIdTransformer: transformGjcModeId, newSessionStarter: createGjcACPNewSessionStarter({ command: options.command, env: options.env, + execFile: options.execFile, }), probeSessionCloser: createGjcACPProbeSessionCloser({ command: options.command, env: options.env, + execFile: options.execFile, }), }); } @@ -148,26 +162,34 @@ export function createGjcACPNewSessionStarter(options: { const runExecFile = options.execFile ?? execFile; return async ({ connection, config, mcpServers, runRequest }) => { - const lifecycleCommand = buildGjcLifecycleCreateCommand(options.command, config.cwd, { + const lifecycleInput: GjcLifecycleCreateInput = { cwd: config.cwd, target: { path: config.cwd, }, readinessTimeoutMs: GJC_ACP_READINESS_TIMEOUT_MS, ...(mcpServers.length > 0 ? { mcpServers } : {}), - }); + }; let createResult: GjcSessionCreateResult; try { - const { stdout } = await runExecFile(lifecycleCommand.command, lifecycleCommand.args, { - cwd: config.cwd, - env: { - ...process.env, - ...options.env, - }, - timeout: GJC_ACP_RAW_CREATE_TIMEOUT_MS, - maxBuffer: GJC_ACP_RAW_CREATE_MAX_BUFFER_BYTES, - encoding: "utf8", + const { stdout } = await withGjcJsonInputFile(lifecycleInput, async (inputFilePath) => { + const lifecycleCommand = buildGjcLifecycleCreateCommand( + options.command, + config.cwd, + lifecycleInput, + { inputFilePath }, + ); + return await runExecFile(lifecycleCommand.command, lifecycleCommand.args, { + cwd: config.cwd, + env: { + ...process.env, + ...options.env, + }, + timeout: GJC_ACP_RAW_CREATE_TIMEOUT_MS, + maxBuffer: GJC_ACP_RAW_CREATE_MAX_BUFFER_BYTES, + encoding: "utf8", + }); }); createResult = extractGjcSessionCreateResult(parseGjcJsonOutput(stdout)); } catch (error) { @@ -227,13 +249,13 @@ export function createGjcACPProbeSessionCloser(options: { export function buildGjcLifecycleCreateCommand( acpCommand: [string, ...string[]], cwd: string, - input: { - cwd: string; - target: { path: string }; - readinessTimeoutMs: number; - mcpServers?: McpServer[]; - }, + input: GjcLifecycleCreateInput, + options: { inputFilePath?: string } = {}, ): GjcLifecycleCommand { + const jsonInputArgs = options.inputFilePath + ? ["--json-input-file", options.inputFilePath] + : ["--json-input", JSON.stringify(input)]; + return buildGjcLifecycleCommand(acpCommand, [ "sdk", "session", @@ -241,8 +263,7 @@ export function buildGjcLifecycleCreateCommand( "global", "--op", "session.create", - "--json-input", - JSON.stringify(input), + ...jsonInputArgs, "--idempotency-key", randomUUID(), "--json", @@ -273,6 +294,21 @@ export function buildGjcLifecycleCloseCommand( ]); } +async function withGjcJsonInputFile( + input: GjcLifecycleCreateInput, + operation: (inputFilePath: string) => Promise, +): Promise { + const directory = await mkdtemp(join(tmpdir(), "paseo-gjc-json-")); + const inputFilePath = join(directory, "input.json"); + try { + await writeFile(inputFilePath, JSON.stringify(input), { mode: 0o600 }); + await chmod(inputFilePath, 0o600); + return await operation(inputFilePath); + } finally { + await rm(directory, { recursive: true, force: true }); + } +} + async function closeGjcLifecycleSession(options: { command: [string, ...string[]]; env?: Record; From 8e630a77d10e990442c54e67b178cd3f5ccdcb32 Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 20:12:49 +0900 Subject: [PATCH 05/15] Address remaining ACP probe review comments --- .../server/agent/providers/acp-agent.test.ts | 77 +++++++++++++++++-- .../src/server/agent/providers/acp-agent.ts | 35 ++++++++- .../agent/providers/generic-acp-agent.ts | 4 + .../agent/providers/gjc-acp-agent.test.ts | 9 ++- .../server/agent/providers/gjc-acp-agent.ts | 6 +- 5 files changed, 118 insertions(+), 13 deletions(-) diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index ac3e15aed8..ec6c740435 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -2253,6 +2253,7 @@ describe("ACPAgentClient fetchCatalog", () => { }, mcpServers: [], runRequest: expect.any(Function), + registerProbeSession: expect.any(Function), }); expect(probeSessionCloser).toHaveBeenCalledWith({ response: { @@ -2269,6 +2270,68 @@ describe("ACPAgentClient fetchCatalog", () => { }); }); + test("closes registered catalog probe sessions without waiting for load completion", async () => { + const started = createDeferred(); + const loadSession = createDeferred(); + const registeredResponse: SessionStateResponse = { + sessionId: "registered-probe-session", + }; + const newSessionStarter = vi.fn( + (context: { registerProbeSession?: (response: SessionStateResponse) => void }) => { + context.registerProbeSession?.(registeredResponse); + started.resolve(undefined); + return loadSession.promise; + }, + ); + const probeSessionCloser = vi.fn(async () => undefined); + + class TestACPAgentClient extends ACPAgentClient { + protected override async spawnProcess(): Promise { + return { + child: { kill: vi.fn(), exitCode: 0, signalCode: null, once: vi.fn() }, + connection: { + newSession: vi.fn().mockRejectedValue(new Error("newSession should not be called")), + } as unknown as ClientSideConnection, + initialize: { agentCapabilities: {} }, + } as SpawnedACPProcess; + } + + protected override async closeProbe(): Promise {} + } + + const client = new TestACPAgentClient({ + provider: "gjc", + logger: createTestLogger(), + defaultCommand: ["gjc", "acp"], + defaultModes: [], + newSessionStarter, + probeSessionCloser, + }); + const controller = new AbortController(); + const refreshContext: ProviderRefreshContext = { + signal: controller.signal, + runActivity: async (_name, operation) => await operation(), + }; + + const refresh = client.fetchCatalog( + { scope: "workspace", cwd: "/tmp/acp-catalog-cwd", force: false }, + refreshContext, + ); + await started.promise; + + controller.abort(new Error("refresh aborted")); + await expect(refresh).rejects.toThrow("refresh aborted"); + + expect(probeSessionCloser).toHaveBeenCalledWith({ + response: registeredResponse, + config: { + provider: "gjc", + cwd: "/tmp/acp-catalog-cwd", + }, + mcpServers: [], + }); + }); + test("closes custom catalog probe sessions that finish after refresh abort", async () => { const started = createDeferred(); const session = createDeferred(); @@ -2406,10 +2469,13 @@ describe("ACPAgentClient probe diagnostics", () => { models: null, configOptions: [], }; - const newSessionStarter = vi.fn(() => { - started.resolve(undefined); - return session.promise; - }); + const newSessionStarter = vi.fn( + (context: { registerProbeSession?: (response: SessionStateResponse) => void }) => { + context.registerProbeSession?.(lateResponse); + started.resolve(undefined); + return session.promise; + }, + ); const probeSessionCloser = vi.fn(async () => undefined); const terminator = new FakeTerminator(); @@ -2448,9 +2514,6 @@ describe("ACPAgentClient probe diagnostics", () => { await started.promise; await vi.advanceTimersByTimeAsync(1); - expect(probeSessionCloser).not.toHaveBeenCalled(); - - session.resolve(lateResponse); const rows = await rowsPromise; expect(rows).toContainEqual({ diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 497078f7c3..b654683a60 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -411,6 +411,7 @@ export interface ACPNewSessionStarterContext { config: AgentSessionConfig; mcpServers: McpServer[]; runRequest: (request: () => Promise) => Promise; + registerProbeSession?: (response: SessionStateResponse) => void; } export type ACPNewSessionStarter = ( @@ -439,6 +440,7 @@ interface ACPAgentClientOptions { configOptionsTransformer?: (configOptions: SessionConfigOption[]) => SessionConfigOption[]; configFeatureOptions?: ACPConfigFeatureOption[]; clientCapabilities?: ACPClientCapabilities; + probeClientCapabilities?: ACPClientCapabilities; clientCapabilityMeta?: ACPClientCapabilityMeta; modeIdTransformer?: (modeId: string) => string | null; toolSnapshotTransformer?: (snapshot: ACPToolSnapshot) => ACPToolSnapshot; @@ -834,6 +836,7 @@ export class ACPAgentClient implements AgentClient { ) => SessionConfigOption[]; private readonly configFeatureOptions: ACPConfigFeatureOption[]; private readonly clientCapabilities?: ACPClientCapabilities; + private readonly probeClientCapabilities?: ACPClientCapabilities; private readonly clientCapabilityMeta?: ACPClientCapabilityMeta; private readonly modeIdTransformer?: (modeId: string) => string | null; private readonly toolSnapshotTransformer?: (snapshot: ACPToolSnapshot) => ACPToolSnapshot; @@ -872,6 +875,7 @@ export class ACPAgentClient implements AgentClient { this.configOptionsTransformer = options.configOptionsTransformer; this.configFeatureOptions = options.configFeatureOptions ?? []; this.clientCapabilities = options.clientCapabilities; + this.probeClientCapabilities = options.probeClientCapabilities; this.clientCapabilityMeta = options.clientCapabilityMeta; this.modeIdTransformer = options.modeIdTransformer; this.toolSnapshotTransformer = options.toolSnapshotTransformer; @@ -1244,7 +1248,7 @@ export class ACPAgentClient implements AgentClient { protocolVersion: PROTOCOL_VERSION, clientCapabilities: buildACPClientCapabilities( this.clientCapabilityMeta, - this.clientCapabilities, + this.probeClientCapabilities ?? this.clientCapabilities, ), clientInfo: { name: "Paseo", version: "dev" }, }), @@ -1450,6 +1454,7 @@ export class ACPAgentClient implements AgentClient { connection: ClientSideConnection; config: AgentSessionConfig; mcpServers: McpServer[]; + registerProbeSession?: (response: SessionStateResponse) => void; }): Promise { if (this.newSessionStarter) { return await this.newSessionStarter({ @@ -1474,6 +1479,10 @@ export class ACPAgentClient implements AgentClient { let response: SessionStateResponse | null = null; let closeRequested = false; let closePromise: Promise | null = null; + let resolveTrackedResponse: (sessionResponse: SessionStateResponse) => void = () => undefined; + const trackedResponse = new Promise((resolve) => { + resolveTrackedResponse = resolve; + }); const closeResponse = (sessionResponse: SessionStateResponse): Promise => { closePromise ??= this.closeProbeSession({ @@ -1484,11 +1493,22 @@ export class ACPAgentClient implements AgentClient { return closePromise; }; - const promise = this.startProbeSession(context).then((sessionResponse) => { + const rememberResponse = (sessionResponse: SessionStateResponse): void => { + const hadResponse = response !== null; response = sessionResponse; + if (!hadResponse) { + resolveTrackedResponse(sessionResponse); + } if (closeRequested) { void closeResponse(sessionResponse); } + }; + + const promise = this.startProbeSession({ + ...context, + registerProbeSession: rememberResponse, + }).then((sessionResponse) => { + rememberResponse(sessionResponse); return sessionResponse; }); @@ -1504,7 +1524,16 @@ export class ACPAgentClient implements AgentClient { return; } try { - await closeResponse(await promise); + const sessionResponse = await Promise.race([ + trackedResponse, + promise.then( + () => response, + () => null, + ), + ]); + if (sessionResponse) { + await closeResponse(sessionResponse); + } } catch { // If session startup failed, there is no provider-owned probe session to close. } diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.ts b/packages/server/src/server/agent/providers/generic-acp-agent.ts index a9a53f2887..3882a50a15 100644 --- a/packages/server/src/server/agent/providers/generic-acp-agent.ts +++ b/packages/server/src/server/agent/providers/generic-acp-agent.ts @@ -52,6 +52,7 @@ interface GenericACPAgentClientOptions { initialCommandsWaitTimeoutMs?: number; diagnosticPhaseTimeoutMs?: number; clientCapabilities?: GenericACPProviderParams["clientCapabilities"]; + probeClientCapabilities?: GenericACPProviderParams["clientCapabilities"]; clientCapabilityMeta?: ACPClientCapabilityMeta; configFeatureOptions?: ACPConfigFeatureOption[]; extensionCommandsParser?: ACPExtensionCommandsParser; @@ -90,6 +91,9 @@ export class GenericACPAgentClient extends ACPAgentClient { ? { initialCommandsWaitTimeoutMs: options.initialCommandsWaitTimeoutMs } : {}), ...(clientCapabilities ? { clientCapabilities } : {}), + ...(options.probeClientCapabilities + ? { probeClientCapabilities: options.probeClientCapabilities } + : {}), ...(options.clientCapabilityMeta ? { clientCapabilityMeta: options.clientCapabilityMeta } : {}), diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts index d53198e85c..b16d548f25 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts @@ -22,7 +22,7 @@ import { } from "./gjc-acp-agent.js"; describe("GjcACPAgentClient", () => { - test("uses terminal delegation and prompt permission handling during catalog discovery", async () => { + test("keeps GJC probe clients non-terminal while preserving prompt permission metadata", async () => { const initialize = vi.fn(async () => ({ agentCapabilities: {} })); const loadSession = vi.fn( async (): Promise => ({ @@ -138,7 +138,7 @@ describe("GjcACPAgentClient", () => { readTextFile: false, writeTextFile: false, }, - terminal: true, + terminal: false, _meta: { gjc: { permissionHandling: "prompt", @@ -428,6 +428,7 @@ describe("GjcACPAgentClient", () => { const loadResponse = {} as LoadSessionResponse; const loadSession = vi.fn(async () => loadResponse); const runRequest = vi.fn(async (request: () => Promise) => await request()); + const registerProbeSession = vi.fn(); const mcpServers: McpServer[] = [ { type: "http", @@ -454,6 +455,7 @@ describe("GjcACPAgentClient", () => { }, mcpServers, runRequest, + registerProbeSession, }); expect(response).toEqual({ @@ -494,6 +496,9 @@ describe("GjcACPAgentClient", () => { cwd: "/repo", mcpServers, }); + expect(registerProbeSession).toHaveBeenCalledWith({ + sessionId: "gjc-session-1", + }); expect(runRequest).toHaveBeenCalledTimes(1); }); diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.ts index 9676934d8d..6c3291926f 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.ts @@ -94,6 +94,9 @@ export class GjcACPAgentClient extends GenericACPAgentClient { label: options.label, providerParams: options.providerParams, clientCapabilities: GJC_CLIENT_CAPABILITIES, + probeClientCapabilities: { + terminal: false, + }, clientCapabilityMeta: GJC_CLIENT_CAPABILITY_META, diagnosticPhaseTimeoutMs: GJC_ACP_READINESS_TIMEOUT_MS, sessionResponseTransformer: transformGjcSessionResponse, @@ -161,7 +164,7 @@ export function createGjcACPNewSessionStarter(options: { }): ACPNewSessionStarter { const runExecFile = options.execFile ?? execFile; - return async ({ connection, config, mcpServers, runRequest }) => { + return async ({ connection, config, mcpServers, runRequest, registerProbeSession }) => { const lifecycleInput: GjcLifecycleCreateInput = { cwd: config.cwd, target: { @@ -197,6 +200,7 @@ export function createGjcACPNewSessionStarter(options: { cause: error, }); } + registerProbeSession?.({ sessionId: createResult.sessionId }); let sessionState: SessionStateResponse; try { From bd3dbb098aacd8232eb61a0b8ccd4a2844eded6f Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 20:44:43 +0900 Subject: [PATCH 06/15] Fix ACP probe cleanup failure handling --- .../server/agent/providers/acp-agent.test.ts | 119 +++++++++++++++ .../src/server/agent/providers/acp-agent.ts | 135 +++++++++++++----- .../agent/providers/gjc-acp-agent.test.ts | 73 +++++++++- .../server/agent/providers/gjc-acp-agent.ts | 112 ++++++++++++--- 4 files changed, 381 insertions(+), 58 deletions(-) diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index ec6c740435..8c8c9b6247 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -2114,6 +2114,64 @@ describe("ACPAgentClient config features", () => { mcpServers: [], }); }); + + test("rejects feature probes when custom closer fails after process cleanup", async () => { + const newSession = vi.fn().mockRejectedValue(new Error("newSession should not be called")); + const newSessionStarter = vi.fn(async () => ({ + sessionId: "probe-session-1", + configOptions: [copilotAgentConfigOption("Probe Agent")], + })); + const probeSessionCloser = vi.fn(async () => { + throw new Error("probe close failed"); + }); + const closeProbe = vi.fn(async () => undefined); + + class TestACPAgentClient extends ACPAgentClient { + protected override async spawnProcess(): Promise { + return { + child: { kill: vi.fn(), exitCode: 0, signalCode: null, once: vi.fn() }, + connection: { + newSession, + } as unknown as ClientSideConnection, + initialize: { agentCapabilities: {} }, + } as SpawnedACPProcess; + } + + protected override async closeProbe(): Promise { + await closeProbe(); + } + } + + const client = new TestACPAgentClient({ + provider: "copilot", + logger: createTestLogger(), + defaultCommand: ["copilot", "--acp"], + configFeatureOptions: [COPILOT_AGENT_FEATURE_OPTION], + newSessionStarter, + probeSessionCloser, + }); + + await expect( + client.listFeatures({ + provider: "copilot", + cwd: "/tmp/acp-features", + }), + ).rejects.toThrow("probe close failed"); + + expect(newSession).not.toHaveBeenCalled(); + expect(closeProbe).toHaveBeenCalledTimes(1); + expect(probeSessionCloser).toHaveBeenCalledWith({ + response: { + sessionId: "probe-session-1", + configOptions: [copilotAgentConfigOption("Probe Agent")], + }, + config: { + provider: "copilot", + cwd: "/tmp/acp-features", + }, + mcpServers: [], + }); + }); }); describe("ACPAgentClient sessionResponseTransformer", () => { @@ -2270,6 +2328,67 @@ describe("ACPAgentClient fetchCatalog", () => { }); }); + test("rejects catalog probes when custom closer fails after process cleanup", async () => { + const newSession = vi.fn().mockRejectedValue(new Error("newSession should not be called")); + const loadSession = vi.fn(); + const newSessionStarter = vi.fn(async () => ({ + sessionId: "probe-session-1", + modes: null, + models: null, + configOptions: [], + })); + const probeSessionCloser = vi.fn(async () => { + throw new Error("probe close failed"); + }); + const closeProbe = vi.fn(async () => undefined); + + class TestACPAgentClient extends ACPAgentClient { + protected override async spawnProcess(): Promise { + return { + child: { kill: vi.fn(), exitCode: 0, signalCode: null, once: vi.fn() }, + connection: { + newSession, + loadSession, + } as unknown as ClientSideConnection, + initialize: { agentCapabilities: {} }, + } as SpawnedACPProcess; + } + + protected override async closeProbe(): Promise { + await closeProbe(); + } + } + + const client = new TestACPAgentClient({ + provider: "gjc", + logger: createTestLogger(), + defaultCommand: ["gjc", "acp"], + defaultModes: [], + newSessionStarter, + probeSessionCloser, + }); + + await expect( + client.fetchCatalog({ scope: "workspace", cwd: "/tmp/acp-catalog-cwd", force: false }), + ).rejects.toThrow("probe close failed"); + + expect(newSession).not.toHaveBeenCalled(); + expect(closeProbe).toHaveBeenCalledTimes(1); + expect(probeSessionCloser).toHaveBeenCalledWith({ + response: { + sessionId: "probe-session-1", + modes: null, + models: null, + configOptions: [], + }, + config: { + provider: "gjc", + cwd: "/tmp/acp-catalog-cwd", + }, + mcpServers: [], + }); + }); + test("closes registered catalog probe sessions without waiting for load completion", async () => { const started = createDeferred(); const loadSession = createDeferred(); diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index b654683a60..0efe4e0fe0 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -518,6 +518,10 @@ interface TrackedACPProbeSession { close: () => Promise; } +interface ACPProbeSessionCloseOptions { + throwOnFailure?: boolean; +} + export interface ACPToolSnapshot { toolCallId: string; title: string; @@ -992,6 +996,9 @@ export class ACPAgentClient implements AgentClient { const mcpServers: McpServer[] = []; let response: SessionStateResponse | null = null; let probeSession: TrackedACPProbeSession | null = null; + let catalog: ProviderCatalog | null = null; + let operationFailed = false; + let operationError: unknown; try { const initializedProbe = await runProviderRefreshActivity(context, "initialize", () => @@ -1051,15 +1058,42 @@ export class ACPAgentClient implements AgentClient { transformed.modes, transformed.configOptions, ); - return { + catalog = { models: this.modelTransformer ? this.modelTransformer(models) : models, modes: modeInfo.modes, }; - } finally { - context?.signal.removeEventListener("abort", handleAbort); + } catch (error) { + operationFailed = true; + operationError = error; + } + + context?.signal.removeEventListener("abort", handleAbort); + let cleanupFailed = false; + let cleanupError: unknown; + try { await probeSession?.close(); + } catch (error) { + cleanupFailed = true; + cleanupError = error; + } + try { await closeProbe(); + } catch (error) { + if (!cleanupFailed) { + cleanupFailed = true; + cleanupError = error; + } + } + if (operationFailed) { + throw operationError; + } + if (cleanupFailed) { + throw cleanupError; } + if (!catalog) { + throw new Error(`${this.provider} ACP catalog probe did not return a catalog`); + } + return catalog; } async listFeatures(config: AgentSessionConfig): Promise { @@ -1072,6 +1106,9 @@ export class ACPAgentClient implements AgentClient { const probe = await this.spawnProcess(PROBE_ENV); const mcpServers: McpServer[] = []; let response: SessionStateResponse | null = null; + let features: AgentFeature[] | null = null; + let operationFailed = false; + let operationError: unknown; try { response = await this.startProbeSession({ connection: probe.connection, @@ -1079,20 +1116,50 @@ export class ACPAgentClient implements AgentClient { mcpServers, }); const transformed = this.transformSessionResponse(response); - return [ + features = [ autoAcceptFeature, ...deriveFeaturesFromACP(transformed.configOptions, this.configFeatureOptions), ]; - } finally { + } catch (error) { + operationFailed = true; + operationError = error; + } + + let cleanupFailed = false; + let cleanupError: unknown; + try { if (response) { - await this.closeProbeSession({ - response, - config: { ...config, provider: this.provider }, - mcpServers, - }); + await this.closeProbeSession( + { + response, + config: { ...config, provider: this.provider }, + mcpServers, + }, + { throwOnFailure: true }, + ); } + } catch (error) { + cleanupFailed = true; + cleanupError = error; + } + try { await this.closeProbe(probe); + } catch (error) { + if (!cleanupFailed) { + cleanupFailed = true; + cleanupError = error; + } + } + if (operationFailed) { + throw operationError; + } + if (cleanupFailed) { + throw cleanupError; + } + if (!features) { + throw new Error(`${this.provider} ACP feature probe did not return features`); } + return features; } async listImportableSessions( @@ -1477,7 +1544,6 @@ export class ACPAgentClient implements AgentClient { mcpServers: McpServer[]; }): TrackedACPProbeSession { let response: SessionStateResponse | null = null; - let closeRequested = false; let closePromise: Promise | null = null; let resolveTrackedResponse: (sessionResponse: SessionStateResponse) => void = () => undefined; const trackedResponse = new Promise((resolve) => { @@ -1485,11 +1551,14 @@ export class ACPAgentClient implements AgentClient { }); const closeResponse = (sessionResponse: SessionStateResponse): Promise => { - closePromise ??= this.closeProbeSession({ - response: sessionResponse, - config: context.config, - mcpServers: context.mcpServers, - }); + closePromise ??= this.closeProbeSession( + { + response: sessionResponse, + config: context.config, + mcpServers: context.mcpServers, + }, + { throwOnFailure: true }, + ); return closePromise; }; @@ -1499,9 +1568,6 @@ export class ACPAgentClient implements AgentClient { if (!hadResponse) { resolveTrackedResponse(sessionResponse); } - if (closeRequested) { - void closeResponse(sessionResponse); - } }; const promise = this.startProbeSession({ @@ -1515,7 +1581,6 @@ export class ACPAgentClient implements AgentClient { return { promise, close: async () => { - closeRequested = true; if (!this.probeSessionCloser) { return; } @@ -1523,25 +1588,24 @@ export class ACPAgentClient implements AgentClient { await closeResponse(response); return; } - try { - const sessionResponse = await Promise.race([ - trackedResponse, - promise.then( - () => response, - () => null, - ), - ]); - if (sessionResponse) { - await closeResponse(sessionResponse); - } - } catch { - // If session startup failed, there is no provider-owned probe session to close. + const sessionResponse = await Promise.race([ + trackedResponse, + promise.then( + () => response, + () => null, + ), + ]).catch(() => null); + if (sessionResponse) { + await closeResponse(sessionResponse); } }, }; } - private async closeProbeSession(context: ACPProbeSessionCloserContext): Promise { + private async closeProbeSession( + context: ACPProbeSessionCloserContext, + options: ACPProbeSessionCloseOptions = {}, + ): Promise { if (!this.probeSessionCloser) { return; } @@ -1557,6 +1621,9 @@ export class ACPAgentClient implements AgentClient { }, "Failed to close ACP probe session", ); + if (options.throwOnFailure) { + throw error; + } } } } diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts index b16d548f25..423e0e4217 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts @@ -1,5 +1,5 @@ import type { ChildProcessWithoutNullStreams } from "node:child_process"; -import { access, readFile, stat } from "node:fs/promises"; +import { access, readFile, rm, stat } from "node:fs/promises"; import { describe, expect, test, vi } from "vitest"; import type { @@ -562,6 +562,77 @@ describe("GjcACPAgentClient", () => { ]); }); + test("closes a gjc lifecycle session when input cleanup fails after create", async () => { + const execFile = vi + .fn() + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + ok: true, + result: { + sessionId: "gjc-session-1", + }, + }), + stderr: "", + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + ok: true, + result: { + closed: true, + }, + }), + stderr: "", + }); + const removeInputDirectory = vi.fn(async (path: string) => { + await rm(path, { recursive: true, force: true }); + throw new Error("cleanup failed"); + }); + const loadSession = vi.fn(); + const runRequest = vi.fn(async (request: () => Promise) => await request()); + const registerProbeSession = vi.fn(); + const starter = createGjcACPNewSessionStarter({ + command: ["gjc", "acp"], + execFile, + removeInputDirectory, + }); + + await expect( + starter({ + connection: { + loadSession, + } as unknown as ClientSideConnection, + config: { + provider: "gjc", + cwd: "/repo", + }, + mcpServers: [], + runRequest, + registerProbeSession, + }), + ).rejects.toThrow("GJC lifecycle input cleanup failed after session.create: cleanup failed"); + + expect(removeInputDirectory).toHaveBeenCalledTimes(1); + expect(loadSession).not.toHaveBeenCalled(); + expect(registerProbeSession).not.toHaveBeenCalled(); + expect(runRequest).not.toHaveBeenCalled(); + expect(execFile).toHaveBeenCalledTimes(2); + expect(execFile.mock.calls[1]![1]).toEqual([ + "sdk", + "session", + "raw", + "control", + "gjc-session-1", + "--op", + "session.close", + "--json-input", + "{}", + "--confirm", + "--json", + "--repo", + "/repo", + ]); + }); + test("closes a gjc probe lifecycle session after catalog use", async () => { const execFile = vi.fn(async () => ({ stdout: JSON.stringify({ diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.ts index 6c3291926f..f94c44c7d2 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.ts @@ -59,6 +59,15 @@ type GjcExecFile = ( }, ) => Promise<{ stdout: string; stderr: string }>; +type GjcInputDirectoryRemover = (path: string) => Promise; + +type GjcJsonInputFileCleanup = { ok: true } | { ok: false; error: unknown }; + +interface GjcJsonInputFileResult { + value: T; + cleanup: GjcJsonInputFileCleanup; +} + const GJC_CLIENT_CAPABILITIES = { terminal: true, }; @@ -161,6 +170,7 @@ export function createGjcACPNewSessionStarter(options: { command: [string, ...string[]]; env?: Record; execFile?: GjcExecFile; + removeInputDirectory?: GjcInputDirectoryRemover; }): ACPNewSessionStarter { const runExecFile = options.execFile ?? execFile; @@ -175,31 +185,71 @@ export function createGjcACPNewSessionStarter(options: { }; let createResult: GjcSessionCreateResult; + let createCleanup: GjcJsonInputFileCleanup = { ok: true }; try { - const { stdout } = await withGjcJsonInputFile(lifecycleInput, async (inputFilePath) => { - const lifecycleCommand = buildGjcLifecycleCreateCommand( - options.command, - config.cwd, - lifecycleInput, - { inputFilePath }, - ); - return await runExecFile(lifecycleCommand.command, lifecycleCommand.args, { - cwd: config.cwd, - env: { - ...process.env, - ...options.env, - }, - timeout: GJC_ACP_RAW_CREATE_TIMEOUT_MS, - maxBuffer: GJC_ACP_RAW_CREATE_MAX_BUFFER_BYTES, - encoding: "utf8", - }); - }); - createResult = extractGjcSessionCreateResult(parseGjcJsonOutput(stdout)); + const createCommandResult = await withGjcJsonInputFile( + lifecycleInput, + async (inputFilePath) => { + const lifecycleCommand = buildGjcLifecycleCreateCommand( + options.command, + config.cwd, + lifecycleInput, + { inputFilePath }, + ); + return await runExecFile(lifecycleCommand.command, lifecycleCommand.args, { + cwd: config.cwd, + env: { + ...process.env, + ...options.env, + }, + timeout: GJC_ACP_RAW_CREATE_TIMEOUT_MS, + maxBuffer: GJC_ACP_RAW_CREATE_MAX_BUFFER_BYTES, + encoding: "utf8", + }); + }, + options.removeInputDirectory, + ); + createCleanup = createCommandResult.cleanup; + createResult = extractGjcSessionCreateResult( + parseGjcJsonOutput(createCommandResult.value.stdout), + ); } catch (error) { throw new Error(`GJC lifecycle session.create failed: ${formatGjcExecError(error)}`, { cause: error, }); } + if (!createCleanup.ok) { + let closeError: unknown; + try { + await closeGjcLifecycleSession({ + command: options.command, + env: options.env, + execFile: runExecFile, + cwd: config.cwd, + sessionId: createResult.sessionId, + }); + } catch (error) { + closeError = error; + } + if (closeError) { + throw new Error( + `GJC lifecycle input cleanup failed after session.create, and session.close failed: ${formatGjcExecError( + closeError, + )}`, + { + cause: createCleanup.error, + }, + ); + } + throw new Error( + `GJC lifecycle input cleanup failed after session.create: ${formatGjcExecError( + createCleanup.error, + )}`, + { + cause: createCleanup.error, + }, + ); + } registerProbeSession?.({ sessionId: createResult.sessionId }); let sessionState: SessionStateResponse; @@ -301,16 +351,32 @@ export function buildGjcLifecycleCloseCommand( async function withGjcJsonInputFile( input: GjcLifecycleCreateInput, operation: (inputFilePath: string) => Promise, -): Promise { + removeInputDirectory: GjcInputDirectoryRemover = (path) => + rm(path, { recursive: true, force: true }), +): Promise> { const directory = await mkdtemp(join(tmpdir(), "paseo-gjc-json-")); const inputFilePath = join(directory, "input.json"); + let outcome: { ok: true; value: T } | { ok: false; error: unknown }; try { await writeFile(inputFilePath, JSON.stringify(input), { mode: 0o600 }); await chmod(inputFilePath, 0o600); - return await operation(inputFilePath); - } finally { - await rm(directory, { recursive: true, force: true }); + outcome = { ok: true, value: await operation(inputFilePath) }; + } catch (error) { + outcome = { ok: false, error }; + } + + let cleanup: GjcJsonInputFileCleanup; + try { + await removeInputDirectory(directory); + cleanup = { ok: true }; + } catch (error) { + cleanup = { ok: false, error }; + } + + if (!outcome.ok) { + throw outcome.error; } + return { value: outcome.value, cleanup }; } async function closeGjcLifecycleSession(options: { From 6524452d7818938685225af507c5090c34f2d06c Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 21:10:26 +0900 Subject: [PATCH 07/15] Close GJC lifecycle sessions after init failures --- .../server/agent/providers/acp-agent.test.ts | 62 +++++++++++++++++++ .../src/server/agent/providers/acp-agent.ts | 30 ++++++++- .../agent/providers/generic-acp-agent.ts | 4 ++ .../agent/providers/gjc-acp-agent.test.ts | 46 ++++++++++++++ .../server/agent/providers/gjc-acp-agent.ts | 13 ++++ 5 files changed, 153 insertions(+), 2 deletions(-) diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index 8c8c9b6247..665d3afcfc 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -3931,6 +3931,68 @@ describe("ACPAgentSession initialization cleanup", () => { expect(terminator.terminated).toContain(child); }); + test("closes custom lifecycle sessions when post-load initialization fails", async () => { + const terminator = new FakeTerminator(); + const child = createProbeChildStub(); + const configOptions = [selectConfigOption("thought_level", ["low"], "low")]; + const response: SessionStateResponse = { + sessionId: "lifecycle-session-1", + configOptions, + }; + const newSessionStarter = vi.fn(async () => response); + const newSessionFailureCloser = vi.fn(async () => undefined); + const thinkingOptionWriter = vi.fn(async () => { + throw new Error("thinking override failed"); + }); + + class FailingConfiguredOverride extends ACPAgentSession { + protected override async spawnProcess(): Promise { + return { + child, + connection: { + newSession: vi.fn().mockRejectedValue(new Error("newSession should not be called")), + } as unknown as ClientSideConnection, + initialize: { agentCapabilities: {} }, + }; + } + } + + const session = new FailingConfiguredOverride( + { + provider: "gjc", + cwd: "/tmp/paseo-acp-test", + thinkingOptionId: "xhigh", + }, + { + provider: "gjc", + logger: createTestLogger(), + defaultCommand: ["gjc", "acp"], + defaultModes: [], + newSessionStarter, + newSessionFailureCloser, + thinkingOptionWriter, + capabilities: { + supportsStreaming: true, + supportsSessionPersistence: true, + }, + terminateProcess: terminator.terminate, + }, + ); + + await expect(session.initializeNewSession()).rejects.toThrow("thinking override failed"); + + expect(newSessionFailureCloser).toHaveBeenCalledWith({ + response, + config: { + provider: "gjc", + cwd: "/tmp/paseo-acp-test", + thinkingOptionId: "xhigh", + }, + mcpServers: [], + }); + expect(terminator.terminated).toContain(child); + }); + test("terminates the ACP process when session/load fails", async () => { const terminator = new FakeTerminator(); const child = createProbeChildStub(); diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 0efe4e0fe0..415cb8751b 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -434,6 +434,7 @@ interface ACPAgentClientOptions { defaultModes?: AgentMode[]; catalogModelResolver?: ACPCatalogModelResolver; newSessionStarter?: ACPNewSessionStarter; + newSessionFailureCloser?: ACPProbeSessionCloser; probeSessionCloser?: ACPProbeSessionCloser; modelTransformer?: (models: AgentModelDefinition[]) => AgentModelDefinition[]; sessionResponseTransformer?: (response: SessionStateResponse) => SessionStateResponse; @@ -467,6 +468,7 @@ interface ACPAgentSessionOptions { defaultCommand: [string, ...string[]]; defaultModes: AgentMode[]; newSessionStarter?: ACPNewSessionStarter; + newSessionFailureCloser?: ACPProbeSessionCloser; modelTransformer?: (models: AgentModelDefinition[]) => AgentModelDefinition[]; sessionResponseTransformer?: (response: SessionStateResponse) => SessionStateResponse; configOptionsTransformer?: (configOptions: SessionConfigOption[]) => SessionConfigOption[]; @@ -830,6 +832,7 @@ export class ACPAgentClient implements AgentClient { protected readonly defaultModes: AgentMode[]; private readonly catalogModelResolver?: ACPCatalogModelResolver; private readonly newSessionStarter?: ACPNewSessionStarter; + private readonly newSessionFailureCloser?: ACPProbeSessionCloser; private readonly probeSessionCloser?: ACPProbeSessionCloser; private readonly modelTransformer?: (models: AgentModelDefinition[]) => AgentModelDefinition[]; private readonly sessionResponseTransformer?: ( @@ -873,6 +876,7 @@ export class ACPAgentClient implements AgentClient { this.defaultModes = options.defaultModes ?? []; this.catalogModelResolver = options.catalogModelResolver; this.newSessionStarter = options.newSessionStarter; + this.newSessionFailureCloser = options.newSessionFailureCloser; this.probeSessionCloser = options.probeSessionCloser; this.modelTransformer = options.modelTransformer; this.sessionResponseTransformer = options.sessionResponseTransformer; @@ -905,6 +909,7 @@ export class ACPAgentClient implements AgentClient { defaultCommand: this.defaultCommand, defaultModes: this.defaultModes, newSessionStarter: this.newSessionStarter, + newSessionFailureCloser: this.newSessionFailureCloser, modelTransformer: this.modelTransformer, sessionResponseTransformer: this.sessionResponseTransformer, configOptionsTransformer: this.configOptionsTransformer, @@ -1655,6 +1660,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { private readonly defaultCommand: [string, ...string[]]; private readonly defaultModes: AgentMode[]; private readonly newSessionStarter?: ACPNewSessionStarter; + private readonly newSessionFailureCloser?: ACPProbeSessionCloser; protected readonly modelTransformer?: (models: AgentModelDefinition[]) => AgentModelDefinition[]; private readonly sessionResponseTransformer?: ( response: SessionStateResponse, @@ -1726,6 +1732,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { this.defaultCommand = options.defaultCommand; this.defaultModes = options.defaultModes; this.newSessionStarter = options.newSessionStarter; + this.newSessionFailureCloser = options.newSessionFailureCloser; this.modelTransformer = options.modelTransformer; this.sessionResponseTransformer = options.sessionResponseTransformer; this.configOptionsTransformer = options.configOptionsTransformer; @@ -1756,6 +1763,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { } async initializeNewSession(): Promise { + let newSessionCleanupContext: ACPProbeSessionCloserContext | null = null; try { const spawned = await this.spawnProcess(); this.child = spawned.child; @@ -1776,12 +1784,17 @@ export class ACPAgentSession implements AgentSession, ACPClient { mcpServers, }), ); + newSessionCleanupContext = { + response, + config: this.config, + mcpServers, + }; this.sessionId = response.sessionId; this.bootstrapThreadEventPending = true; this.applySessionState(response); await this.applyConfiguredOverrides(); } catch (error) { - await this.closeAfterInitializationFailure(error); + await this.closeAfterInitializationFailure(error, newSessionCleanupContext); } } @@ -1839,7 +1852,20 @@ export class ACPAgentSession implements AgentSession, ACPClient { } } - private async closeAfterInitializationFailure(error: unknown): Promise { + private async closeAfterInitializationFailure( + error: unknown, + newSessionCleanupContext?: ACPProbeSessionCloserContext | null, + ): Promise { + if (newSessionCleanupContext && this.newSessionFailureCloser) { + try { + await this.newSessionFailureCloser(newSessionCleanupContext); + } catch (closeError) { + this.logger.warn( + { err: closeError, initializationError: error }, + "Failed to close ACP lifecycle session after initialization failure", + ); + } + } try { await this.close(); } catch (closeError) { diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.ts b/packages/server/src/server/agent/providers/generic-acp-agent.ts index 3882a50a15..ea702ddca6 100644 --- a/packages/server/src/server/agent/providers/generic-acp-agent.ts +++ b/packages/server/src/server/agent/providers/generic-acp-agent.ts @@ -58,6 +58,7 @@ interface GenericACPAgentClientOptions { extensionCommandsParser?: ACPExtensionCommandsParser; catalogModelResolver?: ACPCatalogModelResolver; newSessionStarter?: ACPNewSessionStarter; + newSessionFailureCloser?: ACPProbeSessionCloser; probeSessionCloser?: ACPProbeSessionCloser; sessionResponseTransformer?: (response: SessionStateResponse) => SessionStateResponse; configOptionsTransformer?: (configOptions: SessionConfigOption[]) => SessionConfigOption[]; @@ -114,6 +115,9 @@ export class GenericACPAgentClient extends ACPAgentClient { : {}), ...(options.modeIdTransformer ? { modeIdTransformer: options.modeIdTransformer } : {}), ...(options.newSessionStarter ? { newSessionStarter: options.newSessionStarter } : {}), + ...(options.newSessionFailureCloser + ? { newSessionFailureCloser: options.newSessionFailureCloser } + : {}), ...(options.probeSessionCloser ? { probeSessionCloser: options.probeSessionCloser } : {}), }); diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts index 423e0e4217..5f8a487e11 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts @@ -633,6 +633,52 @@ describe("GjcACPAgentClient", () => { ]); }); + test("preserves input cleanup failure details when lifecycle create fails", async () => { + const createError = new Error("create failed"); + const cleanupError = new Error("cleanup failed"); + const execFile = vi.fn(async () => { + throw createError; + }); + const removeInputDirectory = vi.fn(async (path: string) => { + await rm(path, { recursive: true, force: true }); + throw cleanupError; + }); + const loadSession = vi.fn(); + const runRequest = vi.fn(async (request: () => Promise) => await request()); + const starter = createGjcACPNewSessionStarter({ + command: ["gjc", "acp"], + execFile, + removeInputDirectory, + }); + + let thrown: unknown; + try { + await starter({ + connection: { + loadSession, + } as unknown as ClientSideConnection, + config: { + provider: "gjc", + cwd: "/repo", + }, + mcpServers: [], + runRequest, + }); + } catch (error) { + thrown = error; + } + + expect(thrown).toBeInstanceOf(Error); + expect((thrown as Error).message).toBe( + "GJC lifecycle session.create failed: GJC lifecycle request failed and input cleanup failed: create failed; cleanup: cleanup failed", + ); + expect((thrown as Error).cause).toBeInstanceOf(AggregateError); + expect(((thrown as Error).cause as AggregateError).errors).toEqual([createError, cleanupError]); + expect(removeInputDirectory).toHaveBeenCalledTimes(1); + expect(loadSession).not.toHaveBeenCalled(); + expect(runRequest).not.toHaveBeenCalled(); + }); + test("closes a gjc probe lifecycle session after catalog use", async () => { const execFile = vi.fn(async () => ({ stdout: JSON.stringify({ diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.ts index f94c44c7d2..fd0ab39d82 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.ts @@ -116,6 +116,11 @@ export class GjcACPAgentClient extends GenericACPAgentClient { env: options.env, execFile: options.execFile, }), + newSessionFailureCloser: createGjcACPProbeSessionCloser({ + command: options.command, + env: options.env, + execFile: options.execFile, + }), probeSessionCloser: createGjcACPProbeSessionCloser({ command: options.command, env: options.env, @@ -374,6 +379,14 @@ async function withGjcJsonInputFile( } if (!outcome.ok) { + if (!cleanup.ok) { + throw new AggregateError( + [outcome.error, cleanup.error], + `GJC lifecycle request failed and input cleanup failed: ${formatGjcExecError( + outcome.error, + )}; cleanup: ${formatGjcExecError(cleanup.error)}`, + ); + } throw outcome.error; } return { value: outcome.value, cleanup }; From aa7a879c2873edb60e99a4bfc51d72e3608bccca Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 21:19:42 +0900 Subject: [PATCH 08/15] Abort pending ACP probe startup during cleanup --- .../server/agent/providers/acp-agent.test.ts | 27 +++--- .../src/server/agent/providers/acp-agent.ts | 25 +++--- .../agent/providers/gjc-acp-agent.test.ts | 85 +++++++++++++++++-- .../server/agent/providers/gjc-acp-agent.ts | 24 +++++- 4 files changed, 127 insertions(+), 34 deletions(-) diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index 665d3afcfc..e07482969b 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -2312,6 +2312,7 @@ describe("ACPAgentClient fetchCatalog", () => { mcpServers: [], runRequest: expect.any(Function), registerProbeSession: expect.any(Function), + signal: expect.any(AbortSignal), }); expect(probeSessionCloser).toHaveBeenCalledWith({ response: { @@ -2451,16 +2452,18 @@ describe("ACPAgentClient fetchCatalog", () => { }); }); - test("closes custom catalog probe sessions that finish after refresh abort", async () => { + test("does not wait for unregistered catalog probe sessions after refresh abort", async () => { const started = createDeferred(); const session = createDeferred(); + let startupSignal: AbortSignal | undefined; const lateResponse: SessionStateResponse = { sessionId: "late-probe-session", modes: null, models: null, configOptions: [], }; - const newSessionStarter = vi.fn(() => { + const newSessionStarter = vi.fn((context: { signal?: AbortSignal }) => { + startupSignal = context.signal; started.resolve(undefined); return session.promise; }); @@ -2500,24 +2503,14 @@ describe("ACPAgentClient fetchCatalog", () => { ); await started.promise; - let refreshSettled = false; - void refresh.then( - () => { - refreshSettled = true; - return undefined; - }, - () => { - refreshSettled = true; - return undefined; - }, - ); controller.abort(new Error("refresh aborted")); - await Promise.resolve(); - expect(refreshSettled).toBe(false); - expect(probeSessionCloser).not.toHaveBeenCalled(); + await expect(refresh).rejects.toThrow("refresh aborted"); + expect(startupSignal?.aborted).toBe(true); + expect(probeSessionCloser).not.toHaveBeenCalled(); session.resolve(lateResponse); - await expect(refresh).rejects.toThrow("refresh aborted"); + await Promise.resolve(); + await Promise.resolve(); expect(probeSessionCloser).toHaveBeenCalledWith({ response: lateResponse, diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 415cb8751b..939796518e 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -412,6 +412,7 @@ export interface ACPNewSessionStarterContext { mcpServers: McpServer[]; runRequest: (request: () => Promise) => Promise; registerProbeSession?: (response: SessionStateResponse) => void; + signal?: AbortSignal; } export type ACPNewSessionStarter = ( @@ -1527,6 +1528,7 @@ export class ACPAgentClient implements AgentClient { config: AgentSessionConfig; mcpServers: McpServer[]; registerProbeSession?: (response: SessionStateResponse) => void; + signal?: AbortSignal; }): Promise { if (this.newSessionStarter) { return await this.newSessionStarter({ @@ -1549,11 +1551,13 @@ export class ACPAgentClient implements AgentClient { mcpServers: McpServer[]; }): TrackedACPProbeSession { let response: SessionStateResponse | null = null; + let closeRequested = false; let closePromise: Promise | null = null; let resolveTrackedResponse: (sessionResponse: SessionStateResponse) => void = () => undefined; const trackedResponse = new Promise((resolve) => { resolveTrackedResponse = resolve; }); + const startupAbortController = new AbortController(); const closeResponse = (sessionResponse: SessionStateResponse): Promise => { closePromise ??= this.closeProbeSession( @@ -1573,11 +1577,15 @@ export class ACPAgentClient implements AgentClient { if (!hadResponse) { resolveTrackedResponse(sessionResponse); } + if (closeRequested && this.probeSessionCloser) { + void closeResponse(sessionResponse).catch(() => undefined); + } }; const promise = this.startProbeSession({ ...context, registerProbeSession: rememberResponse, + signal: startupAbortController.signal, }).then((sessionResponse) => { rememberResponse(sessionResponse); return sessionResponse; @@ -1586,23 +1594,20 @@ export class ACPAgentClient implements AgentClient { return { promise, close: async () => { + closeRequested = true; if (!this.probeSessionCloser) { + startupAbortController.abort(new Error(`${this.provider} ACP probe startup cancelled`)); return; } if (response) { await closeResponse(response); return; } - const sessionResponse = await Promise.race([ - trackedResponse, - promise.then( - () => response, - () => null, - ), - ]).catch(() => null); - if (sessionResponse) { - await closeResponse(sessionResponse); - } + startupAbortController.abort(new Error(`${this.provider} ACP probe startup cancelled`)); + void trackedResponse + .then((sessionResponse) => closeResponse(sessionResponse)) + .catch(() => undefined); + void promise.catch(() => undefined); }, }; } diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts index 5f8a487e11..6c216e5429 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts @@ -258,6 +258,13 @@ describe("GjcACPAgentClient", () => { expect(settled).toBe(false); await vi.advanceTimersByTimeAsync(40_000); + await expect(diagnostic).resolves.toEqual({ + diagnostic: expect.stringContaining( + "ACP session/new: error: ACP session/new timed out after 60000ms", + ), + }); + expect(execFile).toHaveBeenCalledTimes(1); + resolveCreate({ stdout: JSON.stringify({ ok: true, @@ -267,12 +274,7 @@ describe("GjcACPAgentClient", () => { }), stderr: "", }); - await expect(diagnostic).resolves.toEqual({ - diagnostic: expect.stringContaining( - "ACP session/new: error: ACP session/new timed out after 60000ms", - ), - }); - expect(execFile).toHaveBeenCalledTimes(2); + await vi.waitFor(() => expect(execFile).toHaveBeenCalledTimes(2)); } finally { vi.useRealTimers(); } @@ -562,6 +564,77 @@ describe("GjcACPAgentClient", () => { ]); }); + test("closes a gjc lifecycle session when startup is aborted after create", async () => { + const controller = new AbortController(); + const abortError = new Error("startup cancelled"); + controller.abort(abortError); + const execFile = vi + .fn() + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + ok: true, + result: { + sessionId: "gjc-session-1", + }, + }), + stderr: "", + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + ok: true, + result: { + closed: true, + }, + }), + stderr: "", + }); + const loadSession = vi.fn(); + const runRequest = vi.fn(async (request: () => Promise) => await request()); + const starter = createGjcACPNewSessionStarter({ + command: ["gjc", "acp"], + execFile, + }); + + await expect( + starter({ + connection: { + loadSession, + } as unknown as ClientSideConnection, + config: { + provider: "gjc", + cwd: "/repo", + }, + mcpServers: [], + runRequest, + signal: controller.signal, + }), + ).rejects.toThrow("startup cancelled"); + + expect(loadSession).not.toHaveBeenCalled(); + expect(runRequest).not.toHaveBeenCalled(); + expect(execFile).toHaveBeenCalledTimes(2); + expect(execFile.mock.calls[0]![2]).toEqual( + expect.objectContaining({ + signal: controller.signal, + }), + ); + expect(execFile.mock.calls[1]![1]).toEqual([ + "sdk", + "session", + "raw", + "control", + "gjc-session-1", + "--op", + "session.close", + "--json-input", + "{}", + "--confirm", + "--json", + "--repo", + "/repo", + ]); + }); + test("closes a gjc lifecycle session when input cleanup fails after create", async () => { const execFile = vi .fn() diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.ts index fd0ab39d82..85198ca74d 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.ts @@ -56,6 +56,7 @@ type GjcExecFile = ( timeout: number; maxBuffer: number; encoding: BufferEncoding; + signal?: AbortSignal; }, ) => Promise<{ stdout: string; stderr: string }>; @@ -179,7 +180,7 @@ export function createGjcACPNewSessionStarter(options: { }): ACPNewSessionStarter { const runExecFile = options.execFile ?? execFile; - return async ({ connection, config, mcpServers, runRequest, registerProbeSession }) => { + return async ({ connection, config, mcpServers, runRequest, registerProbeSession, signal }) => { const lifecycleInput: GjcLifecycleCreateInput = { cwd: config.cwd, target: { @@ -210,6 +211,7 @@ export function createGjcACPNewSessionStarter(options: { timeout: GJC_ACP_RAW_CREATE_TIMEOUT_MS, maxBuffer: GJC_ACP_RAW_CREATE_MAX_BUFFER_BYTES, encoding: "utf8", + signal, }); }, options.removeInputDirectory, @@ -255,6 +257,17 @@ export function createGjcACPNewSessionStarter(options: { }, ); } + const abortError = getGjcLifecycleAbortError(signal); + if (abortError) { + await closeGjcLifecycleSession({ + command: options.command, + env: options.env, + execFile: runExecFile, + cwd: config.cwd, + sessionId: createResult.sessionId, + }).catch(() => undefined); + throw abortError; + } registerProbeSession?.({ sessionId: createResult.sessionId }); let sessionState: SessionStateResponse; @@ -526,6 +539,15 @@ function assertGjcLifecycleCommandSucceeded(stdout: string): void { } } +function getGjcLifecycleAbortError(signal: AbortSignal | undefined): Error | null { + if (!signal?.aborted) { + return null; + } + return signal.reason instanceof Error + ? signal.reason + : new Error("GJC lifecycle session startup cancelled"); +} + function getSessionStateResponseId(response: SessionStateResponse): string | null { return "sessionId" in response && typeof response.sessionId === "string" ? response.sessionId From 66f375d5ecd84eed340233cd569808dd48bb5765 Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 21:24:36 +0900 Subject: [PATCH 09/15] Thread launch env through GJC lifecycle cleanup --- .../server/agent/providers/acp-agent.test.ts | 6 ++ .../src/server/agent/providers/acp-agent.ts | 4 + .../agent/providers/gjc-acp-agent.test.ts | 80 +++++++++++++++- .../server/agent/providers/gjc-acp-agent.ts | 93 ++++++++++++++----- 4 files changed, 158 insertions(+), 25 deletions(-) diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index e07482969b..d9de0ff9c6 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -3964,6 +3964,9 @@ describe("ACPAgentSession initialization cleanup", () => { newSessionStarter, newSessionFailureCloser, thinkingOptionWriter, + launchEnv: { + PASEO_AGENT_ID: "agent-1", + }, capabilities: { supportsStreaming: true, supportsSessionPersistence: true, @@ -3981,6 +3984,9 @@ describe("ACPAgentSession initialization cleanup", () => { cwd: "/tmp/paseo-acp-test", thinkingOptionId: "xhigh", }, + launchEnv: { + PASEO_AGENT_ID: "agent-1", + }, mcpServers: [], }); expect(terminator.terminated).toContain(child); diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 939796518e..ecd0e413e6 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -413,6 +413,7 @@ export interface ACPNewSessionStarterContext { runRequest: (request: () => Promise) => Promise; registerProbeSession?: (response: SessionStateResponse) => void; signal?: AbortSignal; + launchEnv?: Record; } export type ACPNewSessionStarter = ( @@ -423,6 +424,7 @@ export interface ACPProbeSessionCloserContext { response: SessionStateResponse; config: AgentSessionConfig; mcpServers: McpServer[]; + launchEnv?: Record; } export type ACPProbeSessionCloser = (context: ACPProbeSessionCloserContext) => Promise; @@ -1782,6 +1784,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { config: this.config, mcpServers, runRequest: (request) => this.runACPRequest(request), + launchEnv: this.launchEnv, }) : await this.runACPRequest(() => this.connection!.newSession({ @@ -1793,6 +1796,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { response, config: this.config, mcpServers, + launchEnv: this.launchEnv, }; this.sessionId = response.sessionId; this.bootstrapThreadEventPending = true; diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts index 6c216e5429..bd9ad4eb61 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts @@ -474,6 +474,7 @@ describe("GjcACPAgentClient", () => { timeout: 130_000, maxBuffer: 1024 * 1024, encoding: "utf8", + signal: undefined, }), ); const args = execFile.mock.calls[0]![1]; @@ -529,6 +530,9 @@ describe("GjcACPAgentClient", () => { const runRequest = vi.fn(async (request: () => Promise) => await request()); const starter = createGjcACPNewSessionStarter({ command: ["gjc", "acp"], + env: { + GJC_LOG: "debug", + }, execFile, }); @@ -543,10 +547,20 @@ describe("GjcACPAgentClient", () => { }, mcpServers: [], runRequest, + launchEnv: { + GJC_LOG: "trace", + PASEO_AGENT_ID: "agent-1", + }, }), ).rejects.toThrow("load failed"); expect(execFile).toHaveBeenCalledTimes(2); + expect(execFile.mock.calls[0]![2].env).toEqual( + expect.objectContaining({ + GJC_LOG: "trace", + PASEO_AGENT_ID: "agent-1", + }), + ); expect(execFile.mock.calls[1]![1]).toEqual([ "sdk", "session", @@ -562,6 +576,65 @@ describe("GjcACPAgentClient", () => { "--repo", "/repo", ]); + expect(execFile.mock.calls[1]![2].env).toEqual( + expect.objectContaining({ + GJC_LOG: "trace", + PASEO_AGENT_ID: "agent-1", + }), + ); + }); + + test("surfaces session.close failures when ACP load fails", async () => { + const loadError = new Error("load failed"); + const closeError = new Error("close failed"); + const execFile = vi + .fn() + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + ok: true, + result: { + sessionId: "gjc-session-1", + }, + }), + stderr: "", + }) + .mockRejectedValueOnce(closeError); + const loadSession = vi.fn().mockRejectedValue(loadError); + const runRequest = vi.fn(async (request: () => Promise) => await request()); + const starter = createGjcACPNewSessionStarter({ + command: ["gjc", "acp"], + execFile, + }); + + let thrown: unknown; + try { + await starter({ + connection: { + loadSession, + } as unknown as ClientSideConnection, + config: { + provider: "gjc", + cwd: "/repo", + }, + mcpServers: [], + runRequest, + }); + } catch (error) { + thrown = error; + } + + expect(thrown).toBeInstanceOf(AggregateError); + expect((thrown as AggregateError).message).toBe( + "GJC lifecycle session.load failed and session.close failed: load failed; cleanup: GJC lifecycle session.close failed: close failed", + ); + expect((thrown as AggregateError).errors).toEqual([ + loadError, + expect.objectContaining({ + cause: closeError, + message: "GJC lifecycle session.close failed: close failed", + }), + ]); + expect(execFile).toHaveBeenCalledTimes(2); }); test("closes a gjc lifecycle session when startup is aborted after create", async () => { @@ -778,6 +851,10 @@ describe("GjcACPAgentClient", () => { provider: "gjc", cwd: "/repo", }, + launchEnv: { + GJC_LOG: "trace", + PASEO_AGENT_ID: "agent-1", + }, mcpServers: [], }); @@ -801,7 +878,8 @@ describe("GjcACPAgentClient", () => { expect.objectContaining({ cwd: "/repo", env: expect.objectContaining({ - GJC_LOG: "debug", + GJC_LOG: "trace", + PASEO_AGENT_ID: "agent-1", }), timeout: 30_000, maxBuffer: 1024 * 1024, diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.ts index 85198ca74d..fa3102965b 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.ts @@ -20,6 +20,7 @@ import type { SessionStateResponse, } from "./acp-agent.js"; import { GenericACPAgentClient } from "./generic-acp-agent.js"; +import { createProviderEnv } from "../provider-launch-config.js"; interface GjcACPAgentClientOptions { logger: Logger; @@ -180,7 +181,15 @@ export function createGjcACPNewSessionStarter(options: { }): ACPNewSessionStarter { const runExecFile = options.execFile ?? execFile; - return async ({ connection, config, mcpServers, runRequest, registerProbeSession, signal }) => { + return async ({ + connection, + config, + mcpServers, + runRequest, + registerProbeSession, + signal, + launchEnv, + }) => { const lifecycleInput: GjcLifecycleCreateInput = { cwd: config.cwd, target: { @@ -204,10 +213,7 @@ export function createGjcACPNewSessionStarter(options: { ); return await runExecFile(lifecycleCommand.command, lifecycleCommand.args, { cwd: config.cwd, - env: { - ...process.env, - ...options.env, - }, + env: buildGjcLifecycleEnv(options.env, launchEnv), timeout: GJC_ACP_RAW_CREATE_TIMEOUT_MS, maxBuffer: GJC_ACP_RAW_CREATE_MAX_BUFFER_BYTES, encoding: "utf8", @@ -231,6 +237,7 @@ export function createGjcACPNewSessionStarter(options: { await closeGjcLifecycleSession({ command: options.command, env: options.env, + launchEnv, execFile: runExecFile, cwd: config.cwd, sessionId: createResult.sessionId, @@ -259,13 +266,27 @@ export function createGjcACPNewSessionStarter(options: { } const abortError = getGjcLifecycleAbortError(signal); if (abortError) { - await closeGjcLifecycleSession({ - command: options.command, - env: options.env, - execFile: runExecFile, - cwd: config.cwd, - sessionId: createResult.sessionId, - }).catch(() => undefined); + let closeError: unknown; + try { + await closeGjcLifecycleSession({ + command: options.command, + env: options.env, + launchEnv, + execFile: runExecFile, + cwd: config.cwd, + sessionId: createResult.sessionId, + }); + } catch (error) { + closeError = error; + } + if (closeError) { + throw new AggregateError( + [abortError, closeError], + `GJC lifecycle session startup cancelled and session.close failed: ${formatGjcExecError( + abortError, + )}; cleanup: ${formatGjcExecError(closeError)}`, + ); + } throw abortError; } registerProbeSession?.({ sessionId: createResult.sessionId }); @@ -280,13 +301,29 @@ export function createGjcACPNewSessionStarter(options: { }), ); } catch (error) { - await closeGjcLifecycleSession({ - command: options.command, - env: options.env, - execFile: runExecFile, - cwd: config.cwd, - sessionId: createResult.sessionId, - }).catch(() => undefined); + let closeError: unknown; + try { + await closeGjcLifecycleSession({ + command: options.command, + env: options.env, + launchEnv, + execFile: runExecFile, + cwd: config.cwd, + sessionId: createResult.sessionId, + }); + } catch (cleanupError) { + closeError = cleanupError; + } + if (closeError) { + const loadAndCloseError = new AggregateError( + [error, closeError], + `GJC lifecycle session.load failed and session.close failed: ${formatGjcExecError( + error, + )}; cleanup: ${formatGjcExecError(closeError)}`, + { cause: error }, + ); + throw loadAndCloseError; + } throw error; } return { @@ -303,7 +340,7 @@ export function createGjcACPProbeSessionCloser(options: { }): ACPProbeSessionCloser { const runExecFile = options.execFile ?? execFile; - return async ({ response, config }) => { + return async ({ response, config, launchEnv }) => { const sessionId = getSessionStateResponseId(response); if (!sessionId) { throw new Error("GJC probe session did not expose a session id"); @@ -311,6 +348,7 @@ export function createGjcACPProbeSessionCloser(options: { await closeGjcLifecycleSession({ command: options.command, env: options.env, + launchEnv, execFile: runExecFile, cwd: config.cwd, sessionId, @@ -408,6 +446,7 @@ async function withGjcJsonInputFile( async function closeGjcLifecycleSession(options: { command: [string, ...string[]]; env?: Record; + launchEnv?: Record; execFile: GjcExecFile; cwd: string; sessionId: string; @@ -420,10 +459,7 @@ async function closeGjcLifecycleSession(options: { try { const { stdout } = await options.execFile(lifecycleCommand.command, lifecycleCommand.args, { cwd: options.cwd, - env: { - ...process.env, - ...options.env, - }, + env: buildGjcLifecycleEnv(options.env, options.launchEnv), timeout: GJC_ACP_RAW_CLOSE_TIMEOUT_MS, maxBuffer: GJC_ACP_RAW_CREATE_MAX_BUFFER_BYTES, encoding: "utf8", @@ -436,6 +472,15 @@ async function closeGjcLifecycleSession(options: { } } +function buildGjcLifecycleEnv( + providerEnv: Record | undefined, + launchEnv: Record | undefined, +): NodeJS.ProcessEnv { + return createProviderEnv({ + overlays: [providerEnv, launchEnv], + }); +} + function isGjcUnsupportedHostLifecycleMode(modeId: string): boolean { return GJC_UNSUPPORTED_HOST_LIFECYCLE_MODE_IDS.has(modeId); } From 5fdc6bdb75697eaaaa2c79b7d1507c4af0dca234 Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 21:31:14 +0900 Subject: [PATCH 10/15] Close GJC lifecycle sessions on shutdown --- .../server/agent/providers/acp-agent.test.ts | 88 +++++++++++++++++++ .../src/server/agent/providers/acp-agent.ts | 29 +++++- .../agent/providers/generic-acp-agent.ts | 2 + .../server/agent/providers/gjc-acp-agent.ts | 5 ++ 4 files changed, 122 insertions(+), 2 deletions(-) diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index d9de0ff9c6..0e1a508b04 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -18,6 +18,7 @@ import { import { ACPAgentClient, ACPAgentSession, + type ACPProbeSessionCloser, type SpawnedACPProcess, type SessionStateResponse, buildACPClientCapabilities, @@ -3857,6 +3858,93 @@ describe("ACPAgentSession close() tree-kill", () => { await close; }); + test("close() closes custom lifecycle sessions when ACP close is not advertised", async () => { + const terminator = new FakeTerminator(); + const child = createProbeChildStub(); + const sessionCloser = vi.fn(async () => undefined) satisfies ACPProbeSessionCloser; + const unstableCloseSession = vi.fn(async () => undefined); + const session = new ACPAgentSession( + { + provider: "gjc", + cwd: "/tmp/paseo-acp-test", + }, + { + provider: "gjc", + logger: createTestLogger(), + defaultCommand: ["gjc", "acp"], + defaultModes: [], + sessionCloser, + launchEnv: { + PASEO_AGENT_ID: "agent-1", + }, + capabilities: { + supportsStreaming: true, + supportsSessionPersistence: true, + }, + terminateProcess: terminator.terminate, + }, + ); + const internals = asInternals(session); + internals.child = child; + internals.connection = { + unstable_closeSession: unstableCloseSession, + } as unknown as ClientSideConnection; + internals.sessionId = "lifecycle-session-1"; + + await session.close(); + + expect(sessionCloser).toHaveBeenCalledWith({ + response: { sessionId: "lifecycle-session-1" }, + config: { + provider: "gjc", + cwd: "/tmp/paseo-acp-test", + }, + launchEnv: { + PASEO_AGENT_ID: "agent-1", + }, + mcpServers: [], + }); + expect(unstableCloseSession).not.toHaveBeenCalled(); + expect(terminator.terminated).toContain(child); + }); + + test("close() terminates the ACP process when custom lifecycle close fails", async () => { + const terminator = new FakeTerminator(); + const child = createProbeChildStub(); + const closeError = new Error("lifecycle close failed"); + const sessionCloser = vi.fn(async () => { + throw closeError; + }) satisfies ACPProbeSessionCloser; + const session = new ACPAgentSession( + { + provider: "gjc", + cwd: "/tmp/paseo-acp-test", + }, + { + provider: "gjc", + logger: createTestLogger(), + defaultCommand: ["gjc", "acp"], + defaultModes: [], + sessionCloser, + capabilities: { + supportsStreaming: true, + supportsSessionPersistence: true, + }, + terminateProcess: terminator.terminate, + }, + ); + const internals = asInternals(session); + internals.child = child; + internals.connection = {} as ClientSideConnection; + internals.sessionId = "lifecycle-session-1"; + + await expect(session.close()).rejects.toThrow("lifecycle close failed"); + + expect(terminator.terminated).toContain(child); + expect(internals.connection).toBeNull(); + expect(internals.child).toBeNull(); + }); + test("killTerminal terminates the terminal process tree without a direct SIGTERM", async () => { const terminator = new FakeTerminator(); const session = createSession(terminator.terminate); diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index ecd0e413e6..0cb649a933 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -438,6 +438,7 @@ interface ACPAgentClientOptions { catalogModelResolver?: ACPCatalogModelResolver; newSessionStarter?: ACPNewSessionStarter; newSessionFailureCloser?: ACPProbeSessionCloser; + sessionCloser?: ACPProbeSessionCloser; probeSessionCloser?: ACPProbeSessionCloser; modelTransformer?: (models: AgentModelDefinition[]) => AgentModelDefinition[]; sessionResponseTransformer?: (response: SessionStateResponse) => SessionStateResponse; @@ -472,6 +473,7 @@ interface ACPAgentSessionOptions { defaultModes: AgentMode[]; newSessionStarter?: ACPNewSessionStarter; newSessionFailureCloser?: ACPProbeSessionCloser; + sessionCloser?: ACPProbeSessionCloser; modelTransformer?: (models: AgentModelDefinition[]) => AgentModelDefinition[]; sessionResponseTransformer?: (response: SessionStateResponse) => SessionStateResponse; configOptionsTransformer?: (configOptions: SessionConfigOption[]) => SessionConfigOption[]; @@ -836,6 +838,7 @@ export class ACPAgentClient implements AgentClient { private readonly catalogModelResolver?: ACPCatalogModelResolver; private readonly newSessionStarter?: ACPNewSessionStarter; private readonly newSessionFailureCloser?: ACPProbeSessionCloser; + private readonly sessionCloser?: ACPProbeSessionCloser; private readonly probeSessionCloser?: ACPProbeSessionCloser; private readonly modelTransformer?: (models: AgentModelDefinition[]) => AgentModelDefinition[]; private readonly sessionResponseTransformer?: ( @@ -880,6 +883,7 @@ export class ACPAgentClient implements AgentClient { this.catalogModelResolver = options.catalogModelResolver; this.newSessionStarter = options.newSessionStarter; this.newSessionFailureCloser = options.newSessionFailureCloser; + this.sessionCloser = options.sessionCloser; this.probeSessionCloser = options.probeSessionCloser; this.modelTransformer = options.modelTransformer; this.sessionResponseTransformer = options.sessionResponseTransformer; @@ -913,6 +917,7 @@ export class ACPAgentClient implements AgentClient { defaultModes: this.defaultModes, newSessionStarter: this.newSessionStarter, newSessionFailureCloser: this.newSessionFailureCloser, + sessionCloser: this.sessionCloser, modelTransformer: this.modelTransformer, sessionResponseTransformer: this.sessionResponseTransformer, configOptionsTransformer: this.configOptionsTransformer, @@ -963,6 +968,7 @@ export class ACPAgentClient implements AgentClient { runtimeSettings: this.runtimeSettings, defaultCommand: this.defaultCommand, defaultModes: this.defaultModes, + sessionCloser: this.sessionCloser, modelTransformer: this.modelTransformer, sessionResponseTransformer: this.sessionResponseTransformer, configOptionsTransformer: this.configOptionsTransformer, @@ -1668,6 +1674,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { private readonly defaultModes: AgentMode[]; private readonly newSessionStarter?: ACPNewSessionStarter; private readonly newSessionFailureCloser?: ACPProbeSessionCloser; + private readonly sessionCloser?: ACPProbeSessionCloser; protected readonly modelTransformer?: (models: AgentModelDefinition[]) => AgentModelDefinition[]; private readonly sessionResponseTransformer?: ( response: SessionStateResponse, @@ -1740,6 +1747,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { this.defaultModes = options.defaultModes; this.newSessionStarter = options.newSessionStarter; this.newSessionFailureCloser = options.newSessionFailureCloser; + this.sessionCloser = options.sessionCloser; this.modelTransformer = options.modelTransformer; this.sessionResponseTransformer = options.sessionResponseTransformer; this.configOptionsTransformer = options.configOptionsTransformer; @@ -2484,6 +2492,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { return; } this.closed = true; + let sessionCloseError: unknown; this.deliverTranslatedEvents(this.flushPendingUserMessage()); this.settleCommandsReady(); @@ -2501,11 +2510,23 @@ export class ACPAgentSession implements AgentSession, ACPClient { } catch {} try { - if (this.agentCapabilities?.sessionCapabilities?.close) { + if (this.sessionCloser) { + await this.sessionCloser({ + response: { sessionId: this.sessionId }, + config: this.config, + mcpServers: this.acpMcpServers(), + launchEnv: this.launchEnv, + }); + } else if (this.agentCapabilities?.sessionCapabilities?.close) { await this.connection.unstable_closeSession({ sessionId: this.sessionId }); } } catch (error) { - this.logger.debug({ err: error }, "ACP closeSession failed during shutdown"); + if (this.sessionCloser) { + sessionCloseError = error; + this.logger.warn({ err: error }, "ACP lifecycle session close failed during shutdown"); + } else { + this.logger.debug({ err: error }, "ACP closeSession failed during shutdown"); + } } } @@ -2526,6 +2547,10 @@ export class ACPAgentSession implements AgentSession, ACPClient { this.connection = null; this.child = null; this.activeForegroundTurnId = null; + + if (sessionCloseError) { + throw sessionCloseError; + } } async requestPermission(params: RequestPermissionRequest): Promise { diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.ts b/packages/server/src/server/agent/providers/generic-acp-agent.ts index ea702ddca6..08ead8d354 100644 --- a/packages/server/src/server/agent/providers/generic-acp-agent.ts +++ b/packages/server/src/server/agent/providers/generic-acp-agent.ts @@ -59,6 +59,7 @@ interface GenericACPAgentClientOptions { catalogModelResolver?: ACPCatalogModelResolver; newSessionStarter?: ACPNewSessionStarter; newSessionFailureCloser?: ACPProbeSessionCloser; + sessionCloser?: ACPProbeSessionCloser; probeSessionCloser?: ACPProbeSessionCloser; sessionResponseTransformer?: (response: SessionStateResponse) => SessionStateResponse; configOptionsTransformer?: (configOptions: SessionConfigOption[]) => SessionConfigOption[]; @@ -118,6 +119,7 @@ export class GenericACPAgentClient extends ACPAgentClient { ...(options.newSessionFailureCloser ? { newSessionFailureCloser: options.newSessionFailureCloser } : {}), + ...(options.sessionCloser ? { sessionCloser: options.sessionCloser } : {}), ...(options.probeSessionCloser ? { probeSessionCloser: options.probeSessionCloser } : {}), }); diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.ts index fa3102965b..69f8da51cf 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.ts @@ -123,6 +123,11 @@ export class GjcACPAgentClient extends GenericACPAgentClient { env: options.env, execFile: options.execFile, }), + sessionCloser: createGjcACPProbeSessionCloser({ + command: options.command, + env: options.env, + execFile: options.execFile, + }), probeSessionCloser: createGjcACPProbeSessionCloser({ command: options.command, env: options.env, From 574d815382bae5d8251aa7db5200b25b30b6d01f Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 21:34:22 +0900 Subject: [PATCH 11/15] Avoid double closing registered GJC probes --- .../agent/providers/gjc-acp-agent.test.ts | 40 +++++++++++++++++++ .../server/agent/providers/gjc-acp-agent.ts | 4 ++ 2 files changed, 44 insertions(+) diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts index bd9ad4eb61..914f654f0b 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts @@ -584,6 +584,46 @@ describe("GjcACPAgentClient", () => { ); }); + test("lets the probe tracker close registered lifecycle sessions when ACP load fails", async () => { + const execFile = vi.fn().mockResolvedValueOnce({ + stdout: JSON.stringify({ + ok: true, + result: { + sessionId: "gjc-session-1", + }, + }), + stderr: "", + }); + const loadError = new Error("load failed"); + const loadSession = vi.fn().mockRejectedValue(loadError); + const registerProbeSession = vi.fn(); + const runRequest = vi.fn(async (request: () => Promise) => await request()); + const starter = createGjcACPNewSessionStarter({ + command: ["gjc", "acp"], + execFile, + }); + + await expect( + starter({ + connection: { + loadSession, + } as unknown as ClientSideConnection, + config: { + provider: "gjc", + cwd: "/repo", + }, + mcpServers: [], + runRequest, + registerProbeSession, + }), + ).rejects.toThrow("load failed"); + + expect(registerProbeSession).toHaveBeenCalledWith({ + sessionId: "gjc-session-1", + }); + expect(execFile).toHaveBeenCalledTimes(1); + }); + test("surfaces session.close failures when ACP load fails", async () => { const loadError = new Error("load failed"); const closeError = new Error("close failed"); diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.ts index 69f8da51cf..b6195af111 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.ts @@ -294,6 +294,7 @@ export function createGjcACPNewSessionStarter(options: { } throw abortError; } + const closeViaProbeTracker = Boolean(registerProbeSession); registerProbeSession?.({ sessionId: createResult.sessionId }); let sessionState: SessionStateResponse; @@ -306,6 +307,9 @@ export function createGjcACPNewSessionStarter(options: { }), ); } catch (error) { + if (closeViaProbeTracker) { + throw error; + } let closeError: unknown; try { await closeGjcLifecycleSession({ From 64078dda288df3b11d639c1e8ade00f87f589563 Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 21:44:03 +0900 Subject: [PATCH 12/15] Recover aborted GJC lifecycle creates --- .../agent/providers/gjc-acp-agent.test.ts | 70 +++++++++ .../server/agent/providers/gjc-acp-agent.ts | 138 ++++++++++++++---- 2 files changed, 178 insertions(+), 30 deletions(-) diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts index 914f654f0b..8fdc9261a8 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts @@ -21,6 +21,14 @@ import { transformGjcSessionResponse, } from "./gjc-acp-agent.js"; +function idempotencyKeyArgIndex(args: string[]): number { + const index = args.indexOf("--idempotency-key"); + if (index === -1) { + throw new Error("Expected GJC lifecycle command to include an idempotency key"); + } + return index + 1; +} + describe("GjcACPAgentClient", () => { test("keeps GJC probe clients non-terminal while preserving prompt permission metadata", async () => { const initialize = vi.fn(async () => ({ agentCapabilities: {} })); @@ -748,6 +756,68 @@ describe("GjcACPAgentClient", () => { ]); }); + test("recovers and registers a lifecycle session when create is aborted before JSON returns", async () => { + const controller = new AbortController(); + const abortError = new Error("startup cancelled"); + const createAbortError = Object.assign(new Error("The operation was aborted"), { + stdout: "", + stderr: "", + }); + const execFile = vi + .fn() + .mockImplementationOnce(async () => { + controller.abort(abortError); + throw createAbortError; + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + ok: true, + result: { + sessionId: "gjc-session-1", + }, + }), + stderr: "", + }); + const loadSession = vi.fn(); + const registerProbeSession = vi.fn(); + const runRequest = vi.fn(async (request: () => Promise) => await request()); + const starter = createGjcACPNewSessionStarter({ + command: ["gjc", "acp"], + execFile, + }); + + await expect( + starter({ + connection: { + loadSession, + } as unknown as ClientSideConnection, + config: { + provider: "gjc", + cwd: "/repo", + }, + mcpServers: [], + runRequest, + registerProbeSession, + signal: controller.signal, + }), + ).rejects.toThrow("startup cancelled"); + + expect(loadSession).not.toHaveBeenCalled(); + expect(runRequest).not.toHaveBeenCalled(); + expect(registerProbeSession).toHaveBeenCalledWith({ + sessionId: "gjc-session-1", + }); + expect(execFile).toHaveBeenCalledTimes(2); + const firstArgs = execFile.mock.calls[0]![1]; + const secondArgs = execFile.mock.calls[1]![1]; + expect(firstArgs).toContain("session.create"); + expect(secondArgs).toContain("session.create"); + expect(secondArgs[idempotencyKeyArgIndex(secondArgs)]).toBe( + firstArgs[idempotencyKeyArgIndex(firstArgs)], + ); + expect(execFile.mock.calls[1]![2].signal).toBeUndefined(); + }); + test("closes a gjc lifecycle session when input cleanup fails after create", async () => { const execFile = vi .fn() diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.ts index b6195af111..9c4a431306 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.ts @@ -61,6 +61,8 @@ type GjcExecFile = ( }, ) => Promise<{ stdout: string; stderr: string }>; +type GjcExecFileResult = Awaited>; + type GjcInputDirectoryRemover = (path: string) => Promise; type GjcJsonInputFileCleanup = { ok: true } | { ok: false; error: unknown }; @@ -204,17 +206,16 @@ export function createGjcACPNewSessionStarter(options: { ...(mcpServers.length > 0 ? { mcpServers } : {}), }; - let createResult: GjcSessionCreateResult; - let createCleanup: GjcJsonInputFileCleanup = { ok: true }; - try { - const createCommandResult = await withGjcJsonInputFile( + const createIdempotencyKey = randomUUID(); + const runCreateCommand = async (commandSignal?: AbortSignal) => + await withGjcJsonInputFile( lifecycleInput, async (inputFilePath) => { const lifecycleCommand = buildGjcLifecycleCreateCommand( options.command, config.cwd, lifecycleInput, - { inputFilePath }, + { inputFilePath, idempotencyKey: createIdempotencyKey }, ); return await runExecFile(lifecycleCommand.command, lifecycleCommand.args, { cwd: config.cwd, @@ -222,16 +223,74 @@ export function createGjcACPNewSessionStarter(options: { timeout: GJC_ACP_RAW_CREATE_TIMEOUT_MS, maxBuffer: GJC_ACP_RAW_CREATE_MAX_BUFFER_BYTES, encoding: "utf8", - signal, + signal: commandSignal, }); }, options.removeInputDirectory, ); + + const closeCreatedSessionAfterAbort = async ( + sessionId: string, + abortError: Error, + ): Promise => { + if (registerProbeSession) { + registerProbeSession({ sessionId }); + throw abortError; + } + + let closeError: unknown; + try { + await closeGjcLifecycleSession({ + command: options.command, + env: options.env, + launchEnv, + execFile: runExecFile, + cwd: config.cwd, + sessionId, + }); + } catch (error) { + closeError = error; + } + if (closeError) { + throw new AggregateError( + [abortError, closeError], + `GJC lifecycle session startup cancelled and session.close failed: ${formatGjcExecError( + abortError, + )}; cleanup: ${formatGjcExecError(closeError)}`, + ); + } + throw abortError; + }; + + let createResult: GjcSessionCreateResult; + let createCleanup: GjcJsonInputFileCleanup = { ok: true }; + try { + const createCommandResult = await runCreateCommand(signal); createCleanup = createCommandResult.cleanup; createResult = extractGjcSessionCreateResult( parseGjcJsonOutput(createCommandResult.value.stdout), ); } catch (error) { + const abortError = getGjcLifecycleAbortError(signal); + if (abortError) { + let recoveredResult: GjcSessionCreateResult; + try { + recoveredResult = await recoverGjcLifecycleCreateResultAfterAbort({ + error, + runCreateCommand, + }); + } catch (recoveryError) { + const createAndRecoveryError = new AggregateError( + [error, recoveryError], + `GJC lifecycle session.create failed after startup cancellation, and session recovery failed: ${formatGjcExecError( + error, + )}; recovery: ${formatGjcExecError(recoveryError)}`, + { cause: recoveryError }, + ); + throw createAndRecoveryError; + } + await closeCreatedSessionAfterAbort(recoveredResult.sessionId, abortError); + } throw new Error(`GJC lifecycle session.create failed: ${formatGjcExecError(error)}`, { cause: error, }); @@ -271,28 +330,7 @@ export function createGjcACPNewSessionStarter(options: { } const abortError = getGjcLifecycleAbortError(signal); if (abortError) { - let closeError: unknown; - try { - await closeGjcLifecycleSession({ - command: options.command, - env: options.env, - launchEnv, - execFile: runExecFile, - cwd: config.cwd, - sessionId: createResult.sessionId, - }); - } catch (error) { - closeError = error; - } - if (closeError) { - throw new AggregateError( - [abortError, closeError], - `GJC lifecycle session startup cancelled and session.close failed: ${formatGjcExecError( - abortError, - )}; cleanup: ${formatGjcExecError(closeError)}`, - ); - } - throw abortError; + await closeCreatedSessionAfterAbort(createResult.sessionId, abortError); } const closeViaProbeTracker = Boolean(registerProbeSession); registerProbeSession?.({ sessionId: createResult.sessionId }); @@ -369,7 +407,7 @@ export function buildGjcLifecycleCreateCommand( acpCommand: [string, ...string[]], cwd: string, input: GjcLifecycleCreateInput, - options: { inputFilePath?: string } = {}, + options: { inputFilePath?: string; idempotencyKey?: string } = {}, ): GjcLifecycleCommand { const jsonInputArgs = options.inputFilePath ? ["--json-input-file", options.inputFilePath] @@ -384,7 +422,7 @@ export function buildGjcLifecycleCreateCommand( "session.create", ...jsonInputArgs, "--idempotency-key", - randomUUID(), + options.idempotencyKey ?? randomUUID(), "--json", "--repo", cwd, @@ -452,6 +490,19 @@ async function withGjcJsonInputFile( return { value: outcome.value, cleanup }; } +async function recoverGjcLifecycleCreateResultAfterAbort(options: { + error: unknown; + runCreateCommand: () => Promise>; +}): Promise { + const stdoutResult = extractGjcSessionCreateResultFromError(options.error); + if (stdoutResult) { + return stdoutResult; + } + + const retryResult = await options.runCreateCommand(); + return extractGjcSessionCreateResult(parseGjcJsonOutput(retryResult.value.stdout)); +} + async function closeGjcLifecycleSession(options: { command: [string, ...string[]]; env?: Record; @@ -608,6 +659,33 @@ function getSessionStateResponseId(response: SessionStateResponse): string | nul : null; } +function extractGjcSessionCreateResultFromError(error: unknown): GjcSessionCreateResult | null { + const stdout = extractGjcExecStdout(error); + if (!stdout) { + return null; + } + try { + return extractGjcSessionCreateResult(parseGjcJsonOutput(stdout)); + } catch { + return null; + } +} + +function extractGjcExecStdout(error: unknown): string | null { + if (isRecord(error) && typeof error.stdout === "string" && error.stdout.trim()) { + return error.stdout; + } + if (error instanceof AggregateError) { + for (const nestedError of error.errors) { + const stdout = extractGjcExecStdout(nestedError); + if (stdout) { + return stdout; + } + } + } + return null; +} + function formatGjcBrokerError(value: Record): string { const error = value.error; if (isRecord(error)) { From 9958d89395ba0b9b1b448bf4499a7ab09de337dd Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 21:46:02 +0900 Subject: [PATCH 13/15] Avoid duplicate close after ACP init cleanup --- packages/server/src/server/agent/providers/acp-agent.test.ts | 3 +++ packages/server/src/server/agent/providers/acp-agent.ts | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index 0e1a508b04..f3d5cf1861 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -4022,6 +4022,7 @@ describe("ACPAgentSession initialization cleanup", () => { }; const newSessionStarter = vi.fn(async () => response); const newSessionFailureCloser = vi.fn(async () => undefined); + const sessionCloser = vi.fn(async () => undefined); const thinkingOptionWriter = vi.fn(async () => { throw new Error("thinking override failed"); }); @@ -4051,6 +4052,7 @@ describe("ACPAgentSession initialization cleanup", () => { defaultModes: [], newSessionStarter, newSessionFailureCloser, + sessionCloser, thinkingOptionWriter, launchEnv: { PASEO_AGENT_ID: "agent-1", @@ -4077,6 +4079,7 @@ describe("ACPAgentSession initialization cleanup", () => { }, mcpServers: [], }); + expect(sessionCloser).not.toHaveBeenCalled(); expect(terminator.terminated).toContain(child); }); diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 0cb649a933..5eb085d335 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -1876,6 +1876,10 @@ export class ACPAgentSession implements AgentSession, ACPClient { if (newSessionCleanupContext && this.newSessionFailureCloser) { try { await this.newSessionFailureCloser(newSessionCleanupContext); + const closedSessionId = getACPResponseSessionId(newSessionCleanupContext.response); + if (closedSessionId && this.sessionId === closedSessionId) { + this.sessionId = null; + } } catch (closeError) { this.logger.warn( { err: closeError, initializationError: error }, From 61d0eb51b58e9a00dde8ae0c90dc225908873f7a Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 21:52:14 +0900 Subject: [PATCH 14/15] Preserve GJC create handles on cancellation --- .../agent/providers/gjc-acp-agent.test.ts | 46 +++--------- .../server/agent/providers/gjc-acp-agent.ts | 74 ++----------------- 2 files changed, 15 insertions(+), 105 deletions(-) diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts index 8fdc9261a8..b2f29a45fa 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.test.ts @@ -21,14 +21,6 @@ import { transformGjcSessionResponse, } from "./gjc-acp-agent.js"; -function idempotencyKeyArgIndex(args: string[]): number { - const index = args.indexOf("--idempotency-key"); - if (index === -1) { - throw new Error("Expected GJC lifecycle command to include an idempotency key"); - } - return index + 1; -} - describe("GjcACPAgentClient", () => { test("keeps GJC probe clients non-terminal while preserving prompt permission metadata", async () => { const initialize = vi.fn(async () => ({ agentCapabilities: {} })); @@ -482,7 +474,6 @@ describe("GjcACPAgentClient", () => { timeout: 130_000, maxBuffer: 1024 * 1024, encoding: "utf8", - signal: undefined, }), ); const args = execFile.mock.calls[0]![1]; @@ -734,11 +725,7 @@ describe("GjcACPAgentClient", () => { expect(loadSession).not.toHaveBeenCalled(); expect(runRequest).not.toHaveBeenCalled(); expect(execFile).toHaveBeenCalledTimes(2); - expect(execFile.mock.calls[0]![2]).toEqual( - expect.objectContaining({ - signal: controller.signal, - }), - ); + expect(execFile.mock.calls[0]![2].signal).toBeUndefined(); expect(execFile.mock.calls[1]![1]).toEqual([ "sdk", "session", @@ -756,20 +743,12 @@ describe("GjcACPAgentClient", () => { ]); }); - test("recovers and registers a lifecycle session when create is aborted before JSON returns", async () => { + test("registers a created lifecycle session when probe startup is aborted after create", async () => { const controller = new AbortController(); const abortError = new Error("startup cancelled"); - const createAbortError = Object.assign(new Error("The operation was aborted"), { - stdout: "", - stderr: "", - }); - const execFile = vi - .fn() - .mockImplementationOnce(async () => { - controller.abort(abortError); - throw createAbortError; - }) - .mockResolvedValueOnce({ + const execFile = vi.fn().mockImplementationOnce(async () => { + controller.abort(abortError); + return { stdout: JSON.stringify({ ok: true, result: { @@ -777,7 +756,8 @@ describe("GjcACPAgentClient", () => { }, }), stderr: "", - }); + }; + }); const loadSession = vi.fn(); const registerProbeSession = vi.fn(); const runRequest = vi.fn(async (request: () => Promise) => await request()); @@ -807,15 +787,9 @@ describe("GjcACPAgentClient", () => { expect(registerProbeSession).toHaveBeenCalledWith({ sessionId: "gjc-session-1", }); - expect(execFile).toHaveBeenCalledTimes(2); - const firstArgs = execFile.mock.calls[0]![1]; - const secondArgs = execFile.mock.calls[1]![1]; - expect(firstArgs).toContain("session.create"); - expect(secondArgs).toContain("session.create"); - expect(secondArgs[idempotencyKeyArgIndex(secondArgs)]).toBe( - firstArgs[idempotencyKeyArgIndex(firstArgs)], - ); - expect(execFile.mock.calls[1]![2].signal).toBeUndefined(); + expect(execFile).toHaveBeenCalledTimes(1); + expect(execFile.mock.calls[0]![1]).toContain("session.create"); + expect(execFile.mock.calls[0]![2].signal).toBeUndefined(); }); test("closes a gjc lifecycle session when input cleanup fails after create", async () => { diff --git a/packages/server/src/server/agent/providers/gjc-acp-agent.ts b/packages/server/src/server/agent/providers/gjc-acp-agent.ts index 9c4a431306..5243cf2cc5 100644 --- a/packages/server/src/server/agent/providers/gjc-acp-agent.ts +++ b/packages/server/src/server/agent/providers/gjc-acp-agent.ts @@ -61,8 +61,6 @@ type GjcExecFile = ( }, ) => Promise<{ stdout: string; stderr: string }>; -type GjcExecFileResult = Awaited>; - type GjcInputDirectoryRemover = (path: string) => Promise; type GjcJsonInputFileCleanup = { ok: true } | { ok: false; error: unknown }; @@ -206,8 +204,7 @@ export function createGjcACPNewSessionStarter(options: { ...(mcpServers.length > 0 ? { mcpServers } : {}), }; - const createIdempotencyKey = randomUUID(); - const runCreateCommand = async (commandSignal?: AbortSignal) => + const runCreateCommand = async () => await withGjcJsonInputFile( lifecycleInput, async (inputFilePath) => { @@ -215,7 +212,7 @@ export function createGjcACPNewSessionStarter(options: { options.command, config.cwd, lifecycleInput, - { inputFilePath, idempotencyKey: createIdempotencyKey }, + { inputFilePath }, ); return await runExecFile(lifecycleCommand.command, lifecycleCommand.args, { cwd: config.cwd, @@ -223,7 +220,6 @@ export function createGjcACPNewSessionStarter(options: { timeout: GJC_ACP_RAW_CREATE_TIMEOUT_MS, maxBuffer: GJC_ACP_RAW_CREATE_MAX_BUFFER_BYTES, encoding: "utf8", - signal: commandSignal, }); }, options.removeInputDirectory, @@ -265,32 +261,12 @@ export function createGjcACPNewSessionStarter(options: { let createResult: GjcSessionCreateResult; let createCleanup: GjcJsonInputFileCleanup = { ok: true }; try { - const createCommandResult = await runCreateCommand(signal); + const createCommandResult = await runCreateCommand(); createCleanup = createCommandResult.cleanup; createResult = extractGjcSessionCreateResult( parseGjcJsonOutput(createCommandResult.value.stdout), ); } catch (error) { - const abortError = getGjcLifecycleAbortError(signal); - if (abortError) { - let recoveredResult: GjcSessionCreateResult; - try { - recoveredResult = await recoverGjcLifecycleCreateResultAfterAbort({ - error, - runCreateCommand, - }); - } catch (recoveryError) { - const createAndRecoveryError = new AggregateError( - [error, recoveryError], - `GJC lifecycle session.create failed after startup cancellation, and session recovery failed: ${formatGjcExecError( - error, - )}; recovery: ${formatGjcExecError(recoveryError)}`, - { cause: recoveryError }, - ); - throw createAndRecoveryError; - } - await closeCreatedSessionAfterAbort(recoveredResult.sessionId, abortError); - } throw new Error(`GJC lifecycle session.create failed: ${formatGjcExecError(error)}`, { cause: error, }); @@ -407,7 +383,7 @@ export function buildGjcLifecycleCreateCommand( acpCommand: [string, ...string[]], cwd: string, input: GjcLifecycleCreateInput, - options: { inputFilePath?: string; idempotencyKey?: string } = {}, + options: { inputFilePath?: string } = {}, ): GjcLifecycleCommand { const jsonInputArgs = options.inputFilePath ? ["--json-input-file", options.inputFilePath] @@ -422,7 +398,7 @@ export function buildGjcLifecycleCreateCommand( "session.create", ...jsonInputArgs, "--idempotency-key", - options.idempotencyKey ?? randomUUID(), + randomUUID(), "--json", "--repo", cwd, @@ -490,19 +466,6 @@ async function withGjcJsonInputFile( return { value: outcome.value, cleanup }; } -async function recoverGjcLifecycleCreateResultAfterAbort(options: { - error: unknown; - runCreateCommand: () => Promise>; -}): Promise { - const stdoutResult = extractGjcSessionCreateResultFromError(options.error); - if (stdoutResult) { - return stdoutResult; - } - - const retryResult = await options.runCreateCommand(); - return extractGjcSessionCreateResult(parseGjcJsonOutput(retryResult.value.stdout)); -} - async function closeGjcLifecycleSession(options: { command: [string, ...string[]]; env?: Record; @@ -659,33 +622,6 @@ function getSessionStateResponseId(response: SessionStateResponse): string | nul : null; } -function extractGjcSessionCreateResultFromError(error: unknown): GjcSessionCreateResult | null { - const stdout = extractGjcExecStdout(error); - if (!stdout) { - return null; - } - try { - return extractGjcSessionCreateResult(parseGjcJsonOutput(stdout)); - } catch { - return null; - } -} - -function extractGjcExecStdout(error: unknown): string | null { - if (isRecord(error) && typeof error.stdout === "string" && error.stdout.trim()) { - return error.stdout; - } - if (error instanceof AggregateError) { - for (const nestedError of error.errors) { - const stdout = extractGjcExecStdout(nestedError); - if (stdout) { - return stdout; - } - } - } - return null; -} - function formatGjcBrokerError(value: Record): string { const error = value.error; if (isRecord(error)) { From 938403356b8a86bc70fc8cc08d28e68f409827cf Mon Sep 17 00:00:00 2001 From: Suho Han Date: Sat, 15 Aug 2026 21:54:25 +0900 Subject: [PATCH 15/15] Include launch env in ACP terminal delegation --- .../server/agent/providers/acp-agent.test.ts | 45 +++++++++++++++++++ .../src/server/agent/providers/acp-agent.ts | 4 +- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index f3d5cf1861..8f2c12b12e 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -699,6 +699,51 @@ describe("ACPAgentSession terminal tools", () => { ); }); + test("includes launch env in delegated terminal commands", async () => { + const child = createTerminalChildStub(); + const spawn = vi.spyOn(spawnUtils, "spawnProcess").mockReturnValue(child); + const session = new ACPAgentSession( + { + provider: "gjc", + cwd: "/tmp/paseo-acp-test", + }, + { + provider: "gjc", + logger: createTestLogger(), + defaultCommand: ["gjc", "acp"], + defaultModes: [], + launchEnv: { + GJC_SESSION_TOKEN: "launch-token", + PASEO_AGENT_ID: "agent-1", + }, + capabilities: { + supportsStreaming: true, + supportsSessionPersistence: true, + }, + }, + ); + + await session.createTerminal({ + sessionId: "session-1", + command: "node", + args: ["script.js"], + cwd: "/repo", + env: [{ name: "GJC_SESSION_TOKEN", value: "request-token" }], + }); + + expect(spawn).toHaveBeenCalledWith( + "node", + ["script.js"], + expect.objectContaining({ + cwd: "/repo", + envOverlay: expect.objectContaining({ + GJC_SESSION_TOKEN: "request-token", + PASEO_AGENT_ID: "agent-1", + }), + }), + ); + }); + test("surfaces spawn errors through terminal output and waitForTerminalExit", async () => { const child = createTerminalChildStub(); vi.spyOn(spawnUtils, "spawnProcess").mockReturnValue(child); diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 5eb085d335..7008b5b337 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -2716,7 +2716,9 @@ export class ACPAgentSession implements AgentSession, ACPClient { ); const terminalCommand = resolveTerminalCommand(params.command, params.args); const commandEnvOverlays = - terminalCommand.shell === false ? [env, createStringCommandShellEnvOverlay()] : [env]; + terminalCommand.shell === false + ? [this.launchEnv, env, createStringCommandShellEnvOverlay()] + : [this.launchEnv, env]; const child = spawnProcess(terminalCommand.command, terminalCommand.args, { cwd: params.cwd ?? this.config.cwd, ...createProviderEnvSpec({