refactor(daemon): split acp.ts + pi-rpc.ts into an agent-protocol/ capability-barrel

[leonaburime-ucla]

Why

apps/daemon/src/acp.ts (1,744 lines) and apps/daemon/src/pi-rpc.ts (684 lines) were two flat god-files driving the daemon's subprocess agent protocols — ACP (Agent Client Protocol, used by Hermes/Kilo/Kiro/Devin/Vibe/vela-AMR) and pi's --mode rpc. acp.ts in particular mixed JSON-RPC transport, model detection, session-update classification, artifact/tool-call suppression, and a 750-line session orchestrator in one module with no enforced internal boundaries, and pi-rpc.ts reached into acp.ts for its shared line-stream transport (createJsonLineStream). Editing either meant reading past every unrelated concern, and nothing stopped a new cross-file coupling from forming.

This is the maintainability lift for that subsystem: split both into one navigable, boundary-shaped capability-barrel module (the design-systems pattern), so each protocol concern can evolve on its own and the shared transport has one owner.

What users will see

Nothing. This is a pure internal refactor — the public API (10 exports) and all runtime behavior are identical. QA regression surface is the subprocess-agent run path: ACP agents (Hermes, Kilo, Kiro, Devin, Vibe, vela/AMR) and pi RPC — model detection (/api/agents), session streaming (text/thinking/tool_use/tool_result/usage), artifact-write mirroring, permission auto-reply, resume, and abort.

What this does

Merges flat acp.ts + pi-rpc.ts into a single agent-protocol/ module:

• core/ foundation kernel — the one shared primitive: createJsonLineStream (the streaming JSON-line transport both protocols use). Imports no sibling.
• acp/ concern subdir — types, constants, json (value/text helpers), rpc (JSON-RPC send + error shapes), session-params, models (detection/normalization), updates (status/artifact/AMR classification), session (the attachAcpSession orchestrator), each behind the acp/ barrel.
• pi-rpc/ concern subdir — internal (shared guards/types), events (mapPiRpcEvent), models (parsePiModels), session (attachPiRpcSession), behind the pi-rpc/ barrel.
• Moving createJsonLineStream into core/ dissolves the former pi-rpc → acp import edge into a pure star topology (allowedEdges: [], foundation core) — no concern imports another concern; both import only core/.
• Root barrel agent-protocol/index.ts = explicit named re-exports reproducing the exact prior public surface (10 names). Every importer repointed to ./agent-protocol/index.js (server.ts, connectionTest.ts, runtimes/defs/shared.ts + 5 test files).
• Every function body moved byte-identically (line-slice moves, no rewrites). Every file carries a @module docblock, every export is JSDoc'd, and a module README.md documents the shape.

Scope / boundary

One subsystem, pure structural move, no logic changes. The capability-barrel guard registration (scripts/check-barrel-imports.ts / CAPABILITY_BARREL_DOMAINS) is intentionally deferred — that infra is not on main yet; this PR produces the full documented structure and the guard entry lands once the infra does.

Surface area

• UI — new page / dialog / panel / menu item / setting / empty state in apps/web or apps/desktop
• Keyboard shortcut — new or changed
• CLI / env var — new od subcommand or flag, new tools-* flag, or new OD_* env var
• API / contract — new /api/* endpoint, new SSE event, or changed shape in packages/contracts
• Extension point — new entry under skills/, design-systems/, design-templates/, or craft/
• i18n keys — added new translation keys
• New top-level dependency — new entry in the root package.json
• Default behavior change — default model/setting, file/SQLite schema, auto-network, auto-install
• None — internal refactor, docs, tests, or translation update only

Validation

• pnpm --filter @open-design/daemon typecheck (src + tests): green (0 errors).
• pnpm --filter @open-design/daemon exec vitest run acp pi-rpc structured-streams amr-acp: 5 files / 160 tests pass — matches baseline.
• pnpm guard: green (71/71).
• Public-surface: old union of acp.ts + pi-rpc.ts exports (10) == new root-barrel names (10), exactly.
• Byte-identity: 1,960 code lines, multiset-identical between the originals and the new non-barrel files — 0 dropped, 0 added (only deltas are import-path rewrites, added export keywords for barrel re-export, and doc-comments). Re-verified after the doc pass (still 1960==1960).
• Runtime smoke: barrel resolves all 8 runtime exports; runtimes/defs/shared.ts + connectionTest.ts load clean (server.ts is @ts-nocheck, so its attachAcpSession/attachPiRpcSession import was runtime-checked, not just type-checked).

[lefarcen]

🧪 Queued for QA validation — this PR touches runtime behavior that needs a manual QA pass before it's merged. Nothing needed from you right now; we'll update here once that validation is done. Thanks for the contribution! 🙏

[nettee]

I found one refactor regression worth fixing before this module shape gets copied elsewhere: deleting the legacy ACP entrypoint leaves one daemon verification script pointing at a built file that no longer exists. I wasn't able to rerun the daemon Vitest suite locally in this worktree because vitest is not installed here.

_{🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.}

[nettee]

Deleting the ACP entrypoint here leaves apps/daemon/scripts/verify-amr-real-vela.mjs:29 importing ../dist/acp.js, but this refactor stops producing that build artifact. After the next pnpm --filter @open-design/daemon build, the script's dynamic import will fail with ERR_MODULE_NOT_FOUND, which breaks one of the few real-vela smoke verifiers for this subsystem. Please either keep a thin compatibility shim at src/acp.ts that re-exports attachAcpSession from agent-protocol, or update the script in this PR to import the new built path (dist/agent-protocol/index.js or dist/agent-protocol/acp/index.js) so the manual validation path stays usable.

_{🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.}

[leonaburime-ucla]

On it

[leonaburime-ucla]

Good catch — fixed in e26a776. Repointed the dynamic import from ../dist/acp.js to ../dist/agent-protocol/index.js (the root barrel re-exports attachAcpSession). Chose the repoint over a compat shim since a re-export shim is exactly the leftover this capability-barrel refactor is meant to remove. Verified locally: pnpm --filter @open-design/daemon build emits dist/agent-protocol/index.js, and the script's dynamic import resolves attachAcpSession as a function.

[nettee]

@leonaburime-ucla I re-reviewed the current e26a7764c4c30ff93c1274c00a5b03f2b152a37c head and verified the live PR diff is limited to the agent-protocol/ barrel split, importer rewires, test import updates, and the verify-amr-real-vela dist-path fix. I did not find additional correctness or maintainability regressions in the changed ranges beyond the already-addressed smoke-script import issue, and the public adapter surface looks preserved across the new barrels. I couldn't rerun the daemon typecheck/Vitest suite in this worktree because node_modules are absent here, but the scoped refactor itself looks consistent. Nice cleanup on a large protocol module.

_{🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.}

[leonaburime-ucla]

@nettee @lefarcen When will all these PRs be reviewed and merged?

[lefarcen]

Hey @leonaburime-ucla — on this PR, the code-review side is already in good shape and CI is green. The remaining step is QA validation from @AmyShang-alt; once that lands, a maintainer can make the final merge call. I can't promise an exact merge time from here, but it's waiting on that validation rather than another code-review round.

[AmyShang-alt]

QA passed for PR #5200.

Verified current head e26a776 on namespace pr5200 against daemon http://127.0.0.1:61539. ACP/AMR runtime smoke passed: POST /api/runs with agentId=amr produced acp-json-rpc events including model status, thinking deltas, text_delta, Write tool_use for pr5200-smoke.txt, successful tool_result, usage, and final end status=succeeded. No protocol parse failure or AGENT_EXECUTION_FAILED observed.

Abort smoke also passed: POST /api/runs/9767853c-1755-4a86-a7ee-58d2706efe10/cancel returned ok=true with status=canceled, cancelRequested=true, childExited=true, processGroupId=20633, exitCode=130, and no error/errorCode. This confirms the ACP runtime cancellation path still terminates the child process and settles the run cleanly.

Given CI is green and the high-risk ACP runtime path plus cancellation path were manually verified, QA accepts this PR.

[lefarcen]

Thanks @AmyShang-alt — that QA pass is exactly what this one was waiting on. @leonaburime-ucla, the review + validation side is now complete on the current head; the remaining step is the final maintainer merge call.