diff --git a/apps/ade-cli/src/services/agentRegistry.ts b/apps/ade-cli/src/services/agentRegistry.ts index 2b54d5881e..8410fc3ebf 100644 --- a/apps/ade-cli/src/services/agentRegistry.ts +++ b/apps/ade-cli/src/services/agentRegistry.ts @@ -1,5 +1,6 @@ import { CURSOR_CLI_EXECUTABLES } from "../../../desktop/src/shared/providerCliExecutables"; import { resolveProviderRemediation } from "../../../desktop/src/shared/providerRemediation"; +import { COPILOT_NPM_PACKAGE_SPEC } from "../../../desktop/src/shared/acpProviderMetadata"; import type { ShippedProvider } from "../../../desktop/src/shared/providers"; export type AgentCliErrorCategory = "missing" | "unauthenticated"; @@ -230,7 +231,7 @@ export const AGENT_CLI_REGISTRY: AgentCliDescriptor[] = [ agent: "copilot", displayName: "GitHub Copilot CLI", binaryNames: ["copilot"], - installCommand: npmGlobalInstallCommand("@github/copilot"), + installCommand: npmGlobalInstallCommand(COPILOT_NPM_PACKAGE_SPEC), authCommand: "copilot login", missingErrorPatterns: [ /\bcopilot\b.*\b(command not found|not recognized|not found|enoent)\b/i, diff --git a/apps/desktop/src/main/services/chat/acpHost/acpDialects/copilot.ts b/apps/desktop/src/main/services/chat/acpHost/acpDialects/copilot.ts index 2fb91ba15a..8ed7035571 100644 --- a/apps/desktop/src/main/services/chat/acpHost/acpDialects/copilot.ts +++ b/apps/desktop/src/main/services/chat/acpHost/acpDialects/copilot.ts @@ -6,14 +6,12 @@ * * ## Verified rules * - * - Version 1.0.82 (ACP agent 1.0.4) advertises `loadSession`, image prompts, - * and session list. It does **not** advertise `session/resume` or - * `session/close`; both answer -32601. ADE still sends `session/close` and - * degrades, keeping the pooled process. Live 1.0.82 on this machine completed - * real `session/prompt` turns (text `"ping"`, usage on the prompt result and - * `usage_update`). Cancel mid-prompt returned `stopReason: "end_turn"` with - * partial text — github/copilot-cli #4561, live. Config options arrive as - * `currentValue` / nested `value`, not ADE's `value` / `options[].id`. + * - The 1.0.86 compatibility baseline advertises `loadSession`, image prompts, + * HTTP/SSE MCP, and session list/close. It does not advertise + * `session/resume`. Config options arrive as `currentValue` / nested `value`, + * not ADE's `value` / `options[].id`. Older 1.0.x binaries may omit close; + * the host gates lifecycle calls against the handshake and keeps a shared + * process alive when it has to degrade. * - **Known bug.** Cancel may report `stopReason: "end_turn"` * (github/copilot-cli issue 4561). ADE records its own cancel and marks the * turn interrupted whatever the agent says. That accounting lives in the @@ -60,6 +58,7 @@ import { standardClose, standardLoad, standardSetModel, + standardSetConfigOption, transportGatedMcpInjection, withOptionalEnv, } from "./shared"; @@ -89,6 +88,32 @@ export const COPILOT_TUI_ONLY_COMMANDS: ReadonlySet = new Set([ export const COPILOT_CANCEL_DEGRADATION_NOTE = "Copilot sometimes reports a stopped turn as finished. ADE marks it stopped."; +export const COPILOT_PERMISSION_DEGRADATION_NOTE = + "Copilot ACP has no auto-edit mode. ADE maps auto-edit and auto to approval-gated Agent mode."; + +export function copilotPermissionModeDegradationNote(mode: string | null | undefined): string | null { + return mode === "auto-edit" || mode === "auto" ? COPILOT_PERMISSION_DEGRADATION_NOTE : null; +} + +export const COPILOT_NATIVE_MODE_IDS = { + agent: "https://agentclientprotocol.com/protocol/session-modes#agent", + plan: "https://agentclientprotocol.com/protocol/session-modes#plan", + autopilot: "https://agentclientprotocol.com/protocol/session-modes#autopilot", +} as const; + +export const COPILOT_CONFIG_OPTION_IDS = ["mode", "allow_all"] as const; + +export function copilotNativeModeValue(mode: string): string { + if (mode === "plan") return COPILOT_NATIVE_MODE_IDS.plan; + if (mode === "yolo") return COPILOT_NATIVE_MODE_IDS.autopilot; + return COPILOT_NATIVE_MODE_IDS.agent; +} + +/** Copilot's Agent fallback remains approval-gated for ADE supervision. */ +export function copilotSupervisionPermissionMode(mode: string | null | undefined): string | null | undefined { + return mode === "auto-edit" || mode === "auto" ? "default" : mode; +} + function normalizeCommandName(name: string): string { return name.replace(/^\/+/, "").trim().toLowerCase(); } @@ -124,9 +149,14 @@ export const copilotDialect = defineAcpDialect({ binaryNames: ["copilot"], buildSpawnPlan, - // Copilot 1.0.82 answers a `session/cancel` REQUEST with -32601. The - // notification form is the one the binary accepts, same as Grok. + // Copilot 1.0.82 answered a `session/cancel` REQUEST with -32601. The + // notification form is the compatibility-safe path, same as Grok. cancelStyle: "notification", + // `--model` and `--effort` are process global, so two chats with different + // values must not share a process. Those values are part of the pool key. + // `--model` is supported by ACP in the 1.0.86 baseline even though it is not + // a session config option. + // // `--effort` is process global, so two chats with different effort values // must not share a process. The environment carries the config home; the // effort flag is folded into the pool key by the caller through the spawn @@ -155,6 +185,7 @@ export const copilotDialect = defineAcpDialect({ }, degradationNotes: [COPILOT_CANCEL_DEGRADATION_NOTE], + degradationNoteForMode: copilotPermissionModeDegradationNote, usageSource: "usage_update", usage: capability(({ usageUpdate, promptUsage }) => { @@ -189,9 +220,12 @@ export const copilotDialect = defineAcpDialect({ resumeSession: capabilityAbsent, loadSession: capability(standardLoad), - sessionConfig: capabilityAbsent, + nativeModeValue: copilotNativeModeValue, + supervisionPermissionMode: copilotSupervisionPermissionMode, + modeSetupRequired: true, + sessionConfig: capability(standardSetConfigOption), modelSelection: capability(standardSetModel), mcpInjection: capability(transportGatedMcpInjection), imagePrompts: capability(inlineImagePrompt), - configOptionIds: [], + configOptionIds: COPILOT_CONFIG_OPTION_IDS, }); diff --git a/apps/desktop/src/main/services/chat/acpHost/acpDialects/index.ts b/apps/desktop/src/main/services/chat/acpHost/acpDialects/index.ts index fb5f68abd3..5bff23ceea 100644 --- a/apps/desktop/src/main/services/chat/acpHost/acpDialects/index.ts +++ b/apps/desktop/src/main/services/chat/acpHost/acpDialects/index.ts @@ -24,7 +24,15 @@ export function acpDialectFor(providerId: AcpProviderId): AcpDialect { } export { copilotDialect, grokDialect, kimiDialect, qwenDialect }; -export { COPILOT_TUI_ONLY_COMMANDS, includeCopilotSlashCommand } from "./copilot"; +export { + COPILOT_CONFIG_OPTION_IDS, + COPILOT_NATIVE_MODE_IDS, + COPILOT_TUI_ONLY_COMMANDS, + copilotPermissionModeDegradationNote, + copilotNativeModeValue, + copilotSupervisionPermissionMode, + includeCopilotSlashCommand, +} from "./copilot"; export { GROK_CLAUDE_MARKER_OVERRIDE_ENV, GROK_MINIMUM_VERSION, diff --git a/apps/desktop/src/main/services/chat/acpHost/acpDialects/qwen.ts b/apps/desktop/src/main/services/chat/acpHost/acpDialects/qwen.ts index a78799cacb..1638d41f7e 100644 --- a/apps/desktop/src/main/services/chat/acpHost/acpDialects/qwen.ts +++ b/apps/desktop/src/main/services/chat/acpHost/acpDialects/qwen.ts @@ -105,6 +105,7 @@ export const qwenDialect = defineAcpDialect({ loadSession: capability(standardLoad), sessionConfig: capability(standardSetConfigOption), + modeSetupRequired: true, modelSelection: capabilityAbsent, mcpInjection: capability(transportGatedMcpInjection), imagePrompts: capability(inlineImagePrompt), diff --git a/apps/desktop/src/main/services/chat/acpHost/acpHost.fixtures.test.ts b/apps/desktop/src/main/services/chat/acpHost/acpHost.fixtures.test.ts index 4b279c1ce5..05b4410f4a 100644 --- a/apps/desktop/src/main/services/chat/acpHost/acpHost.fixtures.test.ts +++ b/apps/desktop/src/main/services/chat/acpHost/acpHost.fixtures.test.ts @@ -1,10 +1,11 @@ /** * Dialect claims vs captured initialize responses from real binaries. * - * (ACP agent 1.0.4), Grok 1.0.13, and the Kimi Code 0.39.1 compatibility - * baseline. Kimi Code 2.0.0's current ACP reference is covered by the dialect - * contract assertions below. Qwen Code 0.24.0 was captured separately on - * 2026-09-18. + * These fixtures were recorded on 2026-09-18 against Copilot CLI 1.0.86, + * ACP agent 1.0.4, Grok 1.0.13, Qwen Code 0.24.0, and the Kimi Code 0.39.1 + * compatibility baseline. Kimi Code 2.0.0's current ACP reference is covered + * by the dialect contract assertions below. + * Qwen Code 0.24.0 was captured separately on 2026-09-18. */ import { readFileSync } from "node:fs"; import path from "node:path"; @@ -19,21 +20,23 @@ function loadFixture(name: string): T { } describe("captured initialize fixtures", () => { - it("copilot 1.0.82 advertises loadSession and image, not close or resume", () => { + it("copilot 1.0.86 advertises load, close, MCP, and image, not resume", () => { const init = loadFixture("copilot.initialize.json"); expect(init.protocolVersion).toBe(1); + expect(init.agentInfo?.version).toBe("1.0.86"); expect(init.agentCapabilities?.loadSession).toBe(true); + expect(init.agentCapabilities?.mcpCapabilities).toEqual({ http: true, sse: true }); expect(init.agentCapabilities?.promptCapabilities?.image).toBe(true); - expect(init.agentCapabilities?.sessionCapabilities?.list).toEqual({}); - expect(init.agentCapabilities?.sessionCapabilities).not.toHaveProperty("close"); + expect(init.agentCapabilities?.sessionCapabilities).toMatchObject({ close: {}, list: {} }); expect(init.agentCapabilities?.sessionCapabilities).not.toHaveProperty("resume"); - // ADE still declares close and degrades on -32601 rather than killing the - // process (Copilot can host more than one session). Resume stays unclaimed. + expect(copilotDialect.sessionConfig.declared).toBe(true); + expect(copilotDialect.configOptionIds).toEqual(["mode", "allow_all"]); expect(copilotDialect.closeStyle).toBe("close_request"); expect(copilotDialect.loadPolicy).toBe("load_only"); expect(copilotDialect.resumeSession.declared).toBe(false); expect(copilotDialect.cancelStyle).toBe("notification"); expect(copilotDialect.imagePrompts.declared).toBe(true); + expect(copilotDialect.mcpInjection.declared).toBe(true); }); it("grok remains first-class while preserving its honest capability gates", () => { @@ -117,6 +120,7 @@ describe("captured initialize fixtures", () => { expect(mode?.options?.map((entry) => entry.id)).toEqual([ "https://agentclientprotocol.com/protocol/session-modes#agent", "https://agentclientprotocol.com/protocol/session-modes#plan", + "https://agentclientprotocol.com/protocol/session-modes#autopilot", ]); expect(options.find((option) => option.id === "allow_all")?.value).toBe("off"); }); diff --git a/apps/desktop/src/main/services/chat/acpHost/acpHost.test.ts b/apps/desktop/src/main/services/chat/acpHost/acpHost.test.ts index 0d6ecca0b4..9cef7214da 100644 --- a/apps/desktop/src/main/services/chat/acpHost/acpHost.test.ts +++ b/apps/desktop/src/main/services/chat/acpHost/acpHost.test.ts @@ -31,6 +31,9 @@ import { GROK_CLAUDE_MARKER_OVERRIDE_ENV, GROK_SESSION_NOTIFICATION_METHOD, GROK_YOLO_MODE_CHANGED_METHOD, + copilotPermissionModeDegradationNote, + copilotNativeModeValue, + copilotSupervisionPermissionMode, includeCopilotSlashCommand, KIMI_CONFIG_OPTION_IDS, kimiDialect, @@ -345,16 +348,46 @@ describe("spawn plans", () => { expect(plan.env.COPILOT_HOME).toBe("/home/.copilot"); }); - it("passes the selected model to Copilot's ACP process", () => { - const plan = copilotDialect.buildSpawnPlan({ + it("copilot passes the selected model and effort as process-global ACP flags", () => { + const context = { binaryPath: "/bin/copilot", cwd: "/lane/worktree", baseEnv: {}, modelId: "github-copilot/gpt-5.4", - }); + reasoningEffort: "high", + }; + const plan = copilotDialect.buildSpawnPlan(context); + const planWithoutModel = copilotDialect.buildSpawnPlan({ ...context, modelId: undefined }); + expect(plan.args).toEqual(expect.arrayContaining(["--model", "gpt-5.4", "--effort", "high"])); + expect(hashSpawnInvocation(plan)).not.toBe(hashSpawnInvocation(planWithoutModel)); + }); + + it.each([ + ["plan", "https://agentclientprotocol.com/protocol/session-modes#plan"], + ["default", "https://agentclientprotocol.com/protocol/session-modes#agent"], + ["auto-edit", "https://agentclientprotocol.com/protocol/session-modes#agent"], + ["auto", "https://agentclientprotocol.com/protocol/session-modes#agent"], + ["yolo", "https://agentclientprotocol.com/protocol/session-modes#autopilot"], + ] as const)("copilot maps ADE %s to an honest 1.0.86 ACP mode", (mode, expected) => { + expect(copilotNativeModeValue(mode)).toBe(expected); + }); - expect(plan.args).toContain("--model"); - expect(plan.args[plan.args.indexOf("--model") + 1]).toBe("gpt-5.4"); + it.each(["plan", "default", "yolo", null])("copilot only warns about an autonomy downgrade for auto modes (%s)", (mode) => { + expect(copilotPermissionModeDegradationNote(mode)).toBeNull(); + }); + + it.each(["auto-edit", "auto"])("copilot explains its %s downgrade", (mode) => { + expect(copilotPermissionModeDegradationNote(mode)).toContain("approval-gated Agent mode"); + }); + + it.each([ + ["plan", "plan"], + ["default", "default"], + ["auto-edit", "default"], + ["auto", "default"], + ["yolo", "yolo"], + ] as const)("copilot supervises %s as %s", (mode, expected) => { + expect(copilotSupervisionPermissionMode(mode)).toBe(expected); }); // ADE removed its Copilot trust pre-seed: a live three-arm experiment on @@ -565,6 +598,28 @@ describe("session entry policy", () => { expect(plan.suppressReplay).toBe(false); }); + it("falls back to load when the agent handshake omits resume", () => { + const plan = resolveAcpSessionEntry({ + dialect: qwenDialect, + existingSessionId: "s1", + adeHasTranscript: true, + agentCapabilities: { loadSession: true, sessionCapabilities: { list: {} } }, + }); + expect(plan.mode).toBe("load"); + expect(plan.suppressReplay).toBe(true); + }); + + it("starts a fresh session when the agent handshake omits rejoin support", () => { + const plan = resolveAcpSessionEntry({ + dialect: copilotDialect, + existingSessionId: "s1", + adeHasTranscript: true, + agentCapabilities: { sessionCapabilities: { list: {} } }, + }); + expect(plan.mode).toBe("new"); + expect(plan.suppressReplay).toBe(false); + }); + it("suppresses the load replay when ADE already holds the transcript", () => { const plan = resolveAcpSessionEntry({ dialect: copilotDialect, @@ -1012,7 +1067,7 @@ describe("session config", () => { expect([...kimiDialect.configOptionIds]).toEqual(["mode", "model", "thinking"]); }); - it.each(["grok", "copilot"] as const)( + it.each(["grok"] as const)( "%s refuses a config option instead of sending a call it does not support", async (providerId) => { const harness = makeHarness(acpDialectFor(providerId)); @@ -1035,6 +1090,18 @@ describe("session config", () => { params: { sessionId: "session-1", modelId: "gpt-5.4" }, }); }); + + it("copilot accepts its native mode config option", async () => { + const harness = makeHarness(copilotDialect); + harness.agent.on(ACP_METHOD.sessionSetConfigOption, () => ({ result: {} })); + const session = await withDeadline("open", harness.open()); + await withDeadline("set", session.setConfigOption({ + configId: "mode", + value: "https://agentclientprotocol.com/protocol/session-modes#plan", + })); + expect(harness.agent.received.find((entry) => entry.method === ACP_METHOD.sessionSetConfigOption)?.params) + .toMatchObject({ configId: "mode", value: "https://agentclientprotocol.com/protocol/session-modes#plan" }); + }); }); // ───────────────────────────────────────────────────────────────────────────── @@ -1326,6 +1393,17 @@ describe("unsupervised session invariant", () => { expect(notices(harness)).toHaveLength(0); }); + it("treats Copilot's auto-edit downgrade as approval-gated", async () => { + const harness = makeHarness(copilotDialect); + writingTurn(harness); + const session = await withDeadline("open", harness.open({ permissionMode: "auto-edit" })); + await withDeadline("turn", session.prompt({ turnId: "t1", blocks: [textPromptBlock("go")] })); + expect(notices(harness)).toHaveLength(1); + expect(notices(harness)[0]).toMatchObject({ + message: "GitHub Copilot changed files here without asking ADE to approve. ADE's approval cards can't gate this chat.", + }); + }); + it("stays silent for a read-only turn, because reads never prompt anywhere", async () => { const harness = makeHarness(grokDialect); writingTurn(harness, "read"); @@ -1607,8 +1685,18 @@ describe("close and eviction", () => { const harness = makeHarness(copilotDialect); const session = await withDeadline("open", harness.open()); await withDeadline("close", session.close("chat ended")); - // Copilot 1.0.82 answers -32601. Degraded, not thrown. The pooled process - // stays usable for other chats. + // Older Copilot ACP builds can answer -32601. Degraded, not thrown. The + // pooled process stays usable for other chats. + expect(session.connection.isAlive()).toBe(true); + }); + + it("does not send close when an older Copilot handshake omits it", async () => { + const harness = makeHarness(copilotDialect, { + agentCapabilities: { loadSession: true, sessionCapabilities: { list: {} } }, + }); + const session = await withDeadline("open", harness.open()); + await withDeadline("close", session.close("chat ended")); + expect(harness.agent.methodsReceived()).not.toContain(ACP_METHOD.sessionClose); expect(session.connection.isAlive()).toBe(true); }); }); diff --git a/apps/desktop/src/main/services/chat/acpHost/acpHostTypes.ts b/apps/desktop/src/main/services/chat/acpHost/acpHostTypes.ts index b7a3594a47..ea2e6d71ac 100644 --- a/apps/desktop/src/main/services/chat/acpHost/acpHostTypes.ts +++ b/apps/desktop/src/main/services/chat/acpHost/acpHostTypes.ts @@ -68,9 +68,9 @@ export function behaviorOf(entry: AcpCapability): TBehavio /** * How to stop a running turn. * - * Grok, and Copilot 1.0.82, answer a `session/cancel` REQUEST with -32601. - * They accept the same call as a notification. Qwen and Kimi accept the - * request form. + * Grok and Copilot's ACP server answer a `session/cancel` REQUEST with -32601 + * on the compatibility baseline. They accept the same call as a notification. + * Qwen and Kimi accept the request form. */ export type AcpCancelStyle = "request" | "notification"; @@ -283,6 +283,15 @@ export type AcpDialectBase = { /** Build the process spawn plan. Pure: no file system reads, no spawns. */ readonly buildSpawnPlan: (context: AcpSpawnContext) => AcpSpawnPlan; + /** Map ADE's abstract mode to the provider's native config value. */ + readonly nativeModeValue?: (mode: string) => string; + + /** Map ADE's requested mode to the posture the supervision guard should enforce. */ + readonly supervisionPermissionMode?: (mode: string | null | undefined) => string | null | undefined; + + /** Whether failure to apply the native mode must abort runtime setup. */ + readonly modeSetupRequired?: boolean; + readonly cancelStyle: AcpCancelStyle; /** @@ -348,6 +357,9 @@ export type AcpDialectBase = { */ readonly degradationNotes: readonly string[]; + /** Optional mode-specific degradation note, emitted only for that mode. */ + readonly degradationNoteForMode?: (permissionMode: string | null | undefined) => string | null; + /** Optional capabilities. Present ones carry their behavior. */ readonly sessionConfig: AcpCapability; readonly modelSelection: AcpCapability; diff --git a/apps/desktop/src/main/services/chat/acpHost/acpProtocolTypes.ts b/apps/desktop/src/main/services/chat/acpHost/acpProtocolTypes.ts index 19b52eeefd..3531b56b80 100644 --- a/apps/desktop/src/main/services/chat/acpHost/acpProtocolTypes.ts +++ b/apps/desktop/src/main/services/chat/acpHost/acpProtocolTypes.ts @@ -209,7 +209,7 @@ export type AcpSessionConfigOption = { }; /** - * Copilot 1.0.82 (and possibly other agents) send `currentValue` instead of + * Copilot ACP (and possibly other agents) sends `currentValue` instead of * `value`, and nested choices as `{ value, name }` instead of `{ id, name }`. * Canonicalize onto ADE's `value` / `options[].id` shape so a live snapshot * does not land as "no current mode". @@ -570,6 +570,20 @@ export type AcpAgentCapabilities = { _meta?: AcpMeta; }; +/** ACP session capabilities are presence-based: `{}` advertises a method. */ +export function hasAcpSessionCapability( + agentCapabilities: AcpAgentCapabilities | null | undefined, + capability: keyof AcpSessionCapabilities, +): boolean { + const sessionCapabilities = agentCapabilities?.sessionCapabilities; + return sessionCapabilities != null + && Object.prototype.hasOwnProperty.call(sessionCapabilities, capability); +} + +export function hasAcpLoadSessionCapability(agentCapabilities: AcpAgentCapabilities | null | undefined): boolean { + return agentCapabilities?.loadSession === true; +} + export type AcpAuthMethod = { id: string; name: string; diff --git a/apps/desktop/src/main/services/chat/acpHost/acpRuntimeCoordinator.test.ts b/apps/desktop/src/main/services/chat/acpHost/acpRuntimeCoordinator.test.ts new file mode 100644 index 0000000000..757da7a44f --- /dev/null +++ b/apps/desktop/src/main/services/chat/acpHost/acpRuntimeCoordinator.test.ts @@ -0,0 +1,149 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { copilotDialect, kimiDialect } from "./acpDialects"; +import type { Logger } from "../../logging/logger"; + +const openAcpSessionMock = vi.hoisted(() => vi.fn()); + +vi.mock("./acpSession", () => ({ openAcpSession: openAcpSessionMock })); + +import { createAcpRuntime } from "./acpRuntimeCoordinator"; + +describe("createAcpRuntime", () => { + beforeEach(() => { + openAcpSessionMock.mockReset(); + }); + + it("fails closed when the requested mode cannot be applied", async () => { + const modeError = new Error("session/set_config_option unavailable"); + const session = { + providerId: "copilot", + dialect: copilotDialect, + sessionId: "acp-session-1", + entryPlan: { mode: "new", suppressReplay: false, reason: "test" }, + connection: { isAlive: () => true, initializeResult: null }, + initialConfigOptions: [], + initialModeId: null, + unsupervised: false, + prompt: vi.fn(), + cancel: vi.fn(), + setConfigOption: vi.fn().mockRejectedValue(modeError), + close: vi.fn().mockResolvedValue(undefined), + }; + openAcpSessionMock.mockResolvedValue(session); + + const onRuntimeCreated = vi.fn(); + const onRuntimeSetupFailed = vi.fn(); + const onOpenFailed = vi.fn(); + const onReady = vi.fn(); + + await expect(createAcpRuntime({ + owner: { + session: { id: "chat-1" } as never, + laneWorktreePath: "/lane/worktree", + eventSequence: 0, + transcriptBytesWritten: 0, + }, + provider: "copilot", + dialect: copilotDialect, + spawnPlan: { command: "copilot", args: ["--acp"], env: {}, cwd: "/lane/worktree" }, + invocationKey: "invocation-1", + permissionMode: "plan", + modelToken: null, + existingSessionId: null, + supervisionPreflight: null, + supervisionAlreadyNotified: false, + logger: { warn: vi.fn(), info: vi.fn() } as unknown as Logger, + runtimeBudget: { enforce: vi.fn() }, + existingRuntime: null, + runtimeInvalidated: false, + hasExistingRuntime: false, + teardownExistingRuntime: vi.fn(), + nativeModeValue: "plan", + reasoningEffort: null, + setResumeCommand: vi.fn(), + binarySource: "test", + callbacks: { + onEvents: vi.fn(), + onPermissionRequested: vi.fn(), + onPermissionSettled: vi.fn(), + onSlashCommands: vi.fn(), + onConfigOptions: vi.fn(), + onSessionInfo: vi.fn(), + onProcessExit: vi.fn(), + onRuntimeCreated, + onRuntimeSetupFailed, + onOpenFailed, + onReady, + }, + })).rejects.toBe(modeError); + + const runtime = onRuntimeCreated.mock.calls[0]?.[0]; + expect(session.close).toHaveBeenCalledWith("mode setup failed"); + expect(onRuntimeSetupFailed).toHaveBeenCalledWith(runtime, modeError); + expect(onOpenFailed).toHaveBeenCalledWith(modeError); + expect(onReady).not.toHaveBeenCalled(); + }); + + it("degrades when an older dialect cannot apply its optional mode", async () => { + const modeError = new Error("session/set_config_option unavailable"); + const session = { + providerId: "kimi", + dialect: kimiDialect, + sessionId: "acp-session-1", + entryPlan: { mode: "new", suppressReplay: false, reason: "test" }, + connection: { isAlive: () => true, initializeResult: null }, + initialConfigOptions: [], + initialModeId: null, + unsupervised: false, + prompt: vi.fn(), + cancel: vi.fn(), + setConfigOption: vi.fn().mockRejectedValue(modeError), + close: vi.fn().mockResolvedValue(undefined), + }; + openAcpSessionMock.mockResolvedValue(session); + const onReady = vi.fn(); + + await expect(createAcpRuntime({ + owner: { + session: { id: "chat-1" } as never, + laneWorktreePath: "/lane/worktree", + eventSequence: 0, + transcriptBytesWritten: 0, + }, + provider: "kimi", + dialect: kimiDialect, + spawnPlan: { command: "kimi", args: ["acp"], env: {}, cwd: "/lane/worktree" }, + invocationKey: "invocation-1", + permissionMode: "plan", + modelToken: null, + existingSessionId: null, + supervisionPreflight: null, + supervisionAlreadyNotified: false, + logger: { warn: vi.fn(), info: vi.fn() } as unknown as Logger, + runtimeBudget: { enforce: vi.fn() }, + existingRuntime: null, + runtimeInvalidated: false, + hasExistingRuntime: false, + teardownExistingRuntime: vi.fn(), + nativeModeValue: "plan", + reasoningEffort: null, + setResumeCommand: vi.fn(), + binarySource: "test", + callbacks: { + onEvents: vi.fn(), + onPermissionRequested: vi.fn(), + onPermissionSettled: vi.fn(), + onSlashCommands: vi.fn(), + onConfigOptions: vi.fn(), + onSessionInfo: vi.fn(), + onProcessExit: vi.fn(), + onRuntimeCreated: vi.fn(), + onOpenFailed: vi.fn(), + onReady, + }, + })).resolves.toBeDefined(); + + expect(session.close).not.toHaveBeenCalled(); + expect(onReady).toHaveBeenCalledOnce(); + }); +}); diff --git a/apps/desktop/src/main/services/chat/acpHost/acpRuntimeCoordinator.ts b/apps/desktop/src/main/services/chat/acpHost/acpRuntimeCoordinator.ts index d2b924f76d..d4298f92ea 100644 --- a/apps/desktop/src/main/services/chat/acpHost/acpRuntimeCoordinator.ts +++ b/apps/desktop/src/main/services/chat/acpHost/acpRuntimeCoordinator.ts @@ -79,6 +79,8 @@ export type AcpRuntimeCoordinatorCallbacks = { ) => void; /** Assign the runtime to the owning chat before session config is applied. */ onRuntimeCreated: (runtime: AcpRuntimeState) => void; + /** Remove a runtime whose required mode setup failed before readiness. */ + onRuntimeSetupFailed?: (runtime: AcpRuntimeState, error: unknown) => void; /** Record an open failure before it is returned to the chat service. */ onOpenFailed: (error: unknown) => void; /** Persist and publish the provider-ready state after the session is ready. */ @@ -237,15 +239,30 @@ export async function createAcpRuntime( openPermissionIds: new Set(), }; args.callbacks.onRuntimeCreated(runtime); + const createdRuntime = runtime as AcpRuntimeState; if (args.dialect.sessionConfig.declared) { - await session.setConfigOption({ configId: "mode", value: args.nativeModeValue }).catch((error) => { + const nativeModeValue = args.dialect.nativeModeValue?.(args.nativeModeValue) ?? args.nativeModeValue; + try { + await session.setConfigOption({ configId: "mode", value: nativeModeValue }); + } catch (error) { args.logger.warn("agent_chat.acp_set_mode_failed", { sessionId: args.owner.session.id, provider: args.provider, error: error instanceof Error ? error.message : String(error), }); - }); + if (args.dialect.modeSetupRequired) { + // A failed mode setup must never fall through to onReady: the agent may + // now be running with a broader posture than the user selected. + try { + await session.close("mode setup failed"); + } finally { + args.callbacks.onRuntimeSetupFailed?.(createdRuntime, error); + args.callbacks.onOpenFailed(error); + } + throw error; + } + } } if (args.modelToken) { const modelBehavior = behaviorOf(args.dialect.modelSelection); @@ -254,7 +271,7 @@ export async function createAcpRuntime( const call = modelBehavior({ sessionId: session.sessionId, modelId: args.modelToken! }); await session.connection.request(call.method, call.params); } - : args.dialect.sessionConfig.declared + : args.dialect.sessionConfig.declared && args.dialect.configOptionIds.includes("model") ? async () => session.setConfigOption({ configId: "model", value: args.modelToken! }) : null; if (setModel) { @@ -301,6 +318,8 @@ export async function createAcpRuntime( sessionId: args.owner.session.id, provider: args.provider, acpSessionId: session.sessionId, + agentVersion: session.connection.initializeResult?.agentInfo?.version ?? null, + advertisedSessionCapabilities: session.connection.initializeResult?.agentCapabilities?.sessionCapabilities ?? null, entryMode: session.entryPlan.mode, entryReason: session.entryPlan.reason, binarySource: args.binarySource, diff --git a/apps/desktop/src/main/services/chat/acpHost/acpSession.ts b/apps/desktop/src/main/services/chat/acpHost/acpSession.ts index 414b227feb..5087b2a3e0 100644 --- a/apps/desktop/src/main/services/chat/acpHost/acpSession.ts +++ b/apps/desktop/src/main/services/chat/acpHost/acpSession.ts @@ -58,7 +58,10 @@ import { } from "./acpSupervisionGuard"; import { ACP_METHOD, + hasAcpLoadSessionCapability, + hasAcpSessionCapability, normalizeAcpConfigOptions, + type AcpAgentCapabilities, type AcpContentBlock, type AcpMcpServer, type AcpNewSessionResponse, @@ -92,6 +95,8 @@ export function resolveAcpSessionEntry(args: { dialect: AcpDialect; existingSessionId: string | null; adeHasTranscript: boolean; + /** Handshake capabilities. Omit only for dialect-only planning tests. */ + agentCapabilities?: AcpAgentCapabilities | null; }): AcpSessionEntryPlan { if (!args.existingSessionId) { return { mode: "new", suppressReplay: false, reason: "no stored session id" }; @@ -99,9 +104,26 @@ export function resolveAcpSessionEntry(args: { if (args.dialect.loadPolicy === "never") { return { mode: "new", suppressReplay: false, reason: "dialect cannot rejoin a session" }; } - if (args.dialect.loadPolicy === "resume_preferred" && args.dialect.resumeSession.declared) { + const agentSupportsResume = args.agentCapabilities === undefined + ? args.dialect.resumeSession.declared + : hasAcpSessionCapability(args.agentCapabilities, "resume"); + const agentSupportsLoad = args.agentCapabilities === undefined + ? args.dialect.loadSession.declared + : hasAcpLoadSessionCapability(args.agentCapabilities); + if ( + args.dialect.loadPolicy === "resume_preferred" + && args.dialect.resumeSession.declared + && agentSupportsResume + ) { return { mode: "resume", suppressReplay: false, reason: "agent advertises session/resume" }; } + if (!args.dialect.loadSession.declared || !agentSupportsLoad) { + return { + mode: "new", + suppressReplay: false, + reason: "agent does not advertise a supported session rejoin method", + }; + } return { mode: "load", suppressReplay: args.adeHasTranscript, @@ -239,9 +261,12 @@ export async function openAcpSession(args: OpenAcpSessionArgs): Promise { expect(statuses).toContain("completed"); }); + it("configures Copilot's native ACP mode without sending an unsupported model option", async () => { + const harness = await openAcpHarness({ + provider: "copilot", + model: "claude-sonnet-4.6", + modelId: "github-copilot/claude-sonnet-4.6", + sessionOverrides: { permissionMode: "plan" }, + }); + scriptPrompt(harness.agent, []); + + await harness.service.sendMessage({ sessionId: harness.session.id, text: "plan this" }); + await vi.waitFor(() => { + expect(eventTypes(harness)).toContain("done"); + }); + + const configCalls = harness.agent.received.filter((entry) => entry.method === "session/set_config_option"); + expect(configCalls).toHaveLength(1); + expect(configCalls[0]?.params).toMatchObject({ + configId: "mode", + value: "https://agentclientprotocol.com/protocol/session-modes#plan", + }); + expect(configCalls.some((entry) => (entry.params as { configId?: string }).configId === "model")).toBe(false); + }); + it("applies Qwen's selected model and reasoning effort at session startup", async () => { const harness = await openAcpHarness({ provider: "qwen", diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index a09d9fcd7e..7d5bf9c55f 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -805,6 +805,7 @@ import { type AcpSlashCommand, type AcpRuntimeState, } from "./acpHost"; +import { COPILOT_NPM_PACKAGE_SPEC } from "../../../shared/acpProviderMetadata"; import { copilotConfigHome, grokConfigHome, @@ -16472,7 +16473,7 @@ export function createAgentChatService(args: { qwen: "npm install -g @qwen-code/qwen-code", kimi: "curl -LsSf https://code.kimi.com/kimi-code/install.sh | bash", grok: "npm install -g @xai-official/grok@1.0.34", - copilot: "npm install -g @github/copilot", + copilot: `npm install -g ${COPILOT_NPM_PACKAGE_SPEC}`, }; /** @@ -26940,13 +26941,13 @@ export function createAgentChatService(args: { mode: AgentChatAcpPermissionMode, ): boolean => { if (mode !== "yolo") return false; - // Qwen and Kimi take the whole posture through + // Qwen, Kimi, and Copilot take the whole posture through // `session/set_config_option`, so the agent stops asking and there is // nothing for ADE to auto-answer. return !dialect.sessionConfig.declared; }; - /** Native `mode` value for a dialect that accepts one. Qwen's ladder. */ + /** ADE's abstract value; the dialect maps it to the native wire value. */ const acpNativeModeValue = (mode: AgentChatAcpPermissionMode): string => mode; /** Provider-native model token for the spawn plan, when the user picked one. */ @@ -26963,15 +26964,24 @@ export function createAgentChatService(args: { * usage, and a user who cannot see why the meter vanished assumes ADE broke. * The shown-set is persisted so a runtime restart does not repeat it. */ - const emitAcpDegradationNotes = (managed: ManagedChatSession, dialect: AcpDialect): void => { - if (!dialect.degradationNotes.length) return; + const emitAcpDegradationNotes = ( + managed: ManagedChatSession, + dialect: AcpDialect, + permissionMode?: string | null, + ): void => { + const modeNote = dialect.degradationNoteForMode?.(permissionMode); + const notes = [ + ...dialect.degradationNotes, + ...(modeNote ? [modeNote] : []), + ]; + if (!notes.length) return; if (!managed.acpDegradationNotesShown) { managed.acpDegradationNotesShown = new Set( readPersistedState(managed.session.id)?.acpDegradationNotesShown ?? [], ); } let emitted = false; - for (const note of dialect.degradationNotes) { + for (const note of notes) { if (managed.acpDegradationNotesShown.has(note)) continue; managed.acpDegradationNotesShown.add(note); emitted = true; @@ -27284,6 +27294,12 @@ export function createAgentChatService(args: { managed.seededAcpSessionId = runtime.session.sessionId; managed.session.acpPermissionMode = permissionMode; }, + onRuntimeSetupFailed: (runtime) => { + if (managed.runtime !== runtime) return; + managed.runtime = null; + managed.runtimeInvalidated = true; + managed.seededAcpSessionId = undefined; + }, onOpenFailed: (error) => { const message = error instanceof Error ? error.message : String(error); recordAcpAuthProbeResult( @@ -27348,7 +27364,7 @@ export function createAgentChatService(args: { return; } - emitAcpDegradationNotes(managed, runtime.dialect); + emitAcpDegradationNotes(managed, runtime.dialect, runtime.permissionMode); runtime.busy = true; runtime.activeTurnId = turnId; diff --git a/apps/desktop/src/renderer/components/settings/providers/acpProviders.tsx b/apps/desktop/src/renderer/components/settings/providers/acpProviders.tsx index 94a40fee70..cf3fbabd23 100644 --- a/apps/desktop/src/renderer/components/settings/providers/acpProviders.tsx +++ b/apps/desktop/src/renderer/components/settings/providers/acpProviders.tsx @@ -11,7 +11,7 @@ import React from "react"; import { COLORS, MONO_FONT, SANS_FONT, outlineButton } from "../../lanes/laneDesignTokens"; import { ProviderLogo } from "../../shared/ProviderLogos"; import { listModelDescriptorsForProvider, providerTierIsPreview } from "../../../../shared/modelRegistry"; -import { ACP_PROVIDER_METADATA } from "../../../../shared/acpProviderMetadata"; +import { ACP_PROVIDER_METADATA, COPILOT_NPM_PACKAGE_SPEC } from "../../../../shared/acpProviderMetadata"; import { CopyableCommand, SubsectionTitle } from "./providerUi"; import type { AcpSettingsProviderId, @@ -83,7 +83,7 @@ export const ACP_PROVIDER_SPECS: readonly AcpProviderSpec[] = [ id: "copilot", tagline: "Uses your GitHub account through the copilot CLI.", logoFamily: "github-copilot", - installCommand: "npm install -g @github/copilot", + installCommand: `npm install -g ${COPILOT_NPM_PACKAGE_SPEC}`, credentialSource: "Signed in through `copilot login`; the free plan includes the CLI. ADE does not write ~/.copilot.", setup: "Install the Copilot CLI and run `copilot login`. ADE reuses that GitHub login and never writes Copilot's config.json. Cancelled turns can still look finished on Copilot's side; ADE marks them stopped.", }, diff --git a/apps/desktop/src/shared/acpProviderMetadata.ts b/apps/desktop/src/shared/acpProviderMetadata.ts index 0c36234dca..1d64465b52 100644 --- a/apps/desktop/src/shared/acpProviderMetadata.ts +++ b/apps/desktop/src/shared/acpProviderMetadata.ts @@ -7,6 +7,15 @@ export const ACP_PROVIDER_IDS = ["qwen", "kimi", "grok", "copilot"] as const; export type AcpProviderId = (typeof ACP_PROVIDER_IDS)[number]; +/** + * Copilot ACP compatibility baseline validated against the live CLI. + * + * ACP is still a public preview in Copilot CLI, so this is a tested baseline + * rather than a promise that every future vendor release is wire-compatible. + */ +export const COPILOT_ACP_COMPATIBILITY_BASELINE = "1.0.86" as const; +export const COPILOT_NPM_PACKAGE_SPEC = `@github/copilot@${COPILOT_ACP_COMPATIBILITY_BASELINE}` as const; + export type AcpProviderMetadata = { readonly label: string; readonly statusLabel: string; diff --git a/docs/features/chat/acp-providers-spec.md b/docs/features/chat/acp-providers-spec.md index a0fb70079d..f716b6837f 100644 --- a/docs/features/chat/acp-providers-spec.md +++ b/docs/features/chat/acp-providers-spec.md @@ -281,24 +281,36 @@ Rust, Apache-2.0) Markdown heading theme colors without changing the ACP launch contract. ADE's setup/error copy recommends `@xai-official/grok@1.0.34` for this baseline. -### Copilot (`copilot --acp`, npm `@github/copilot`, PREVIEW) -- Caps on 1.0.82 (ACP agent 1.0.4): `loadSession`, image prompts, session - list. `session/resume` and `session/close` are **not** advertised and - answer -32601. Slash as ordinary prompts + `available_commands_update`; - TUI-only commands (`/diff`, `/resume`, `/login`, `/undo`…) must be filtered - from the picker or they hit the model. -- KNOWN BUG: `session/cancel` as a REQUEST answers -32601. Send it as a - notification. Live 1.0.82 cancel mid-count returned `stopReason:"end_turn"` - with partial text `"1\n2\n3\n4\n5"` (github/copilot-cli #4561) → client-side - cancel accounting is mandatory. ADE still attempts `session/close` and - degrades, keeping the process for pooling. Real `session/prompt` turns work - (`"ping"`, usage on the prompt result + `usage_update`). ACP model selection - is not a `session/new` config option: ADE uses Copilot's native - `session/set_model` request when the installed CLI supports it. Older ACP - builds accepted that request without changing inference and stayed on Auto, - so the runtime must tolerate a provider-side fallback. Config options use - `currentValue` and nested `value`, which ADE canonicalizes onto `value` / - `options[].id`. +### Copilot (`copilot --acp`, npm `@github/copilot@1.0.86`, PREVIEW) +- The 1.0.86 compatibility baseline (ACP agent 1.0.86, captured + 2026-09-18) advertises `loadSession`, image prompts, HTTP/SSE MCP, and + session list/close. It does not advertise `session/resume`. ADE checks the + handshake before sending lifecycle methods, and older 1.0.x binaries that + omit close release a shared lease without killing other chats. +- ACP mode controls are live: `agent`, `plan`, and `autopilot`, plus the + `allow_all` option. ADE maps its abstract permission ladder to those native + mode ids and normalizes Copilot's `currentValue` / nested `value` shape. + Copilot has no intermediate auto-edit mode, so ADE deliberately maps + `auto-edit` and `auto` down to approval-gated Agent mode and tells the user + about that downgrade. +- Slash commands arrive as ordinary prompts plus `available_commands_update`; + TUI-only commands (`/diff`, `/resume`, `/login`, `/undo`…) are filtered from + the picker or they hit the model. +- KNOWN BUG: `session/cancel` as a REQUEST answers -32601 on the observed + compatibility path. Send it as a notification. Historical live 1.0.82 + cancellation returned `stopReason:"end_turn"` with partial text + `"1\n2\n3\n4\n5"` (github/copilot-cli #4561), so client-side cancel accounting + remains mandatory until GitHub documents a fix. +- `--model` and `--effort` are process-global ACP launch flags. ADE passes the + selected model and effort at launch and folds both into the pool identity; + `session/new` cannot override them. Usage arrives on the prompt result and + `usage_update`. +- ACP model selection is not a `session/new` config option: ADE passes the + selected model at launch and uses Copilot's native `session/set_model` when + that method is advertised. Older ACP builds accepted that request without + changing inference and stayed on Auto, so the runtime tolerates a + provider-side fallback. Config options use `currentValue` and nested + `value`, which ADE canonicalizes onto `value` / `options[].id`. - Server-start flags (`--effort`, `--available-tools`, `--excluded-tools`) are process-global; `session/new` cannot override. - **Trust pre-seed: REMOVED. ADE does not write Copilot's config.** There was diff --git a/docs/features/chat/acp-verification-brief.md b/docs/features/chat/acp-verification-brief.md index 7581d9242f..0a898013b3 100644 --- a/docs/features/chat/acp-verification-brief.md +++ b/docs/features/chat/acp-verification-brief.md @@ -81,6 +81,12 @@ verified once, on one version. Re-verify what you can and flag what you cannot: (camelCase — not the `trusted_folders` older notes claimed). ADE writes neither: the trust pre-seed is removed and nothing on the Copilot path may write `$COPILOT_HOME` again. +- Copilot 1.0.86 ACP: `loadSession`, image prompts, HTTP/SSE MCP, and + `session/close` are advertised; `session/resume` is absent. `config.json` is + JSONC; older live 1.0.82 persisted `trustedFolders` (camelCase — not the + `trusted_folders` older notes claimed). ADE writes neither: the trust + pre-seed is removed and nothing on the Copilot path may write `$COPILOT_HOME` + again. ACP mode options include agent, plan, and autopilot. Headless ACP `session/new` did not deadlock without a seed or `--add-dir`. Cwd writes emit 0 `session/request_permission` with `allow_all` off.