Skip to content

Render AskUserQuestion as an answerable card in the desktop app - #703

Merged
alexeyzimarev merged 26 commits into
mainfrom
alexeyzimarev/ai-2361-desktop-shell-elicitation-questions-for-pty-sessions-through
Aug 31, 2026
Merged

alexeyzimarev merged 26 commits into
mainfrom
alexeyzimarev/ai-2361-desktop-shell-elicitation-questions-for-pty-sessions-through

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Member

AI-2361 — no GitHub issue exists for this one, so the closing half is dropped.

What & why

A PTY Claude session blocked on AskUserQuestion showed a useless Allow / Allow always / Deny card in the desktop app, even though the request already rides the permission lane end to end (the PermissionRequest hook payload reaches the app as a pending permission entry, and the web answers it with allow plus updatedInput.answers). The app now classifies those entries and renders a question card — every question in the payload, option buttons or checkboxes, an Other free-text per question, click-to-answer for a single single-select — answering over the existing permission/1 wire. No daemon, CLI, or wire change, so it works against any daemon already advertising permission/1; the tray splits its wording ("1 question waiting"), and a settlement from any surface clears card, pips and tray as before.

Where to look

ClaudeElicitation in Core owns the whole contract: a strict, capped, parser-created immutable model plus a composer that validates every answer against it — that pairing is what bounds the composed resolve frame. Anything unparseable or over-cap falls back to the plain permission card, with "Allow always" hidden for the question tool. The spec riding this PR records the decisions, including why the questions passthrough is byte-faithful in the composer but semantic at the wire.

Verification

  • dotnet test --solution Capacitor.slnx: 10762 total, 10695 passed, 66 skipped, 1 failed — Installed_codex_schema_matches_the_vendored_pin, a machine-local codex 0.150.1 vs vendored 0.147.0 pin drift, unrelated to this diff.
  • dotnet publish src/Capacitor.Cli/Capacitor.Cli.csproj -c Release 2>&1 | grep -E 'IL[23][01][0-9]{2}' → no output.
  • New coverage: 35 ClaudeElicitationTests (parse, compose, caps at their boundaries, JSON-element ownership past GC, a maximal worst-case-escaped payload round-tripped through FrameCodec); service classification matrix, answer/ack/push interleavings and single-snapshot summary; question-card fast path, single flight and dispose-mid-flight; mixed NEEDS YOU row headless smoke; tray wording variants.

🤖 Generated with Claude Code

@linear-code

linear-code Bot commented Aug 31, 2026

Copy link
Copy Markdown

AI-2361

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-08-31T07:17:22.275206Z 338db6b PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Render AskUserQuestion as an answerable desktop card

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Classifies valid Claude AskUserQuestion requests and safely composes bounded, validated answers.
• Renders multi-question controls and submits answers over the existing permission protocol.
• Distinguishes pending questions in tray messaging while preserving shared settlement behavior.
Diagram

sequenceDiagram
    participant Daemon as Permission Daemon
    participant Service as Permission Service
    participant Core as Elicitation Core
    participant UI as Question Card
    participant Tray as Tray Summary
    actor User
    Daemon->>Service: Pending entry
    Service->>Core: Parse tool input
    Core-->>Service: Questions model
    Service-->>UI: Render question card
    Service-->>Tray: Publish split counts
    User->>UI: Select answers
    UI->>Service: AnswerAsync
    Service->>Core: Compose answers
    Core-->>Service: updatedInput
    Service->>Daemon: Allow resolve
    Daemon-->>Service: Ack or settlement
    Service-->>UI: Evict settled card
    Service-->>Tray: Clear attention
Loading
High-Level Assessment

The app-side classification and reuse of permission/1 is the best approach because AskUserQuestion already traverses that path with updatedInput support. A new question frame family was considered but would duplicate daemon, Core, and compatibility work while still requiring fallback behavior for existing installations.

Files changed (20) +3196 / -77

Enhancement (7) +488 / -39
IPermissionService.csExpose classified questions and answer summaries +14/-0

Expose classified questions and answer summaries

• Classifies Claude AskUserQuestion entries into parsed question models. Adds a consistent permission/question summary and the AnswerAsync service contract.

src/Capacitor.App/Services/IPermissionService.cs

PermissionService.csResolve elicitation answers through permission/1 +20/-2

Resolve elicitation answers through permission/1

• Composes question answers into allow resolves, shares acknowledgement and tombstone handling with permission decisions, and publishes split pending counts.

src/Capacitor.App/Services/PermissionService.cs

ChatTabViewModel.csCreate mixed pending card view models +5/-3

Create mixed pending card view models

• Branches pending entries into permission or question cards while retaining shared ordering, disposal, and collection behavior.

src/Capacitor.App/ViewModels/ChatTabViewModel.cs

QuestionCardViewModel.csImplement answerable question-card state +160/-0

Implement answerable question-card state

• Models question groups, option selection, per-question Other text, fast-path submission, all-answer gating, single-flight sends, and transport errors.

src/Capacitor.App/ViewModels/QuestionCardViewModel.cs

TrayViewModel.csSplit tray wording for questions and permissions +32/-32

Split tray wording for questions and permissions

• Consumes the consistent pending summary, preserves aggregate attention behavior, and renders separate question, permission, and consent phrases.

src/Capacitor.App/ViewModels/TrayViewModel.cs

ChatTabView.axamlRender interactive question cards +71/-2

Render interactive question cards

• Adds typed templates for question groups, option buttons, checkboxes, Other inputs, inline fast-path answers, Submit, selection styling, and errors.

src/Capacitor.App/Views/ChatTabView.axaml

ClaudeElicitation.csParse and compose Claude elicitation payloads +186/-0

Parse and compose Claude elicitation payloads

• Adds immutable capped models and strict AskUserQuestion parsing. Validates complete answer sets and composes detached updatedInput JSON while preserving original question bytes.

src/Capacitor.Cli.Core/ClaudeElicitation.cs

Bug fix (1) +9 / -36
PermissionCardViewModel.csReuse the card base and restrict Allow always +9/-36

Reuse the card base and restrict Allow always

• Moves shared lifecycle state into PendingCardViewModel. Hides Allow always for AskUserQuestion fallback cards to avoid persistent question-tool approval.

src/Capacitor.App/ViewModels/PermissionCardViewModel.cs

Refactor (1) +55 / -0
PendingCardViewModel.csExtract the shared pending-card lifecycle +55/-0

Extract the shared pending-card lifecycle

• Introduces a common card base for identity, ordering, busy/error state, disposables, and safe no-op updates after eviction.

src/Capacitor.App/ViewModels/PendingCardViewModel.cs

Tests (8) +638 / -2
ChatTabViewModelTests.csTest mixed pending-card classification +21/-2

Test mixed pending-card classification

• Verifies question and permission entries create the correct ordered card types and question eviction leaves the permission card intact.

test/Capacitor.App.Tests.Unit/ChatTabViewModelTests.cs

ChatTabViewSmokeTests.csSmoke-test question-card rendering +23/-0

Smoke-test question-card rendering

• Confirms question options, Other input, prompt text, and a normal permission card coexist in the headless Avalonia view.

test/Capacitor.App.Tests.Unit/ChatTabViewSmokeTests.cs

FakePermissionService.csSupport elicitation scenarios in permission tests +27/-0

Support elicitation scenarios in permission tests

• Adds question-entry fixtures, split summaries, scripted AnswerAsync outcomes, and captured answer payloads to the test double.

test/Capacitor.App.Tests.Unit/FakePermissionService.cs

PermissionCardViewModelTests.csTest AskUserQuestion fallback permissions +13/-0

Test AskUserQuestion fallback permissions

• Verifies unclassified AskUserQuestion fallback cards suppress the Allow always action.

test/Capacitor.App.Tests.Unit/PermissionCardViewModelTests.cs

PermissionServiceTests.csTest classification, answer transport, and settlement +111/-0

Test classification, answer transport, and settlement

• Covers the classification matrix, resolve payload shape, validation failures, transport outcomes, ack/push races, tombstones, and consistent summaries.

test/Capacitor.App.Tests.Unit/PermissionServiceTests.cs

QuestionCardViewModelTests.csTest question-card interactions and lifecycle +165/-0

Test question-card interactions and lifecycle

• Covers fast paths, free text, multi-question gating, exclusive selection, single-flight behavior, retries, exceptions, and disposal during submission.

test/Capacitor.App.Tests.Unit/QuestionCardViewModelTests.cs

TrayViewModelTests.csTest pending-question tray wording +28/-0

Test pending-question tray wording

• Verifies question-only and mixed headers assert attention and clear after all pending entries settle.

test/Capacitor.App.Tests.Unit/TrayViewModelTests.cs

ClaudeElicitationTests.csTest elicitation parsing and composition bounds +250/-0

Test elicitation parsing and composition bounds

• Exercises malformed inputs, caps, immutability, JSON ownership and byte preservation, answer validation and ordering, and maximal FrameCodec round trips.

test/Capacitor.Cli.Core.Tests.Unit/ClaudeElicitationTests.cs

Documentation (3) +2006 / -0
CHANGES.mdDocument desktop elicitation question cards +14/-0

Document desktop elicitation question cards

• Adds the user-facing feature record, including existing-wire compatibility, bounded Core parsing/composition, and fallback behavior for invalid payloads.

docs/CHANGES.md

2026-08-30-ai2361-elicitation-question-cards.mdAdd the elicitation implementation plan +1578/-0

Add the elicitation implementation plan

• Provides the task-by-task implementation, testing, protocol, UI, and AOT verification plan for AI-2361.

docs/superpowers/plans/2026-08-30-ai2361-elicitation-question-cards.md

2026-08-30-ai2361-elicitation-question-cards-design.mdDefine the elicitation card design +414/-0

Define the elicitation card design

• Documents classification, answer shape, payload caps, interaction behavior, settlement semantics, edge cases, and rejected protocol alternatives.

docs/superpowers/specs/2026-08-30-ai2361-elicitation-question-cards-design.md

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 338db6b896

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +74 to +75
var s = obj.Str(name)?.Trim();
return s is { Length: > 0 } && s.Length <= maxChars ? s : null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve protocol strings after validating whitespace

When a valid question or option label contains leading or trailing whitespace, ProtocolString returns the trimmed value rather than the original protocol value. The composer consequently emits a different answer-map key or selected label from the one in the verbatim questions array, so Claude cannot reliably associate the answer with that question or option; measuring only the trimmed result also lets an over-cap raw string pass. Use trimming only to test for blank content, while preserving and applying the cap to the original string.

Useful? React with 👍 / 👎.

@qodo-code-review

qodo-code-review Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Comment preserves verification history ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The touched TrayViewModel comment records that behavior was decompile-verified, which is
review-time evidence rather than a durable constraint understandable from current source. Keeping
this historical aside makes the comment stale and violates the current-only comment requirement.
Code

src/Capacitor.App/ViewModels/TrayViewModel.cs[R94-95]

+        // consent.PendingCount is DynamicData's CountChanged, which seeds the current count on
+        // subscribe (decompile-verified) — deliberately NOT StartWith(0)'d, which would inject a
Evidence
Rule 22 requires comments to remain useful from current source alone and forbids review or
change-history narration. The changed comment explicitly preserves the transient verification note
decompile-verified.

CLAUDE.md: Write Only Current, Necessary, Non-Historical Comments
src/Capacitor.App/ViewModels/TrayViewModel.cs[94-95]

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

## Issue description
The comment includes the historical aside `decompile-verified` instead of documenting only the durable observable contract.

## Issue Context
Retain any necessary explanation of why `StartWith(0)` is unsafe, but remove how that behavior was verified and shorten the touched comment where possible.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/TrayViewModel.cs[94-101]

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


2. Implementation plan committed ✗ Dismissed 📘 Rule violation ⚙ Maintainability
Description
The PR adds a dated, task-by-task implementation plan containing branch, commit, and internal
work-tracking instructions. This creates the planning and time-bound repository artifact prohibited
by the checklist.
Code

docs/superpowers/plans/2026-08-30-ai2361-elicitation-question-cards.md[R1-3]

+# Elicitation Question Cards (AI-2361) Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
Evidence
Rule 23 excludes planning and time-bound artifacts. The added file identifies itself as an
Implementation Plan, includes the dated work ID AI-2361, and instructs agents to execute tracked
implementation tasks.

CLAUDE.md: Exclude Plans, Review Artifacts, Closed Work IDs, and Time-Bound Narration From Comments
docs/superpowers/plans/2026-08-30-ai2361-elicitation-question-cards.md[1-3]

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

## Issue description
A dated implementation plan and its internal work-tracking instructions were committed as a repository artifact.

## Issue Context
Compliance rule 23 excludes plans, review artifacts, closed work IDs, and time-bound narration. Keep only durable user or architecture documentation that describes the current system without task checklists, branch/commit instructions, dates, or internal tracker references.

## Fix Focus Areas
- docs/superpowers/plans/2026-08-30-ai2361-elicitation-question-cards.md[1-1578]

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


3. Assertions have redundant comments ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The trailing comments merely restate that the asserted Allow and A labels represent a permission
card and a question option. They add no non-obvious behavioral contract and violate the requirement
to omit comments that only narrate the code.
Code

test/Capacitor.App.Tests.Unit/ChatTabViewSmokeTests.cs[R494-495]

+            await Assert.That(buttons).Contains("Allow");           // the permission card
+            await Assert.That(buttons).Contains("A");               // a question option button
Evidence
Rule 22 permits comments for non-obvious traps or constraints, not comments that restate code. The
comments the permission card and a question option button only paraphrase the adjacent label
assertions.

CLAUDE.md: Write Only Current, Necessary, Non-Historical Comments
test/Capacitor.App.Tests.Unit/ChatTabViewSmokeTests.cs[494-495]

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

## Issue description
Two trailing test comments only label the UI values asserted on the same lines.

## Issue Context
The test name and assertions already establish that permission and question cards coexist. Preserve the assertions while deleting comments that merely restate them.

## Fix Focus Areas
- test/Capacitor.App.Tests.Unit/ChatTabViewSmokeTests.cs[494-495]

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



Remediation recommended

4. Trimmed keys mismatch questions ✓ Resolved 🐞 Bug ≡ Correctness
Description
ProtocolString trims question text and option labels, but ComposeAnswers replays the original
untrimmed questions while using those trimmed strings as answer keys and values. A valid question
such as "  Pick one  " is therefore returned under "Pick one", so the answer no longer
corresponds to the original question contract.
Code

src/Capacitor.Cli.Core/ClaudeElicitation.cs[R74-75]

+        var s = obj.Str(name)?.Trim();
+        return s is { Length: > 0 } && s.Length <= maxChars ? s : null;
Evidence
The parser stores trimmed protocol strings, while the composer writes the raw retained questions
array and derives answer keys/values from the parsed strings. The PR's own contract specifies that
answer keys are the question texts and values are labels, proving these representations must match.

src/Capacitor.Cli.Core/ClaudeElicitation.cs[72-75]
src/Capacitor.Cli.Core/ClaudeElicitation.cs[103-115]
src/Capacitor.Cli.Core/ClaudeElicitation.cs[128-140]
src/Capacitor.Cli.Core/ClaudeElicitation.cs[148-183]
docs/superpowers/specs/2026-08-30-ai2361-elicitation-question-cards-design.md[48-60]

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

## Issue description
Question text and option labels are trimmed in the parsed model, while the composer passes through the original questions JSON. This can emit answer keys and values that differ from the original protocol strings.

## Issue Context
Whitespace-only protocol strings should still be rejected, but accepted strings must retain their exact values because question text is the answer-object key and option labels are answer values. Apply caps to the retained value rather than returning a normalized value, and add round-trip coverage for leading/trailing whitespace.

## Fix Focus Areas
- src/Capacitor.Cli.Core/ClaudeElicitation.cs[72-75]
- test/Capacitor.Cli.Core.Tests.Unit/ClaudeElicitationTests.cs[125-146]

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


Grey Divider

Context sources
Review mode: ⚖️ Balanced

Grey Divider

Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.App/ViewModels/TrayViewModel.cs Outdated
Comment thread test/Capacitor.App.Tests.Unit/ChatTabViewSmokeTests.cs Outdated
Comment thread src/Capacitor.Cli.Core/ClaudeElicitation.cs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant