Vendor-neutral ACP host via AcpDialect (I3)

Task 8806 · Done · Coder · created · updated

## Original requirement (epic 8783, verbatim) > Review implementation of model provider interfaces: > > - Claude, Codex, Antigravity, Pi, ACP, > > Identify opportunity to unify, move more code from specific to common, make it more industry standard, more reliable. Do web research to potentially adopt a better architecture. > > Implement top ideas. Settled inputs: PRD (BA task 8784) and architecture (task 8785) — read them with `task_read` 8784 / 8785 and their `prd` / `architecture` artifacts. Decisions there are settled; realize them, do not reopen them. **Quality bar (PRD, verbatim).** A change that passes AC5 by moving code into a grab-bag module with vendor `if` branches inside it FAILS. Common code must be vendor-neutral, and vendor facts stay as data on the adapter or catalogue row. A review artifact that lists ideas without evidence or ranking FAILS AC1/AC2. ## Scope — idea I3 (architecture §3 rank 3, child 3) Make the generic ACP host vendor-neutral: vendor ACP facts become **data** on the catalogue row. 1. Add the pinned seam (architecture §5, Seam 1) in the catalogue's `Capabilities` (`apps/jaahd/agent_providers.py`): ```python @dataclass(frozen=True) class AcpDialect: error_kind_meta: str = "" # e.g. "errorKind" — key on a prompt error's data naming the cause error_kinds: tuple[tuple[str, str], ...] = () # agent's kind -> Jaah break cause (acp/events.py error_cause vocabulary) error_text_patterns: tuple[tuple[str, str], ...] = () # regex on agent text / error message -> break cause rate_limit_meta: str = "" # PromptResponse/_meta key carrying rate-limit reset (Claude: "_claude/rateLimit") usage_meta: str = "" # _meta key carrying per-model usage (Claude: "quota") ``` Reached as `Capabilities.acp_dialect: AcpDialect = AcpDialect()` — every provider defaults to empty, generic behaviour unchanged. Expose `dialect_for(provider)`. `error_text_patterns` must be honoured by the normaliser on agent message text too (child A needs it: Antigravity reports "API key not valid" as agent text with `stopReason: end_turn`, architecture §2.2, risk R3) — include a unit test with a synthetic dialect proving an agent-text match records a break, not a success. 2. Populate Claude/DeepSeek from today's literals in `apps/jaahd/acp/events.py:73-87` (Claude error regex + `errorKind` map), `:92,398-399,609-614` (`CLAUDE_RATE_LIMIT_META`) and `apps/jaahd/acp/host.py:108-115` (claude-agent-acp error payload). `acp/events.py` and `acp/host.py` read only `dialect_for(provider)` fields. 3. Rename the generic `claude_session_id` keyword to `native_session_id` across its call sites (`acp/runtime.py:466`, `fleet_dispatch.py:1630`, `ctl.py:530`, `clauded_wt.py:465`, `bedrock_usage_tail.py:88`, and their tests). It is an internal Python keyword, not a DB column, verb or MCP name, so ADR-0166's no-rename table is not touched. Check first: if any site is a wire or CLI flag name (e.g. `ctl.py:530`), keep that spelling and record why (risk R6). 4. Widen `VENDOR_PATTERN` (`tools/tests/test_vendor_name_census.py:44`) to `codex|gemini|claude|anthropic|antigravity|agy|pi-acp|deepseek`. Allowlist the seam files (`agents/*`, `agent_providers.py`, pins) and fail on any hit in `apps/jaahd/acp/`. `acp/host.py:160` (`ENGINE_STDERR_ENV`, whose comment names `pi_rpc.py`) is a comment; reword it generically or allowlist with the reason "documents a consumer". 5. Replay fixtures under `apps/jaahd/tests/acp_replay/` stay byte-identical (AC7). **PR body:** cite vendor sites in `acp/` before (4) → after (0) and the LOC delta. **Docs:** name `AcpDialect`/`dialect_for` in `apps/jaahd/README.md`'s adapter-seam section and in the ACP host section (the Claude table at about `:1074-1091` now points at the dialect). `python3 tools/check_context.py` passes. This child defines the seam child A ("Antigravity on ACP") populates; A is sequenced after this merges. Credentials & reach: AC1–AC3, AC5, AC7, AC9, AC10 (repo tests, `./dc check full`, `check_context.py`) — Nothing beyond the worktree — verified (worktree present). Out of scope: From PRD §6 (verbatim) — do NOT touch: - **Do NOT** move Claude subscription (oauth) runs onto ACP, or change `Capabilities.runtime` for `claude` oauth (ADR-0167). - **Do NOT** delete the legacy terminal launch path (`fleet_launch`/`fleet_dispatch` exec lines, `answer_delivery` tmux paste, `ComposerTui`). It still serves Claude subscriptions and non-Linux nodes. - **Do NOT** replace or wrap ACP with A2A, an LLM gateway, LiteLLM-style proxies or direct vendor SDKs (D3). - **Do NOT** rename agent verbs, MCP tool names or ACP extension methods (ADR-0166 table), lifecycle states, `terminal_type` values or DB columns. - **Do NOT** drive `agy`'s TUI (S1, ADR-0165 anti-pattern), and do not fork an upstream ACP adapter unless no option closes the gap (ADR-0167 rule). - **Do NOT** add a new provider, or bump any vendor CLI pin other than `agy` (and only if S1 requires it). - **Do NOT** touch the cloud compute "provider" work of epic 8760 (AWS/GCP/Azure node providers, `NodeLink` storage adapters). That is a different "provider". - **Do NOT** redesign console pages (Settings › Providers, Brains roster, task page, Audit › Sessions). The only console delta is Antigravity runs gaining ACP chat/live-control views and their catalogue gap text. - **Do NOT** make ACP available on Windows or macOS nodes (`acp_runtime.py:75` Linux gate stays). - **Do NOT** build ideas ranked below the top 3–5. File them (AC6). - **Do NOT** change the API key handling invariant: no model key in an engine's environment or `launch.json` (ADR-0165 self-check). - Do NOT populate an Antigravity dialect here (child A does it after this merges). - Do NOT change any wire/CLI spelling of `claude_session_id` (keep and record). - Your mechanism's diff must not touch the other unification children's mechanisms (I1–I5 are separate PRs; file overlap is fine, scope overlap is not). Done means: **AC5 — Top 3–5 ideas built, each single-homed and fenced (R5)** (PRD, verbatim). Pass per built idea: - (i) the mechanism has exactly one implementation; - (ii) every former copy calls it; - (iii) a test (census or fence) fails when a second copy or a vendor literal is re-added at the guarded sites; - (iv) the merged PR cites before/after copy count and LOC. A built idea that leaves a residual copy names it in its PR body as a follow-up. → verify, per idea: 1. Grep the repo for the old mechanism's signature (e.g. the per-adapter staged-write pattern): one definition remains. 2. Add a scratch copy at a guarded site locally and run the fence test: it fails. Revert the scratch copy. 3. Read the PR: before/after figures are present. **AC7 — Behaviour preserved, by test (R7, S2)** (PRD, verbatim). Pass, all green on the merged develop: - `apps/jaahd/tests/test_agent_adapters.py` (Claude launch.sh + settings byte-identical to the goldens); - the ACP replay fixtures under `apps/jaahd/tests/acp_replay/`; - `tools/tests/test_provider_capabilities.py`; - every provider's adapter tests; - `./dc check full`. → verify: the merge lane's `./dc check full` GATE_RESULT `status=pass` on each PR. Any golden that had to be regenerated is called out in that PR with the reason. A golden change for any lane other than Antigravity is a failure unless it is byte-neutral in behaviour. **AC9 — No new vendor branches (R5)** (PRD, verbatim). Pass: - No new `provider ==` / vendor-literal branch is introduced in launch sites, `apps/jaahd/acp/` or `apps/jaahd/provider_health.py` dispatch. - The existing census still passes. - Vendor literals in `acp/` (C9) are either removed or kept with a recorded reason. → verify: `git diff develop...<merge> -- apps/jaahd/acp apps/jaahd/fleet_launch.py apps/jaahd/fleet_dispatch.py` shows no added vendor names. `./dc test apps/jaahd/tests/test_agent_adapters.py` is green. **AC10 — Docs current (R8)**, this child's part (the README `:551/:552` "No ACP host exists yet" line and the Antigravity section are owned by the Antigravity ACP child — do not edit them here): - `apps/jaahd/README.md` describes the present; the new common helper is named in the adapter-seam section. - `python3 tools/check_context.py` passes. → verify: run `python3 tools/check_context.py`, which prints no failure; grep the adapter-seam section for the helper name. - `grep -rniE 'claude|anthropic|codex|gemini|antigravity|agy|pi-acp|deepseek' apps/jaahd/acp/` returns only reasoned allowlist hits; widened census test green and fails on a scratch vendor literal in `acp/` (output cited). - All `acp_replay` fixtures byte-identical and green; Claude/DeepSeek ACP break and rate-limit classification unchanged (existing tests green). - Synthetic-dialect test: an `error_text_patterns` match on agent text records a break (output cited).

Outcome

Built AcpDialect on the catalogue row (dialect_for). The ACP host is now vendor-neutral: vendor names in acp/ went from 31 to 4 pinned lines. Agent-text break added, native_session_id renamed, ACP census fence added. PR #6568 merged by the lane's full gate; its deploy was superseded by a later push. Follow-up: draft #8828. Memory: none applied.

Pull request

Artifacts

Activity

  • Draft
  • Ready
  • Runner started
  • Runner ended (done)