Repository navigation
feat: adapt ask_user interactions across channels - #817
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37f75d7399
ℹ️ 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".
| w.MessageID = cb.Message.ID | ||
| } | ||
|
|
||
| ready, needText, toast := applyAskUserCallback(w, parsed) |
There was a problem hiding this comment.
In group chats, any member can press these inline buttons because the wizard contains no sender identity and this path mutates/submits it solely from the callback token. A different member can therefore change the original user's selections; on a final single-select or Submit tap, their synthetic /respond can resolve the request when they have no active session (the explicit-ID resolution path permits an empty session), submitting their answers into the original user's waiting run. Even when their session causes resolution to fail, submitAskUserWizard has already deleted the shared wizard, preventing the requester from completing it.
Useful? React with 👍 / 👎.
| w := a.askUserStore().get(parsed.Token) | ||
| if w == nil { | ||
| _ = bot.Respond(cb, &tele.CallbackResponse{Text: i18n.New("").T("cmd.userInput.expired"), ShowAlert: true}) | ||
| return true |
There was a problem hiding this comment.
After a server restart or adapter reconnect, every outstanding Telegram ask_user card becomes unusable: its callback token only indexes this process-local map, so the newly created adapter returns expired at the w == nil branch. Although the pending request is durable in PostgreSQL, this native path neither restores a wizard from it nor exposes a usable fallback response identifier, leaving the agent run waiting until expiry.
Useful? React with 👍 / 👎.
DingTalk and WeCom smart-robot webhooks only deliver a group message when the bot is @-mentioned, so every inbound is already bot-directed. Neither adapter set is_mentioned, so isDirectedAtBot returned false in groups and handlePlainTextUserInput silently dropped the answer — group ask_user hung forever. Mark user messages (not event callbacks) as mentioned. Guarded by a directed-group fallback regression test.
…on state Remove the process-local ask_user wizard. Telegram inline-button callbacks now carry the request UUID and apply structured ops (select/toggle/set_text/ navigate/submit) to the persistent TextInteractionState via a new userinput.AdvanceInteraction — the same CAS-guarded state machine the plain-text fallback uses. The card is re-rendered from the persisted state. Consequences: - A pending ask_user survives server restarts: state lives in user_input_requests.interaction_json, and the card binds via prompt_external_message_id (persisted on send through UpdatePromptMessage). - Buttons and plain-text replies can no longer desync: one durable state, two input surfaces, optimistic-lock CAS on every write. - Callback acks are silent (no more 'OK' toast) and same-page taps are reported unchanged, so the page indicator never triggers a no-op edit. - The only process-local remainder is the force-reply prompt binding; losing it just means re-tapping the fill-answer button. AdvanceText now shares persistInteraction with AdvanceInteraction so there is exactly one CAS write path.
37f75d7 to
a843b5e
Compare
… only The respond:<id>[:<option>] callback namespace cannot be deleted yet: the shared channel layer (inbound/channel.go buildUserInputActions, pipeline/driver.go) still emits these buttons for every adapter, and chat-flow ask_user requests never expire — a card rendered before this deploy stays tappable indefinitely. What this commit does remove: the bare respond:<id> dispatch path. It only produced a '/respond <id>' hint command with no answer attached; the callback handler already acks it as a silent no-op, so building an inbound message for it was dead weight. buildTelegramCallbackInboundMessage now rejects answer-less respond callbacks outright. Full deletion of the namespace is deferred until the shared action builder emits aui~ ops directly (cross-adapter change, separate PR).
…ck/misskey Pay down the respond: debt at its source: the shared stream mapper no longer fabricates per-option respond: keyboards. It now emits exactly one marker action (Type=user_input) whose only jobs are (a) keeping the show_tool_calls_in_im filter from dropping the pending card and (b) legacy upgrade compat. No adapter other than Telegram ever rendered these buttons, and Telegram replac A289 es them with the aui structured keyboard — the fabricated options were dead weight that could only mislead future adapters into depending on the legacy encoding. New native-button channels should render from the payload and encode their own structured ops, as Telegram does. Also close the remaining plain-text fallback directedness gaps: - slack: flag is_reply_to_bot when the thread root author is the bot (ParentUserId == selfUserID), so thread replies to the bot's ask_user prompt count as bot-directed in channels. - misskey: flag is_reply_to_bot when the replied-to note is the bot's own, so answers without an explicit @-mention are not dropped. QQ needs no change: the platform only delivers @-messages and the adapter already marks them is_mentioned.
Summary
ask_user was only usable on the web UI; on IM channels it degraded to a
/respondslash-command hint or nothing at all. This PR makes ask_user a first-class interaction on all 13 channels, backed by one durable state machine with two input surfaces:user_input_requests.interaction_json+interaction_revision(optimistic-lock CAS). All input surfaces write the sameTextInteractionState; a pending ask_user survives server restarts and cannot desync across surfaces.select/toggle/set_text/navigate/submit) viauserinput.AdvanceInteraction; the card re-renders from persisted state.back/skipcommands (en/zh/ja), per-question prompts, and a submitted summary. Gated by a newNativeUserInputchannel capability.is_mentioned; without this, group replies to ask_user were silently dropped.command_ui_language.Migration
0111_user_input_text_interaction(renumbered from 0108 after rebasing onto main, which took 0108–0110): addsinteraction_json JSONB/interaction_revision INTEGERwith constant defaults — PG11+ fast path, no table rewrite. Idempotent, paired down migration,0001_init.up.sqlupdated.Design notes
AdvanceTextandAdvanceInteractionshare a singlepersistInteractionCAS write path.Known limitations / follow-ups
respond:parsing is kept for upgrade compatibility only. The shared layer now emits a single marker action (filter keepalive) instead of fabricated per-optionrespond:keyboards 8000 the structuredInteractionOppath is the only real button protocol. The Telegram-siderespond:parser stays for one compat window because cards rendered by a pre-deploy server never expire; it can be deleted in a follow-up once that window passes.InteractionOpstate machine is channel-agnostic; render from the payload and encode structured ops as Telegram does.Validation
go test ./internal/userinput ./internal/channel/... ./internal/conversation/...— greengolangci-linton all touched packages — cleanmise run sqlc-generate), zero drift