Skip to content

fix(acp): signal QuestionNotSupported instead of resolving empty answers#2507

Open
ayaangazali wants to merge 1 commit into
MoonshotAI:mainfrom
ayaangazali:fix-acp-question-not-supported
Open

fix(acp): signal QuestionNotSupported instead of resolving empty answers#2507
ayaangazali wants to merge 1 commit into
MoonshotAI:mainfrom
ayaangazali:fix-acp-question-not-supported

Conversation

@ayaangazali

@ayaangazali ayaangazali commented Jul 18, 2026

Copy link
Copy Markdown

Related Issue

Resolve #2495

Description

In ACP server mode every QuestionRequest gets resolved with an empty dict (src/kimi_cli/acp/session.py line 211), which is indistinguishable from the user actually dismissing the question. The model receives "User dismissed the question without answering" even though the ACP client never rendered anything, so it can retry the tool or just give up on getting an answer.

This applies the short term fix from the issue: signal QuestionNotSupported instead. It's the same thing the Wire server already does for clients without the supports_question capability (src/kimi_cli/wire/server.py line 1055), and it routes into the existing fallback in the ask_user tool which tells the model to ask the question in plain text instead. So over ACP (Zed, JetBrains, etc) the question actually reaches the user now, just as regular text.

One side effect worth calling out: EnterPlanMode/ExitPlanMode also send QuestionRequest, so over ACP they now return their "client does not support plan mode" ToolError instead of a phantom dismissal. That matches how Wire clients without question support already behave, so I think it's the right call, but flagging it in case you see it differently.

Follow up ideas from the issue (out of scope here): hide the AskUserQuestion tool in ACP mode like _sync_ask_user_tool_visibility does for Wire, or map questions onto session/request_permission. Happy to take a stab at either if there's interest.

Test: added tests/acp/test_session_questions.py which runs a prompt turn through ACPSession with a fake CLI yielding a QuestionRequest and asserts the request resolves with QuestionNotSupported.

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked the related issue, if any.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have run make gen-changelog to update the changelog. (i updated CHANGELOG.md by hand in the same style, the gen-changelog skill needs a Kimi API setup i don't have)
  • I have run make gen-docs to update the user documentation. (checked docs/, nothing describes ACP question behavior so there was nothing to regenerate)

small disclosure: i'm a freshman in college and i talked the logic through with claude code before writing this. the diff is tiny but if i misread how any of this works i would honestly love to know, still learning :)


Open in Devin Review

Copilot AI review requested due to automatic review settings July 18, 2026 10:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

Resolving QuestionRequest with an empty dict is indistinguishable from
a real user dismissal, so over ACP the model saw a phantom dismissal
and could retry the tool. Raising QuestionNotSupported routes into the
existing fallback that tells the model to ask in plain text, matching
what the Wire server already does for clients without the
supports_question capability.
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.

ACP: AskUserQuestion/QuestionRequest resolves empty — structured questions unusable over ACP

2 participants