docs(acp): spec-driven plan to migrate from the Copilot SDK to ACP - #59
docs(acp): spec-driven plan to migrate from the Copilot SDK to ACP#59BOTOOM wants to merge 3 commits into
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| - **Q2** Which of the *other* target agents support `session/load`/`session/resume` replay | ||
| in practice? Copilot does; the SDK example does not. Determines how much of our local | ||
| history we can retire (R-040). Measured per agent during Phase 5. |
There was a problem hiding this comment.
🔍 Referencia cruzada incorrecta a R-040 en la pregunta abierta Q2
La pregunta abierta Q2 vincula la posibilidad de retirar el historial local con R-040, pero en docs/specs/acp/03-requirements.md:258-260 R-040 es "Elicitation (elicitation/create, v2 draft)". Los requisitos que realmente gobiernan el replay y el historial son R-046/R-047 (docs/specs/acp/03-requirements.md:270-279). Como el flujo de trabajo declarado en docs/specs/acp/README.md:20-29 exige trazabilidad por IDs de requisito, una referencia cruzada errónea puede desviar la verificación en la Fase 6.
Was this helpful? React with 👍 or 👎 to provide feedback.
| **R-050 (P0)** Quick actions (explain/summarise/translate/…) run over ACP. | ||
| - AC1 Each quick action produces a streamed answer in the panel. `e2e` | ||
|
|
||
| **R-051 (P0)** The writing assistant runs over ACP with inline replacement intact. | ||
| - AC1 Selecting text and applying a rewrite replaces it in the page. `e2e` | ||
|
|
||
| **R-052 (P0)** Context-aware mode parity per R-032. | ||
| - AC1 A page-specific question is answered using page content. `e2e` | ||
|
|
There was a problem hiding this comment.
🔍 Requisitos P0 de paridad funcional sin fase asignada en el roadmap
La sección H define R-050, R-051 y R-052 como P0 (paridad de quick actions, asistente de escritura y modo contextual), pero ninguna fase del roadmap los lista en su bloque "Requirements" (docs/specs/acp/04-roadmap.md:12-104). Lo mismo ocurre con R-061 y R-063. Dado que el roadmap declara que ninguna fase se fusiona con un criterio P0 sin verificar (docs/specs/acp/04-roadmap.md:111), conviene asignarlos explícitamente (probablemente Fase 4) para que no queden fuera del MVP.
Was this helpful? React with 👍 or 👎 to provide feedback.
| `openai-compatible.provider.ts` logic from `feat/acp`: streaming, tool calling, and | ||
| capability advertisement. Registered as a built-in catalog entry with an endpoint/model | ||
| config. | ||
| Requirements: R-010 AC3 (as a built-in), plus the A/C/F sets applied to this agent. |
There was a problem hiding this comment.
🔍 La Fase 7 cita R-010 AC3 para agentes integrados, pero ese criterio cubre agentes personalizados
La Fase 7 dice cumplir "R-010 AC3 (as a built-in)", mientras que R-010 AC3 en docs/specs/acp/03-requirements.md:83 describe un agente definido por el usuario lanzable de extremo a extremo, no una entrada integrada del catálogo. Si el agente propio apps/acp-openai-agent debe registrarse como built-in, falta un criterio de aceptación explícito para ello.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Revisión atendida en |
1 participant
Description
Docs only — the spec that the ACP refactor will be implemented against. No runtime code changes.
docs/specs/acp/: research + measured protocol evidence, target architecture, requirements with acceptance criteria, a 9-phase roadmap, and 8 ADRs. Plus.agents/skills/acp-client/SKILL.mdas the working reference for the protocol layer.The decisions that shape the whole refactor, and why:
PROTOCOL_VERSION = 1, and both agents probed in a spike negotiated v1. Version-specific shapes are confined toacp/normalize/{v1,v2}.tsbehind one internalAcpEventunion modelled on v2's upsert semantics, so v1 is the constrained case.cwd, or hold credentials.session/request_permissionis an agent→client request that blocks the turn, and SSE cannot carry it.npx/uvx/binary launch specs + sha256); T2 profiles — named launch tuples so variants coexist (devin acpvsdevin acp --cloud,copilot --acp --stdiovs--acp --port N, MiniMax'smini-agent-acp, private builds); T3 non-ACP endpoints (LM Studio/Ollama) served by our own ACP agent (ADR-0006); T4 products with no ACP path. Support is then measured: a conformance probe (R-017) runs initialize → prompt → slash command → image → permission → history → cancel against any profile and generates the support table indocs/ACP.md, instead of us hand-integrating agents.agy --acpis an open upstream request (google-antigravity/antigravity-cli#31) and Google's terms forbid third-party software using an Antigravity login, so we ship no bridge (R-018). Related risk: Gemini CLI is being transitioned to Antigravity CLI, so the Gemini catalog entry is pinned and treated as at-risk. Copilot cloud likewise has no ACP surface — "Copilot over ACP" means the CLI.origin/feat/acpis superseded, not merged (ADR-0008). Despite the name it contains no ACP — it's an in-house provider layer whose CLI adapter scrapes stdout into synthetic Copilot-shaped events. Three pieces are cherry-picked: the encrypted credential store, the OpenAI-compatible client, and the provider-grouped UI patterns.loadSession: true; the SDK example agent answers-32601tosession/list/load/resume), and cross-agent listing must work offline before any agent is spawned.approveAllbecomes a user prompt with rememberedallow_alwaysand an explicit per-agent opt-in for blanket approval.Spike evidence behind the above (throwaway client, SDK
client()+ndJsonStreamover spawned subprocesses):copilot --acp --stdio→ v1,loadSession: true,promptCapabilities.image: true/audio: false/embeddedContext: true, auth methodcopilot-login; unauthenticatedsession/newfails with{ code: -32000, message: "Authentication required" }.-32601for the history methods (so unsupported ≠ broken: gate on capabilities, don't try/catch), andagent_message_chunks still arrive aftersession/cancel.Product consequences called out in the spec, since they aren't just internal: agents must now be installed and authenticated locally (
copilot login), a configurable workspace root becomes mandatory becausesession/newrequires an absolutecwd, Copilot ACP fixes reasoning/tool-filtering at server start rather than per session, and every control (models, modes, slash commands, images) becomes capability-driven at runtime.Roadmap: Phase 1 protocol layer + fixture agent → 2 catalog/profiles/auth → 3 WS gateway + chat → 4 ACP-native UI (MVP) → 5 all-agents catalog + conformance probe → 6 history → 7 local models → 8 remove the Copilot SDK → 9 v2. The SDK path stays behind
ACP_ENABLED=falseuntil Phase 4 exits, somasterkeeps working throughout.Type of Change
Related Issues
Checklist
pnpm typecheck) — docs only, no code touchedpnpm lint) —pnpm lintis red onmasterbefore this branch (38 pre-existing Biome errors in TS files); this PR adds only Markdownpnpm test) — unaffectedScreenshots (if applicable)
n/a
Link to Devin session: https://app.devin.ai/sessions/bab32da5729e4a95a9cb79f1648f005e
Requested by: @BOTOOM