Sign up
View stargazersSign up

fix(acp): questions from ACP agents show their titled choices#21770

closed1
main
GitHub fork: fix/acp-titled-question-choices
opened23d agobyGGeorgieSmile
2 files changed
View on GitHubTwo-way sync enabledPR last imported 1d ago

Problem

When an ACP agent asks a question whose choices are titled oneOf options (or items.anyOf for multi-select), T3 Code shows the question with no choices. Only plain enum choices were converted, so the agent's valid choices were dropped.

Repro: add OMP (v18.12.1) as a local ACP provider, then ask it to use its question tool with options Alpha, Beta and Gamma. Only "Which test option?" appears, with nothing to pick.

Change

The ACP adapter now also reads titled oneOf / items.anyOf choices. It shows each choice's title and description, and sends the original const value back to the agent through the existing option.value path (the web composer already submits option.value ?? option.label). Plain enum and boolean questions behave as before.

Scope and approval

Small, focused fix for an obvious bug: the agent sends valid choices and T3 Code drops them. It changes one function in the ACP adapter and adds its tests.

Verification

  • vp test run in packages/provider-acp: 114 passed, including 4 new tests (titled single-select, titled and plain multi-select, existing enum/boolean questions, malformed options).
  • tsc-rs --noEmit in packages/provider-acp: no errors.
  • vp lint and vp fmt --check on the changed files: clean.
  • Manual, dev desktop build on macOS with OMP over ACP, same prompt before and after the change: before, the question showed no choices; after, it shows Alpha, Beta and Gamma with their descriptions.
  • Not checked: mobile, and multi-select in a live session (unit tests only).

Known, separate issue (not changed here): OMP also sends an optional "Other (type your own)" field, and T3 Code requires every field to be answered, so Submit stays disabled until that field has text. I'll report it separately.

BeforeAfter
Before: question shows no choicesAfter: Alpha, Beta and Gamma with descriptions

Written with Claude Opus 5.5 in Claude Code, run from T3 Code.

Checks

7 successful · 10 skipped
All checks passed
4a3e904

Added 1 commit

  • fix(acp): questions from ACP agents show their titled choices4a3e904
MmacroscopeappBot23d ago
Synced from GitHub

Approvability

Verdict: Approved at 4a3e904

Macroscope's review found this PR approvable — This is a focused ACP elicitation bug fix that restores titled choice labels, descriptions, exact values, and existing multi-select handling without changing defaults or public schemas. The production logic is isolated and supported by targeted regression tests.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot23d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c80b59fb-4f29-49d3-be0d-1c852a625a5a
📥 Commits

Reviewing files that changed from the base of the PR and between 34c5a6d4461ff7be2c1a059c3c51c3496c1e14df and 4a3e9040fbe69654d62388829531185c3d4e5c91.

📒 Files selected for processing: 2
  • packages/provider-acp/src/server/adapter.ts
  • packages/provider-acp/src/server/elicitation.test.ts
📝 Walkthrough

The ACP adapter now exports acpElicitationQuestions to map form-schema properties to user-input questions. The form elicitation handler uses this helper. Tests cover choice variants, fallback labels and descriptions, booleans, custom text, and malformed options.

Changes: ACP elicitation questions

Layer / File(s)Summary
Question mapping and handler integration
packages/provider-acp/src/server/adapter.ts, packages/provider-acp/src/server/elicitation.test.ts		The adapter exports acpElicitationQuestions and uses it in the form elicitation handler. Tests cover single- and multi-select choices, fallback labels and descriptions, booleans, custom text, and malformed options.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 4a3e9

Titled choices and multi-select answers follow the existing submission path without an identified merge-blocking issue.

### Architecture Summary

Architecture risk: 🔵 Low · up to 4a3e9

The change affects 1 system.

Changed systems: packages/provider-acp

Architecture concerns No architecture-level concerns identified.

Review details

Systems and components

  • observed — packages/provider-acp (library) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in packages/provider-acp/src/server/adapter.ts: Added acpElicitationQuestions to map schema properties into user-input questions, including enum choices, titled oneOf choices, array items.anyOf choices marked multi-select, and boolean options. Titles and descriptions fall back to the question index, message, or option label when empty.
  • observed — Modified behavior in packages/provider-acp/src/server/adapter.ts: The form elicitation handler now calls acpElicitationQuestions instead of building questions inline. The removed mapper supported enums and booleans but did not handle titled choices or multi-select arrays.
  • observed — Modified behavior in packages/provider-acp/src/server/elicitation.test.ts: Imports the test helpers and acpElicitationQuestions for the new test suite.
  • observed — Modified behavior in packages/provider-acp/src/server/elicitation.test.ts: Adds expectations for a titled single-select question: choice titles become labels, values are retained, and a missing description falls back to the title.
Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely identifies the ACP bug fix. It states that titled choices from ACP agent questions are now shown.
Description checkPassedThe description includes all required sections: Problem, Change, Scope and approval, and Verification. It provides reproduction details, implementation scope, test results, manual verification, limita…
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description includes all required sections: Problem, Change, Scope and approval, and Verification. It provides reproduction details, implementation scope, test results, manual verification, limitations, screenshots, and agent attribution.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.