Skip to content

[Fix] Discord prompts omit buttons for multi-question input#743

Draft
roomote-roomote[bot] wants to merge 8 commits into
developfrom
fix/discord-elicitation-buttons-0vyci4qtrsdp6
Draft

[Fix] Discord prompts omit buttons for multi-question input#743
roomote-roomote[bot] wants to merge 8 commits into
developfrom
fix/discord-elicitation-buttons-0vyci4qtrsdp6

Conversation

@roomote-roomote

@roomote-roomote roomote-roomote Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Opened on behalf of Daniel Riccio. Follow up by mentioning @roomote-roomote, in the web UI, or in Discord.

What changed

Discord multi-question input prompts now show one current question with its option buttons, rather than rendering the entire form as text-only. Telegram and Teams retain their complete multi-question text prompts until their handlers gain the same wizard flow. Telegram single-question prompts retain their option and Cancel buttons.

Why this change was made

The shared publishing path must preserve each provider's existing interaction contract while Discord adopts its button-driven wizard.

Impact

Discord users receive the wizard flow. Telegram and Teams continue to show all questions for one structured text reply, while Telegram single-question prompts remain button-driven. Publisher regression coverage verifies each provider path.

@roomote-roomote

roomote-roomote Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

No new code issues found. See task

  • packages/communication/src/discord-request-user-input.ts:222 Multi-question prompts with 21 or more options no longer include a Cancel button — dismissed: option buttons are capped at 20, reserving the fifth Discord row for Cancel; regression coverage now verifies this with 21 options.
  • packages/sdk/src/server/lib/communication-request-user-input.ts:101 Telegram single-question prompts retain their option and Cancel buttons.
  • packages/sdk/src/server/lib/communication-request-user-input.ts:104 Empty question arrays retain Telegram's Cancel button, so users can dismiss the prompt even though text-reply parsing rejects zero questions.

Reviewed 66dba05

@roomote-roomote

roomote-roomote Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the review finding: the reported Cancel-button loss does not occur because options are capped at 20, reserving the fifth Discord row for Cancel. Added a 21-option regression test to lock that behavior down in 1da97f2.

@daniel-lxs

Copy link
Copy Markdown
Member

Review

Overview

Multi-question request_user_input prompts in Discord previously rendered every question as text with only a Cancel button, forcing users to type "one answer per line." This PR switches to a wizard flow: the prompt message shows only the current question with its option buttons, each answer atomically advances currentQuestionIndex in Redis (accumulating answers), the prompt message is edited in place to show the next question, and the final answer submits the full accumulated answer set. Both the button-click path and the typed-reply path support the flow.

What's solid (verified in the code)

  • Concurrency handling is correct. advancePendingCommunicationRequestUserInputQuestion reuses the existing Lua claim script with expectedQuestionIndex as a compare-and-swap guard, so concurrent replies/clicks on the same question can't double-advance — losers get the "already received" notice. The stale-button guard (questionIndex !== current.questionIndex → "that option is for an earlier question") covers clicks on an outdated prompt render.
  • Answer accumulation is right in both paths: previous pendingRequest.answers are spread before the new answer, and the final finalizeDiscordRequestUserInputAnswer enqueues the full set.
  • Parsing typed replies against [current.question] instead of all questions is the correct counterpart change, and it keeps the isOther free-text fallback rejection (conversational interjections still fall through to normal follow-up).
  • The 21-option Cancel-row test is good regression coverage for the pre-existing slice(0, 20) + Cancel-row + slice(0, 5) cap logic.
  • CI fully green (lint, types, tests, knip, builds).

Issues

1. Unguarded editMessage after a successful advance claim (both paths) — main concern.
finalizeDiscordRequestUserInputAnswer wraps its editMessage in try/catch with a warn + fallback reply. The two new advance blocks in apps/api/src/handlers/discord/request-user-input.ts call provider.editMessage bare. If the prompt message was deleted or the edit fails, the exception propagates after Redis has already advanced the question index: the user's answer is recorded but they never see the next question, the "Picked:" confirmation never posts, and in the callback path the deferred interaction is left hanging. Recommend mirroring finalize: try/catch, log a warning, and fall back to posting the next-question prompt as a new reply.

2. Dead end when promptMessageId is missing mid-form.
In the callback path the user gets "Your answer was saved. Reply in the thread to continue." — but the next question is never rendered anywhere, so they don't know what to answer. The typed-reply path is worse: it just says "Picked: X" with no next question at all. Their next message will be parsed blind against question N+1's options. Rare case (only when the original prompt post failed), but the fix is cheap: include the next question's prompt text in the reply (or post a fresh prompt message). This same fallback would also serve issue 1.

3. No handler-level tests for the new flow.
The new tests cover only the pure rendering helpers in packages/communication. All the new branching — advance-claim win/lose, prompt re-render, finalize-on-last-question, accumulated answers — lives in apps/api/src/handlers/discord/request-user-input.ts, which has no test file even though the sibling __tests__/ directory has 16 of them with established mocking patterns. At minimum: button answer advances and edits the prompt; last answer submits accumulated answers; losing claim posts the already-received notice.

Minor notes

  • Copy nit: a losing advance claim mid-form posts "I already received your answer. Please wait for the agent to continue." — misleading when the form just moved on to the next question and the user should keep answering.
  • Duplication: the ~45-line "claim advance → edit prompt → confirm" block is nearly identical in the message and callback handlers. Worth extracting a shared helper — it would also give issues 1 and 2 a single place to fix.
  • Final-message display: the edited prompt ends up showing only the last question and its answer; earlier answers survive only as ephemeral/thread confirmations. The agent gets everything, so this is UX polish only.
  • Transitional edge: pending prompts created before deploy still display "one answer per line" instructions; such a multi-line reply will now usually fail single-question parsing and fall through to the agent as a normal follow-up. Self-healing, not blocking.
  • Mid-form Cancel submits answers: {}, discarding already-given answers — consistent with single-question Cancel semantics, just noting it's intentional-looking.

Verdict

The core design is right — atomic per-question claims, correct answer accumulation, proper stale-interaction guards. I'd ask for issue 1 (try/catch around the advance-path editMessage) before merge since it can strand a user mid-form with their answer half-recorded, and ideally a handler test or two. A quick local E2E of a 2-question button flow through the mock Discord harness would be a good final check.

@roomote-roomote

roomote-roomote Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the requested robustness gaps in 7c49ee9: prompt advancement is centralized, failed or missing prompt edits return the next question, and handler tests cover advance, final submission, lost claims, and edit failures.

Follow-up in 19ef5eb: fallback replies include the next question's buttons.

Cross-provider correction in 9a0248d: Telegram and Teams retain their full multi-question text prompts and no callback buttons; 792956d adds publisher-level regression coverage.

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