[AI-686] ACP permission & elicitation bridge (daemon half)

[realtonyyoung]

What

Daemon half of AI-686 — ACP permission & elicitation bridge (child of the AI-682 ACP epic). Wires ACP's blocking human-in-the-loop calls (session/request_permission, capability-gated elicitation/create) — which arrive in-process in the daemon ACP runtime — through to the server and back, so a hosted Cursor/ACP agent blocks until a human answers in the Capacitor UI.

Server half is a separate PR in kcap-server. This half is developed + tested entirely in kcap-cli; the only cross-repo step is a follow-up src/cli bump in kcap-server after this merges.

Changes

• AcpInteractionBridge — parses an inbound ACP request, forwards it via ServerConnection.RequestAcpInteractionAsync, awaits the human decision, and maps it back fail-safe: MapPermissionDecision is an explicit affirmative allowlist (allow/allow_once/allow_always/answered) matched by OptionId exclusively — any unrecognized/deny/cancel/option-less/null-or-unresolvable-OptionId decision → the ACP cancelled response. No default: selected, no first-option fallback anywhere.
• PendingAcpInteractionRegistry — id-keyed await/resolve, early-buffer, idempotent, disconnect/cancel → defensive cancelled exactly once (mirrors the proven PendingPermissionRegistry).
• ServerConnection.RequestAcpInteractionAsync + the AcpInteractionResolved push handler.
• Wire DTOs (SessionRequestPermissionParams/PermissionOptionDto/PermissionOutcomeDto/…) in AcpMessages.cs, source-gen registered on CapacitorJsonContext — in lockstep with the server DTOs (ACP-wire outcome spelling "cancelled", distinct from the server-internal "cancel").
• Production wiring: AcpHostedAgentRuntimeFactory passes a non-null requestInteraction (real ServerConnection.RequestAcpInteractionAsync); AcpHostedAgentRuntime wires AcpConnection.OnServerRequest. Elicitation capability is not advertised (defensive if received). PTY/Claude/Codex runtimes untouched.
• Fixed a pre-existing bug: AcpConnection.HandleServerRequestAsync used a possibly-cancelled token for its final response write, dropping the disconnect-while-pending cancelled reply.

Testing

Capacitor.Cli.Tests.Unit 2498/2498; NativeAOT/trim publish gate clean (no IL2026/IL3050). Covers the full fail-safe mapping matrix, the registry lifecycle, FakeAcpAgent-driven end-to-end session/request_permission, and the real-factory wiring test (verified to fail if requestInteraction regresses to null). Spec-derived wire shapes are marked (the AI-684 probe never reached a permission turn).

Review

Built via subagent-driven development: 4 tasks (B1–B4) each TDD'd + per-task-reviewed, then a whole-branch opus review → READY TO MERGE (no Critical/Important; 2 acceptable-follow-up Minors).

🤖 Generated with Claude Code

[linear-code]

AI-686

[qodo-code-review]

PR Summary by Qodo

AI-686: Bridge ACP permission/elicitation interactions through server (daemon side)

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Bridge ACP permission/elicitation requests from daemon ACP runtime to server and back.
• Add fail-closed decision mapping and pending-interaction correlation with disconnect safety.
• Register AOT-safe JSON DTOs and add end-to-end + unit coverage for wiring and mapping.

Diagram

sequenceDiagram
  participant Agent as "ACP Agent"
  participant Conn as "AcpConnection"
  participant Bridge as "AcpInteractionBridge"
  participant SConn as "ServerConnection (SignalR)"
  participant Reg as "PendingAcpInteractionRegistry"
  participant UI as "Capacitor UI/Server"

  Agent->>Conn: "session/request_permission" / "elicitation/create"
  Conn->>Bridge: OnServerRequest(request)
  Bridge->>SConn: AcpRequestInteraction(request)
  SConn->>Reg: AwaitDecision(requestId)
  UI-->>SConn: AcpInteractionResolved(decision)
  SConn-->>Reg: Resolve(requestId, decision)
  Reg-->>SConn: decision
  SConn-->>Bridge: decision
  Bridge-->>Conn: PermissionOutcomeResult
  Conn-->>Agent: JSON-RPC result (selected/cancelled)

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Shared contract package for interaction DTOs/outcome enums

• ➕ Compile-time alignment between server/daemon vocab (avoid outcome-string drift)
• ➕ Centralizes wire contract versioning and documentation
• ➖ Cross-repo dependency management/version skew becomes a release concern
• ➖ May complicate NativeAOT/trim posture if not carefully designed

2. Generic pending-registry abstraction (PendingRegistry)

• ➕ Eliminates copy-shape registries and reduces maintenance duplication
• ➖ Couples two flows that may diverge (permission vs ACP interaction semantics)
• ➖ Harder to tailor buffering/cancellation behavior per interaction type later

3. Server-driven mapping (daemon forwards raw ACP request, server returns ACP-shaped outcome)

• ➕ Single place for outcome mapping rules and allowlist decisions
• ➕ Daemon stays thinner; fewer security-sensitive mapping rules client-side
• ➖ Daemon would need to trust server to always return ACP-correct shapes
• ➖ Harder to preserve “always write exactly one response” invariants on disconnect

Recommendation: Current approach is strong for safety and operability: the daemon owns the “exactly one ACP response” guarantee and fails closed via an explicit affirmative allowlist with OptionId-only selection. Consider a shared contract package in a later iteration if outcome-string drift becomes a recurring issue, but keep the daemon-side fail-safe mapping even then to preserve disconnect/teardown correctness.

Files changed (15) +1735 / -31

Enhancement (8) +603 / -20

AcpMessages.csAdd ACP permission + elicitation wire DTOs +80/-0

Add ACP permission + elicitation wire DTOs

• Introduces spec-derived DTOs for session/request_permission and defensive elicitation/create, including option and outcome result shapes. Uses JsonPropertyName + JsonIgnore rules to match ACP camelCase wire vocabulary and omit null optionId/schema fields.

src/Capacitor.Cli.Core/Acp/AcpMessages.cs

Models.csRegister new DTOs and add server-contract mirror types +64/-0

Register new DTOs and add server-contract mirror types

• Registers new ACP DTOs and new AcpInteraction* hub contract types on CapacitorJsonContext for AOT-safe source-gen serialization. Adds AcpInteractionRequest/Option/Decision/Resolution records aligned with server SignalR snake_case naming policy.

src/Capacitor.Cli.Core/Models.cs

AcpInteractionBridge.csBridge ACP interaction requests to server and map decisions fail-closed +263/-0

Bridge ACP interaction requests to server and map decisions fail-closed

• Adds a new bridge handling session/request_permission and defensive elicitation/create by forwarding to an injected RequestAcpInteractionAsync-like delegate. Implements explicit affirmative outcome allowlist and OptionId-only selection with no first-option fallback; all failures map to ACP "cancelled".

src/Capacitor.Cli.Daemon/Acp/AcpInteractionBridge.cs

DaemonRunner.csWire ServerConnection into ACP runtime factory +5/-1

Wire ServerConnection into ACP runtime factory

• Updates DI wiring so AcpHostedAgentRuntimeFactory receives ServerConnection for production interaction forwarding. Ensures Cursor/ACP launches can call RequestAcpInteractionAsync.

src/Capacitor.Cli.Daemon/DaemonRunner.cs

AcpHostedAgentRuntime.csOptionally wire OnServerRequest via AcpInteractionBridge +27/-1

Optionally wire OnServerRequest via AcpInteractionBridge

• Adds optional requestInteraction delegate + agentId parameters; when provided, creates AcpInteractionBridge and assigns AcpConnection.OnServerRequest. Preserves AI-684 behavior when delegate is null (method-not-found default posture).

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs

AcpHostedAgentRuntimeFactory.csInject ServerConnection and wire real interaction delegate in StartAsync +61/-18

Inject ServerConnection and wire real interaction delegate in StartAsync

• Adds ServerConnection dependency and passes ctx.AgentId + connection.RequestAcpInteractionAsync into AcpHostedAgentRuntime. Extracts process/stream creation behind a connectionSource seam to enable factory-level tests without spawning cursor-agent.

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntimeFactory.cs

PendingAcpInteractionRegistry.csAdd pending registry for ACP interaction requestId correlation +70/-0

Add pending registry for ACP interaction requestId correlation

• Introduces a copy-shape registry (like PendingPermissionRegistry) for awaiting AcpInteractionDecision by requestId. Supports early-buffering, bounded eviction, and cancellation cleanup.

src/Capacitor.Cli.Daemon/Services/PendingAcpInteractionRegistry.cs

ServerConnection.csSupport AcpRequestInteraction and AcpInteractionResolved push handling +33/-0

Support AcpRequestInteraction and AcpInteractionResolved push handling

• Adds PendingAcpInteractionRegistry, registers AcpInteractionResolved push handler, and implements RequestAcpInteractionAsync using invoke-then-await-push pattern. Mirrors existing permission flow retry/ownership handling.

src/Capacitor.Cli.Daemon/Services/ServerConnection.cs

Bug fix (1) +24 / -9

AcpConnection.csEnsure server-request responses are written even after cancellation +24/-9

Ensure server-request responses are written even after cancellation

• Fixes a pre-existing shutdown/disconnect bug by writing the final server-request response with CancellationToken.None. Keeps the “exactly one response frame” invariant even when the handler CT is already cancelled.

src/Capacitor.Cli.Daemon/Acp/AcpConnection.cs

Tests (6) +1108 / -2

AcpHostedAgentRuntimePermissionTests.csEnd-to-end tests for blocking ACP permission flow in runtime +241/-0

End-to-end tests for blocking ACP permission flow in runtime

• Adds e2e coverage that a permission request blocks until the injected decision resolves, validates backward-compat null-wiring behavior, verifies cancel maps to ACP cancelled, and asserts disconnect/Dispose resolves pending requests deterministically.

test/Capacitor.Cli.Tests.Unit/Acp/AcpHostedAgentRuntimePermissionTests.cs

AcpInteractionBridgeTests.csUnit tests for parsing/forwarding and fail-closed mapping +378/-0

Unit tests for parsing/forwarding and fail-closed mapping

• Covers decision mapping matrix: allowlist outcomes, null/unresolvable SelectedOptionId, duplicate labels, unrecognized outcomes, and exception/cancellation handling. Validates defensive elicitation/create forwarding of requestedSchema.

test/Capacitor.Cli.Tests.Unit/Acp/AcpInteractionBridgeTests.cs

AcpInteractionMessagesTests.csSerialization round-trip tests for new DTOs and hub contracts +166/-0

Serialization round-trip tests for new DTOs and hub contracts

• Verifies camelCase ACP wire shapes (including cancelled spelling and null omission) and snake_case hub-contract mirror shapes. Ensures new types are registered on CapacitorJsonContext for AOT-safe serialization.

test/Capacitor.Cli.Tests.Unit/Acp/AcpInteractionMessagesTests.cs

FakeAcpAgent.csExtend fake agent to send server→client permission requests and capture responses +114/-2

Extend fake agent to send server→client permission requests and capture responses

• Adds scripting to inject session/request_permission during session/prompt and await the connection’s JSON-RPC response without deadlocking the read loop. Tracks sent server requests and last response/error for assertions.

test/Capacitor.Cli.Tests.Unit/Acp/FakeAcpAgent.cs

AcpHostedAgentRuntimeFactoryTests.csFactory-level wiring tests for requestInteraction delegate +160/-0

Factory-level wiring tests for requestInteraction delegate

• Drives the real AcpHostedAgentRuntimeFactory with an in-memory FakeAcpAgent connectionSource and a capturing ServerConnection subclass. Asserts the produced runtime invokes RequestAcpInteractionAsync and maps cancel → cancelled; includes negative control for method-not-found when unwired.

test/Capacitor.Cli.Tests.Unit/Services/AcpHostedAgentRuntimeFactoryTests.cs

PendingAcpInteractionRegistryTests.csUnit tests for pending registry buffering and cancellation +49/-0

Unit tests for pending registry buffering and cancellation

• Mirrors PendingPermissionRegistry tests: early resolve buffering, resolve-after-await completion, and cancellation token behavior. Guards against hangs with timeouts.

test/Capacitor.Cli.Tests.Unit/Services/PendingAcpInteractionRegistryTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

1. Null options crash bridge ✓ Resolved 🐞 Bug ≡ Correctness

Description

AcpInteractionBridge.HandlePermissionAsync dereferences parsed.Options without null/element
validation; if ACP params omit or null out options, the bridge throws and AcpConnection falls back
to a generic JSON-RPC -32603 error instead of a well‑formed ACP cancelled outcome. This violates
the bridge’s fail-safe contract and can wedge/obscure permission handling for malformed or
future-shape frames.

Code

src/Capacitor.Cli.Daemon/Acp/AcpInteractionBridge.cs[R63-104]

+        var interactionRequest = new AcpInteractionRequest(
+            AgentId: agentId,
+            AcpSessionId: acpSessionId,
+            Kind: "permission",
+            ToolName: TryGetToolTitle(parsed.ToolCall),
+            ToolInput: parsed.ToolCall,
+            ToolCallId: TryGetToolCallId(parsed.ToolCall),
+            Prompt: null,
+            // Spec-review Finding 6: carry OptionId through to the server-facing DTO so a UI
+            // decision can round-trip it back — Options: o.Name (Label) is now display-only.
+            Options: parsed.Options.Select(o => new AcpInteractionOption(o.OptionId, o.Name, null, o.Kind)).ToArray(),
+            IsMultiSelect: false
+        );
+
+        AcpInteractionDecision decision;
+
+        try {
+            decision = await requestInteraction(interactionRequest, ct).ConfigureAwait(false);
+        } catch (OperationCanceledException) {
+            // Spec-review Finding 3(b): connection-closed / runtime-disposing / CT-cancelled while
+            // this interaction is pending (AcpHostedAgentRuntime.DisposeAsync cancels its _cts,
+            // which is the SAME token flowing through AcpConnection's read loop → HandleServerRequestAsync
+            // → this bridge → RequestAcpInteractionAsync → PendingAcpInteractionRegistry.AwaitDecisionAsync,
+            // whose own ct.Register callback removes the pending entry and TrySetCanceled()s it —
+            // see Task B2). PRE-FIX, this exception type was explicitly excluded from the catch
+            // below and propagated uncaught, so AcpConnection.HandleServerRequestAsync's own
+            // catch-all converted it to a generic JSON-RPC "Internal error" (-32603) instead of the
+            // well-formed ACP `cancelled` outcome every OTHER failure path in this bridge produces.
+            // Best-effort: WriteServerResponseAsync may itself fail if the wire is already torn
+            // down (AcpConnection's own catch around the write handles that silently) — this
+            // bridge's only job is to make the ATTEMPTED response well-formed.
+            logger.LogDebug("ACP: session/request_permission cancelled (connection closing) for agent {AgentId}; defaulting to cancelled", agentId);
+
+            return CancelledResult();
+        } catch (Exception ex) {
+            logger.LogDebug(ex, "ACP: RequestAcpInteractionAsync threw for agent {AgentId}; defaulting to cancelled", agentId);
+
+            return CancelledResult();
+        }
+
+        return MapPermissionDecision(decision, parsed.Options);
+    }

Evidence

The ACP DTO declares options as a non-nullable array, but System.Text.Json does not enforce
nullable annotations and can still produce null for missing/null JSON fields; the bridge then uses
parsed.Options without checks, so malformed/unexpected frames throw. AcpConnection catches handler
exceptions and turns them into -32603 Internal error, not an ACP cancelled result.

src/Capacitor.Cli.Core/Acp/AcpMessages.cs[71-75]
src/Capacitor.Cli.Daemon/Acp/AcpInteractionBridge.cs[48-104]
src/Capacitor.Cli.Daemon/Acp/AcpConnection.cs[269-308]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`AcpInteractionBridge.HandlePermissionAsync` assumes `SessionRequestPermissionParams.Options` is non-null and contains non-null elements. If the agent sends `options: null` or omits the field (spec is noted as not probe-confirmed), deserialization can yield `parsed.Options == null`, causing `NullReferenceException` during `Select(...)` or in `MapPermissionDecision`, and the caller (`AcpConnection.HandleServerRequestAsync`) will respond with JSON-RPC `-32603` instead of ACP `{"outcome":{"outcome":"cancelled"}}`.

## Issue Context
The bridge’s design intent (and comments) is fail-safe: all parse/mapping failures should deterministically return an ACP `cancelled` result. Null `options` (or null items) is a protocol-boundary input case that currently escapes that guarantee.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Acp/AcpInteractionBridge.cs[48-104]
- src/Capacitor.Cli.Core/Acp/AcpMessages.cs[71-75]
- src/Capacitor.Cli.Daemon/Acp/AcpConnection.cs[269-308]

## Suggested fix outline
- In `HandlePermissionAsync`, normalize options early:
 - `var options = parsed.Options ?? Array.Empty<PermissionOptionDto>();`
 - Optionally filter null items: `options = options.Where(o => o is not null).ToArray();`
 - If `options.Length == 0`, immediately return `CancelledResult()` (since there’s nothing resolvable anyway).
- Use the normalized `options` variable both when building `interactionRequest.Options` and when calling `MapPermissionDecision(decision, options)`.
- Consider also guarding `parsed.ToolCall` being `JsonValueKind.Undefined` by setting `ToolInput: null` in that case (to avoid downstream serialization exceptions), but the core requirement is to prevent any exception from escaping the bridge.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Empty sessionId forwarded ✓ Resolved 🐞 Bug ☼ Reliability

Description

AcpHostedAgentRuntime wires OnServerRequest to call the bridge with _sessionId ?? "", so any
server→client request handled before _sessionId is assigned (or if it is unexpectedly unset) will
be forwarded to the server as an AcpInteractionRequest with an empty AcpSessionId. This can
break interaction correlation and makes behavior depend on protocol timing assumptions rather than
defensive validation.

Code

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[R70-73]

+        if (requestInteraction is not null) {
+            _interactionBridge = new AcpInteractionBridge(requestInteraction, agentId, logger);
+            _connection.OnServerRequest = (request, ct) => _interactionBridge.HandleAsync(request, _sessionId ?? "", ct);
+        }

Evidence

The runtime installs the server-request handler in the constructor and explicitly supplies an
empty-string fallback. _sessionId is only populated later after session/new completes, so there
is a real state in which the callback exists but sessionId is not yet available.

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[57-73]
src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[104-128]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`AcpHostedAgentRuntime` passes `_sessionId ?? ""` into `AcpInteractionBridge.HandleAsync`. If a server request is dispatched while `_sessionId` is still null (read loop starts before handshake completes) or is otherwise unavailable, the bridge will send an `AcpInteractionRequest` to the server with `AcpSessionId == ""`.

## Issue Context
Even if the ACP protocol *usually* only sends permission/elicitation requests during turns after `session/new`, this code currently relies on that sequencing. The bridge already parses `sessionId` from request params, so the runtime doesn’t need to manufacture an empty fallback.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[70-73]
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[104-128]
- src/Capacitor.Cli.Daemon/Acp/AcpInteractionBridge.cs[48-76]

## Suggested fix outline
- Prefer removing the `acpSessionId` argument from `AcpInteractionBridge.HandleAsync` and use `parsed.SessionId` (or validate it matches `_sessionId` when `_sessionId` is known).
 - If the request has no resolvable `sessionId`, return `CancelledResult()`.
- Alternatively (smaller change): make the runtime pass `_sessionId` as nullable and have the bridge pick `acpSessionId` if non-empty, else fall back to `parsed.SessionId`; if both are missing/empty, cancel.
- Avoid ever constructing `AcpInteractionRequest` with an empty `AcpSessionId` string.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Fake dispatch masks exceptions ✓ Resolved 🐞 Bug ☼ Reliability

Description

FakeAcpAgent.RunAsync no longer awaits DispatchLineAsync; dispatch faults are only written to Debug
output and the test harness may hang/time out instead of failing with the underlying exception. This
reduces test signal quality and can increase CI flakiness and debugging time.

Code

test/Capacitor.Cli.Tests.Unit/Acp/FakeAcpAgent.cs[R170-187]

+                // AI-686: dispatched as untracked background work rather than awaited in-loop.
+                // EnqueuePermissionRequestDuringNextPrompt's session/request_permission send-and-await
+                // (SendServerRequestAndAwaitResponseAsync) is itself triggered from a session/prompt's
+                // dispatch — its TaskCompletionSource can only be completed by THIS SAME loop reading
+                // the connection's reply line. Awaiting the dispatch here would therefore deadlock:
+                // the loop would be blocked inside the prompt's dispatch waiting for a line that only
+                // the loop itself can read. Firing dispatch as background work keeps the loop free to
+                // read subsequent lines (including that reply) while a single dispatch is in flight.
+                // Record() (called synchronously at the top of DispatchLineAsync, before any await)
+                // still runs before this method returns to the loop for lines processed one at a
+                // time by ReadLineAsync, so ReceivedCalls order is unaffected for the existing tests
+                // that assert strict ordering (FakeAcpAgentTests) — those never have two lines
+                // in flight at once.
+                var dispatchTask = DispatchLineAsync(line, ct);
+                _ = dispatchTask.ContinueWith(t => {
+                    if (t.IsFaulted)
+                        System.Diagnostics.Debug.WriteLine($"FakeAcpAgent: DispatchLineAsync faulted: {t.Exception}");
+                }, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default);

Evidence

The read loop now starts dispatch as background work and intentionally does not await it; the
continuation does not propagate the exception, so callers don’t observe failures directly.

test/Capacitor.Cli.Tests.Unit/Acp/FakeAcpAgent.cs[158-188]
test/Capacitor.Cli.Tests.Unit/Acp/FakeAcpAgent.cs[194-217]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`FakeAcpAgent.RunAsync` fire-and-forgets `DispatchLineAsync` and only logs faults via `Debug.WriteLine`, which means exceptions no longer fail tests promptly and can manifest as timeouts/hangs.

## Issue Context
The change avoids a deadlock for the new “fake sends server→client request and waits for response” scripting, but it also makes failures harder to diagnose.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Acp/FakeAcpAgent.cs[158-188]
- test/Capacitor.Cli.Tests.Unit/Acp/FakeAcpAgent.cs[194-249]

## Suggested fix outline
- Keep the non-blocking dispatch model, but capture exceptions in a thread-safe way and make them observable:
 - Store the first fault in a `TaskCompletionSource<Exception>` or `ExceptionDispatchInfo` field.
 - Optionally cancel the main loop token/source (in tests) when a dispatch fault occurs.
 - Expose a `Task Completion`/`Exception` property that tests can await/assert, or have `DisposeAsync`/`RunAsync` rethrow if a fault occurred.
- Alternatively, track dispatch tasks in a collection and await them on shutdown to surface any exceptions deterministically.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---