Admit native trusted tools in the unattended reviewer (fixes workspace-backed launch reap)

[realtonyyoung]

Problem

A workspace-backed unattended review flow with an ACP reviewer (observed with kiro) is reaped at launch:

Error (participant_launch_failed): Participant 'reviewer' agent '…' failed to launch: unattended_interaction_forbidden:session/request_permission

Context-only mode (diff inlined) reviews cleanly with the same vendor, so the frame is provoked by the workspace-backed mode reading the mirrored worktree, not by the vendor being broken.

Root cause

Under AllowlistedAutoApprove, the daemon admits a leaked session/request_permission frame only if it names a tool the launch trusts. But UnattendedToolAdmission.AdmittedFor was built from only the @server/tool namespaced half of the trust list, while the --trust-tools argv (KiroReviewerTrustList.Build) also grants the native tools fs_read and thinking.

So a leaked prompt for fs_read — a tool the launch already trusts, which the vendor prompts for intermittently even when trusted (the same leak that motivated AllowlistedAutoApprove for the injected result channel) — was refused by admission and reaped. A workspace-backed launch reads the mirrored worktree and hits fs_read; context-only reads nothing and never does.

The factory already states the intended invariant — the admitted set is "built from the SAME injected specs and identity the trust argv is built from … Two derivations would let the reviewer be TRUSTED to call a tool the policy then refuses to approve" — so admission being narrower than trust is the defect.

Fix

1. AdmittedFor returns the full trust set — native tools ∪ namespaced entries — so admission equals trust. fs_write/execute_bash are in neither, so the read-only containment floor is unchanged.
2. Reap diagnostics: the AllowlistedAutoApprove permission-frame reap now logs the (untrusted, agent-supplied) tool title + kind. Previously only the method was logged, so the leaked frame was undiagnosable even from the daemon log.

Caveat

The match stays a complete-title comparison (the deliberate anti-substring rule). Kiro's MCP-tool titles are Running: @server/tool; a native-tool title shape was never captured by probe, so if a native frame's title is presentation text rather than fs_read, the fix is inert for that frame and fails closed — the new diagnostic (2) is what will let us refine IsAdmitted with the real title. The companion kcap-server change surfaces a context-only remediation hint on this coded reason so the workaround no longer needs tribal knowledge.

Testing

• UnattendedToolAdmissionTests: the previously-pinned "namespaced half" test is now "the full trust list"; added native-tool admitted cases (fs_read, thinking) and the containment-floor refusals (fs_write, execute_bash).
• Existing AcpInteractionBridgeTests / preset tests remain green; AOT publish is IL-clean.

Closes #614
Linear: AI-2096

[qodo-code-review]

PR Summary by Qodo

Admit Native Trusted Tools for Unattended ACP Reviews

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Align unattended admission with Kiro’s complete native and namespaced trust list.
• Preserve read-only containment by continuing to reject write and shell tools.
• Log rejected permission-frame tool metadata for actionable launch diagnostics.

Diagram

graph TD
    A["Kiro Reviewer"] --> B["Permission Frame"] --> C{"Tool trusted?"} -->|Yes| D{"Allow option?"} -->|Yes| E["Auto Approve"]
    C -->|No| F["Log and Reap"]
    D -->|No| F

Loading

High-Level Assessment

The chosen approach is appropriate: admission now matches the launch’s established trust boundary while retaining complete-title matching and fail-closed rejection. Special-casing fs_read would risk future drift, while blanket auto-approval would weaken containment; equality tests protect the shared trust invariant.

Files changed (3) +87 / -22

Bug fix (2) +42 / -16

AcpInteractionBridge.csLog tool context when reaping forbidden permission frames +25/-10

Log tool context when reaping forbidden permission frames

• Introduces a shared reap path for rejected allowlisted permission requests. The path records the agent-supplied tool title and kind while preserving the existing coded failure reason and cancellation behavior.

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

UnattendedToolAdmission.csInclude native trusted tools in unattended admission +17/-6

Include native trusted tools in unattended admission

• Builds the admitted set from both Kiro’s native tools and injected namespaced tools, making admission equal the launch trust list. Complete-title matching remains unchanged, so untrusted write and shell tools continue to fail closed.

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

Tests (1) +45 / -6

UnattendedToolAdmissionTests.csCover full trust-list admission and containment boundaries +45/-6

Cover full trust-list admission and containment boundaries

• Updates the trust-equivalence assertion to include native tools and adds cases admitting fs_read and thinking. New negative cases confirm fs_write and execute_bash remain outside the unattended admission boundary.

test/Capacitor.Cli.Daemon.Tests.Unit/Acp/UnattendedToolAdmissionTests.cs

[qodo-code-review]

Code Review by Qodo

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

1. AdmittedFor couples shared ACP ✗ Dismissed 📘 Rule violation ⚙ Maintainability

Description

AdmittedFor now derives shared ACP admission data directly from Kiro’s vendor-specific trust-list
implementation. This violates the requirement to confine vendor-specific implementation changes to
Harness/<Vendor>/.

Code

src/Capacitor.Cli.Daemon/Acp/UnattendedToolAdmission.cs[R53-55]

+        KiroReviewerTrustList.NativeTools
+            .Concat(KiroReviewerTrustList.NamespacedEntries(injected, identity))
+            .ToHashSet(StringComparer.Ordinal);

Evidence

Rule 2 requires vendor-specific implementations to remain in their Harness/<Vendor>/ directory.
The changed shared ACP method directly references KiroReviewerTrustList.NativeTools and
NamespacedEntries, both supplied by the Kiro harness implementation.

CLAUDE.md: Isolate Vendor-Specific Harness Code to Harness/<Vendor>/ Directories and Keep Shared Code Outside
src/Capacitor.Cli.Daemon/Acp/UnattendedToolAdmission.cs[51-55]
src/Capacitor.Cli.Daemon/Harness/Kiro/KiroReviewerTrustList.cs[31-61]
src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntimeFactory.cs[233-240]

Agent prompt

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

## Issue description
Shared ACP admission code directly derives its identities from the Kiro-specific `KiroReviewerTrustList`.

## Issue Context
The generic ACP runtime factory and interaction bridge consume this admission set, while Kiro merely selects the applicable policy through its vendor descriptor. Keep the generic exact-title admission check shared, but derive Kiro’s trusted identities inside `Harness/Kiro/` and pass them through the vendor configuration.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Acp/UnattendedToolAdmission.cs[51-55]
- src/Capacitor.Cli.Daemon/Harness/Kiro/KiroReviewerTrustList.cs[31-61]
- src/Capacitor.Cli.Daemon/Acp/AcpVendorDescriptor.cs[345-368]
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntimeFactory.cs[233-240]

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

---

2. New comments over-explain mechanics ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The new XML summaries and logging comments span multiple lines narrating implementation details,
examples, and PR history that the code already expresses. This conflicts with the requirement that
comments remain concise and focus only on non-obvious rationale or constraints.

Code

src/Capacitor.Cli.Daemon/Acp/UnattendedToolAdmission.cs[R45-48]

+    /// <para>The native half is load-bearing: the trust argv grants <c>fs_read</c>/<c>thinking</c>
+    /// natively, but the vendor leaks prompts even for tools in its own trust list (measured on Kiro),
+    /// so a workspace-backed launch that reads the mirrored worktree can raise a permission frame for
+    /// <c>fs_read</c>. Admitting only the namespaced half left that frame un-admittable — reaping the

Evidence

Rule 11 requires short comments centered on non-obvious rationale. The cited additions repeatedly
describe control flow, trusted tool examples, launch failure history, and behavior already encoded
by the adjacent methods and tests.

CLAUDE.md: Comments Must Be Concise and Explain Non-Obvious “Why” (Avoid Verbose or Paraphrasing Comments)
src/Capacitor.Cli.Daemon/Acp/AcpInteractionBridge.cs[78-85]
src/Capacitor.Cli.Daemon/Acp/AcpInteractionBridge.cs[560-564]
src/Capacitor.Cli.Daemon/Acp/UnattendedToolAdmission.cs[39-50]
test/Capacitor.Cli.Daemon.Tests.Unit/Acp/UnattendedToolAdmissionTests.cs[119-124]

Agent prompt

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

## Issue description
Several newly added comments are verbose and paraphrase implementation behavior rather than briefly documenting the non-obvious constraint.

## Issue Context
Retain essential rationale such as admission equaling trust, exact-title matching, and log fields being untrusted. Remove incident narration, repeated mechanics, and details already apparent from method bodies or tests.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Acp/AcpInteractionBridge.cs[78-85]
- src/Capacitor.Cli.Daemon/Acp/AcpInteractionBridge.cs[560-564]
- src/Capacitor.Cli.Daemon/Acp/UnattendedToolAdmission.cs[39-50]
- test/Capacitor.Cli.Daemon.Tests.Unit/Acp/UnattendedToolAdmissionTests.cs[90-94]
- test/Capacitor.Cli.Daemon.Tests.Unit/Acp/UnattendedToolAdmissionTests.cs[119-124]

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

---

Context sources

Review mode: ⚖️ Balanced: This changes unattended permission admission and reviewer launch/reaping behavior, a security-sensitive trust boundary with meaningful behavioral risk, but the logic is localized enough that one careful review pass should suffice.

Tip of the day

💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗