Repository navigation
Diagnose and surface silently refused ACP permissions - #879
Merged
Merged
Conversation
The bridge already fails closed to `cancelled` when a decision's optionId matches none offered; only the mismatch itself was invisible in the log. The turn-end note is keyed on that same wire outcome, not the raw decision, so it covers the fail-closed path too.
PR Summary by QodoDiagnose ACP permission mismatches and surface silent refusals
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
1. Hidden thinking leaves the chat blank
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+1854
to
+1855
| if (envelope.Kind is AcpEventKind.AssistantText or AcpEventKind.AssistantThinking) | ||
| Volatile.Write(ref _turnHadAssistantOutput, 1); |
There was a problem hiding this comment.
1. Hidden thinking leaves the chat blank 🐞 Bug ≡ Correctness
EmitEnvelope sets _turnHadAssistantOutput for AssistantThinking, so the turn-end check treats internal reasoning as a visible reply. ChatTabViewModel.Apply has no case for that event kind, so a cancelled permission followed only by thinking still leaves the chat blank and suppresses the new explanation.
Agent Prompt
## Issue description
Assistant-thinking envelopes currently suppress the silent-permission-refusal note even though the chat does not render those envelopes, leaving the user with no visible explanation.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1355-1358]
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1852-1855]
- test/Capacitor.Cli.Daemon.Tests.Unit/Services/AcpHostedAgentRuntimePermissionTests.cs[360-394]
## Recommended Fix
Set the visibility flag only for assistant event kinds actually rendered in chat, currently `AssistantText`. Add a cancelled-permission test with thinking-only output and assert that the system note is still emitted.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1321
to
+1324
| // Reset THIS turn's silent-permission-refusal signal (see the finally block below) — | ||
| // a prior turn's cancelled permission or assistant text must never leak into this one. | ||
| Volatile.Write(ref _turnHadAssistantOutput, 0); | ||
| Volatile.Write(ref _turnPermissionCancelled, 0); |
There was a problem hiding this comment.
4. Late events mislabel the next turn 🐞 Bug ☼ Reliability
_turnHadAssistantOutput and _turnPermissionCancelled are process-wide flags reset per admitted turn, while EmitEnvelope and NotePermissionOutcome mutate them without checking the active turn or incarnation. If a late update or swept permission response from the previous incarnation runs after the next reset, it can suppress the next turn's refusal note or make that turn emit a note for a permission it never requested.
Agent Prompt
## Issue description
The new refusal and assistant-output flags are shared across the runtime and can be changed by asynchronous events that do not belong to the currently executing turn.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1321-1324]
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1852-1855]
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[2764-2768]
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[2834-2842]
## Recommended Fix
Store the refusal state in a per-turn tracker carrying the active incarnation or generation. Update it only when the emitting connection or permission route matches that tracker, and add tests where an old-incarnation interaction settles after a successor turn begins.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
A transport drop declines the pending permission as cancelled and runs the turn-end finally, so the note is now emitted only for a turn that reached its stopReason. Agent-controlled option ids/kinds are stripped of control chars and capped before logging.
Collaborator
Author
|
Ran a Codex review (
Qodo's three comment-convention notes are addressed in the same commit. The root cause — the kcap-server permission card echoing the wrong optionId — remains a separate cross-repo fix; this PR makes it diagnosable and stops the blank chat. |
This was referenced Sep 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #851 — AI-2677
What & why
Launching Copilot from the desktop app, a permission approved in the web UI resolved as
cancelled, the tool returned "The user rejected this tool call", and the turn ended with nothing in the chat.AcpInteractionBridge.MapPermissionDecisionis fail-closed: an affirmative outcome maps to allow only when itsSelectedOptionIdmatches an option the agent offered; anything else iscancelled. The daemon logged neither the offered ids nor the received decision, so the mismatch was invisible, and a refused turn left the chat blank.Two kcap-cli changes: the bridge now logs the offered options as
optionId:kindpairs when a request is issued and the received(outcome, selectedOptionId, matched)when it resolves, so an id echoed back that matches nothing offered showsmatched=falsein the daemon log. And an ACP turn that ends after a permission is cancelled with no assistant output emits a system note ("The tool call was not permitted; the turn ended.") so the chat shows why it stopped.Not in scope (cross-repo): the actual root cause is the kcap-server web UI permission card echoing back a different
optionIdthan Copilot offered. This PR makes that failure diagnosable and stops the blank chat; the card itself is a separate kcap-server fix.Where to look
The offered/received option ids are the one deliberately non-payload-free permission log — they are the diagnostic this exists for — logged narrowly (ids and kinds, never labels or tool args). The turn-end note keys on the ACP
cancelledresponse shape, which covers both a genuine cancel and the fail-closed mismatch.Verification
matched=false, reproducing the Copilot shape; a matching id logsmatched=true) and runtime tests (cancelled + no assistant output emits the note; with assistant output or an allow, no note). TDD sanity check confirmed the runtime test fails without the emission.