feat: GitHub Copilot provider via ACP - #7885
Conversation
CopilotSettings mirrors the other opt-in CLI providers (enabled flag, binaryPath, customModels) and wires into ServerSettings.providers and the settings patch map so instances can be configured from Settings.
…tarting agents Three capabilities the Copilot ACP binding needs, all provider-neutral: - session/update available_commands_update now parses into an AvailableCommandsChanged event (skills arrive as slash commands). - Session updates that land before startup settles are buffered and re-dispatched to the root session instead of dropped; Copilot advertises commands immediately after session/new. The load-replay idle gate keeps receiving touches while buffered. - First-sighting tool calls that carry rawInput are no longer suppressed until detail arrives, so subagent launches announce themselves; input-less placeholders stay suppressed. An empty authMethodId skips the authenticate round-trip for agents that auth outside ACP.
…asks Three opt-in flags for the shared mock agent: emit available_commands_update after session/new, a foreground task tool call, and the copilot-cli background-agent shape (launch, early end_turn, then post-turn chunks plus an idle report).
Spawn input for `copilot --acp`, a runtime factory that skips ACP authenticate (GitHub login/BYOK happen outside the protocol), and model selection helpers mirroring the Grok binding.
Turn loop follows the Grok ACP adapter (prompt steering, approvals, cancel) minus the xAI quirks. Two Copilot-specific behaviors: - task tool calls (rawInput.agent_type) emit task.started/progress/ completed with agentKind "agent" so they render on the Agents surface instead of opaque tool rows. - Background launches (mode: "background") park the turn settlement: copilot-cli answers end_turn early and keeps streaming progress, so the turn stays routable until a follow-up call reports the agent idle; then task.completed flushes the parked turn.completed. Stop cancels outright; a new sendTurn closes leftovers as stopped.
…P probe Status probe runs `copilot --version` then one short-lived ACP session that collects the model list from session setup and skills from available_commands_update. Skills map onto ServerProviderSkill with the invocation (`/name`) as path since upstream exposes no skill files.
CopilotDriver wires adapter, snapshot maintenance and text generation; text generation intentionally runs on the session's current model instead of pinning one. Driver added to BUILT_IN_DRIVERS.
Mock-agent driven: basic prompt streaming, subagent tool calls emitting task events (no double render), background flow holding the turn open until idle, and skills collection from available_commands_update.
Picker entry, browser-safe driver metadata with a preview badge, GitHub mark icons for web and mobile.
Built-in driver table gains copilot plus a note on command-based skills and background-task turn holds.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| if (terminalStatus) { | ||
| ctx.heldOpenTaskIds.delete(taskId); | ||
| yield* offerRuntimeEvent({ |
There was a problem hiding this comment.
🟡 Medium Layers/CopilotAdapter.ts:559
Repeated terminal ToolCallUpdated notifications emit duplicate task.completed events for the same taskId, causing clients to apply completion handling more than once. emitSubagentTaskEvents deletes the live-task entry but never checks it before emitting; ignore terminal updates whose taskId is no longer live.
if (terminalStatus) {
+ if (!ctx.heldOpenTaskIds.has(taskId)) {
+ return;
+ }
ctx.heldOpenTaskIds.delete(taskId);🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/CopilotAdapter.ts around lines 559-561:
Repeated terminal `ToolCallUpdated` notifications emit duplicate `task.completed` events for the same `taskId`, causing clients to apply completion handling more than once. `emitSubagentTaskEvents` deletes the live-task entry but never checks it before emitting; ignore terminal updates whose `taskId` is no longer live.
| const rawInput = toolCall.data.rawInput; | ||
| if (isRecord(rawInput)) { | ||
| for (const key of ["agent_type", "agent", "agentName", "agent_name"] as const) { | ||
| const value = rawInput[key]; |
There was a problem hiding this comment.
🟠 High Layers/CopilotAdapter.ts:147
A non-empty rawInput.agent_type value other than "task" is classified as a subagent launch, so ordinary tool calls are removed from the tool timeline and added to heldOpenTaskIds; this can leave turn.completed parked without that tool's completion signal. Restrict the agent_type check to the documented value "task" while retaining the fallback heuristics for the other keys.
for (const key of ["agent_type", "agent", "agentName", "agent_name"] as const) {
- if (typeof value === "string" && value.trim().length > 0) {
+ if (
+ typeof value === "string" &&
+ value.trim().length > 0 &&
+ (key !== "agent_type" || value === "task")
+ ) {🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/CopilotAdapter.ts around line 147:
A non-empty `rawInput.agent_type` value other than `"task"` is classified as a subagent launch, so ordinary tool calls are removed from the tool timeline and added to `heldOpenTaskIds`; this can leave `turn.completed` parked without that tool's completion signal. Restrict the `agent_type` check to the documented value `"task"` while retaining the fallback heuristics for the other keys.
| startOnce.pipe( | ||
| Effect.tap((result) => | ||
| Ref.set(startStateRef, { _tag: "Started", result }).pipe( | ||
| Effect.andThen(drainPreStartUpdates(result.sessionId)), |
There was a problem hiding this comment.
🟡 Medium acp/AcpSessionRuntime.ts:721
A live session/update arriving after Ref.set(startStateRef, { _tag: "Started", result }) but before drainPreStartUpdates completes is processed immediately, ahead of older buffered notifications. This reverses wire order and can emit or update order-sensitive state incorrectly; keep updates buffered until draining finishes and transition through one ordered path.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/acp/AcpSessionRuntime.ts around line 721:
A live `session/update` arriving after `Ref.set(startStateRef, { _tag: "Started", result })` but before `drainPreStartUpdates` completes is processed immediately, ahead of older buffered notifications. This reverses wire order and can emit or update order-sensitive state incorrectly; keep updates buffered until draining finishes and transition through one ordered path.
There was a problem hiding this comment.
Effect service conventions review of the new Copilot driver/adapter/provider modules. Structure, namespace imports, dependency acquisition, and layer/make usage all follow the existing ACP provider pattern. Three error-modeling findings: wrapper detail fields are built from cause.message (or a stringified defect) instead of stable structural attributes, which also feeds the caller-visible message getter of ProviderDriverError / ProviderAdapter*Error.
Posted via Macroscope — Effect Service Conventions
| new ProviderAdapterProcessError({ | ||
| provider: PROVIDER, | ||
| threadId: input.threadId, | ||
| detail: cause.message, |
There was a problem hiding this comment.
detail only copies cause.message, which then becomes the ProviderAdapterProcessError message. Suggest a stable detail describing the stage; the ACP error stays available as cause.
| detail: cause.message, | |
| detail: "Failed to start the GitHub Copilot ACP session.", |
Posted via Macroscope — Effect Service Conventions
| new ProviderAdapterRequestError({ | ||
| provider: PROVIDER, | ||
| method: "session/prompt", | ||
| detail: cause.message, |
There was a problem hiding this comment.
Same here: detail duplicates cause.message and drops the resource context that is available at this boundary. Suggest naming the failing operation/resource and keeping the filesystem error as cause.
| detail: cause.message, | |
| detail: `Failed to read attachment '${attachment.id}'.`, |
Posted via Macroscope — Effect Service Conventions
| new ProviderDriverError({ | ||
| driver: DRIVER_KIND, | ||
| instanceId, | ||
| detail: `Failed to build GitHub Copilot snapshot: ${cause.message ?? String(cause)}`, |
There was a problem hiding this comment.
detail interpolates cause.message / String(cause), and ProviderDriverError.message is derived from detail — so the wrapper message comes from the cause (and can carry arbitrary defect text). Consider keeping a stable, structural detail and letting cause carry the underlying failure.
| detail: `Failed to build GitHub Copilot snapshot: ${cause.message ?? String(cause)}`, | |
| detail: "Failed to build the GitHub Copilot provider snapshot.", |
Posted via Macroscope — Effect Service Conventions
1 participant
Problem
T3 Code had no GitHub Copilot support. Copilot CLI now ships a native ACP server (
copilot --acp, public preview), which means it can reuse the shared ACP runtime that Grok and Cursor already drive — no new transport needed.How
Ten commits, bottom-up, each independently readable:
CopilotSettings(opt-in, like Grok/Cursor).available_commands_updateinto an event; buffer session updates that land before startup settles instead of dropping them (Copilot advertises commands right aftersession/new); first-sighting tool calls withrawInputare no longer suppressed; emptyauthMethodIdskipsauthenticate(Copilot auths outside ACP). All provider-neutral.tasktool calls project ontotask.started/progress/completedso subagents render on the Agents surface.end_turnearly and keeps streaming progress afterwards; the turn stays routable until a follow-up call reports the agent idle, then the parkedturn.completedflushes. Stop cancels outright.ServerProviderSkillwith/nameas path).Verification
end_turnwork into the original turn and completes whenread_agentreports idle (ground truth taken from native event logs).Known ceilings (marked
ponytail:in source)read_agentoutput until upstream exposes structured lifecycle events.loadSession: false); skills refresh at probe time only.ox-alpha-free via OpenCode
Note
Add GitHub Copilot provider via ACP
copilotprovider driver registered in builtInDrivers.ts, with settings schema (enabled,binaryPath,customModels) added to settings.ts and UI entries in web and mobile.copilot --version(4s timeout), then ACP discovery (25s timeout) to collect models and available commands as skills.authenticatewhenauthMethodIdis empty, emitAvailableCommandsChangedevents, and emit first-seen tool calls with non-emptyrawInput.rollbackThreadinCopilotAdapterreturns an unsupported-operation error; consumers expecting rollback will fail.shouldEmitToolCallUpdatenow emits first-seen tool calls with non-emptyrawInput, which may increase event volume for other ACP-based providers sharing this runtime.📊 Macroscope summarized 714a699. 17 files reviewed, 3 issues evaluated, 0 issues filtered, 3 comments posted
🗂️ Filtered Issues