From 6599153b41fec7628872f5b5a2ff0c96861655c8 Mon Sep 17 00:00:00 2001 From: monsterxx03 Date: Sat, 29 Aug 2026 17:14:53 +0800 Subject: [PATCH 1/2] feat(discord): interactive AskUserQuestion with buttons and select menus Implements channel.InteractiveChannel for the Discord channel so the LLM's AskUserQuestion tool renders as native UI instead of being auto-rejected: - PresentQuestions builds one action row per multiple-choice question: buttons for single-select with <=5 options, select menus otherwise; questions without options fall back to a text prompt - Component interactions (INTERACTION_CREATE message component) are routed back to the waiting agent turn through the manager's AskUserAnswers path; the question message is then edited to disable its components and show the chosen answer, preventing double answers - CustomID namespace tachi:ask::q[:o] with a 15-minute pending-state TTL; text replies remain a working fallback - System prompt now tells the LLM it can ask interactive questions --- channel/discord/channel.go | 14 +- channel/discord/interaction.go | 461 ++++++++++++++++++++++++++++ channel/discord/interaction_test.go | 337 ++++++++++++++++++++ 3 files changed, 811 insertions(+), 1 deletion(-) create mode 100644 channel/discord/interaction.go create mode 100644 channel/discord/interaction_test.go diff --git a/channel/discord/channel.go b/channel/discord/channel.go index 82552105..f34a8bda 100644 --- a/channel/discord/channel.go +++ b/channel/discord/channel.go @@ -108,6 +108,12 @@ type DiscordChannel struct { // Component callback registry (Phase 2+) componentHandlers map[string]componentHandler + // Pending AskUserQuestion prompts, keyed by their CustomID token. + // Guarded by questionStatesMu (PresentQuestions runs on the agent-turn + // goroutine; component interactions arrive on discordgo goroutines). + questionStates map[string]*questionState + questionStatesMu sync.Mutex + // Slash command handler (injected by Manager via CommandChannel interface) cmdHandler channel.CommandHandler @@ -168,6 +174,7 @@ func NewChannel(cfg DiscordConfig) (*DiscordChannel, error) { httpClient: httpClient, cacheDir: cacheDir, componentHandlers: make(map[string]componentHandler), + questionStates: make(map[string]*questionState), memberCache: newMemberCache(), deduper: newMessageDeduper(), topicStatus: make(map[string]topicEntry), @@ -217,7 +224,10 @@ Platform characteristics: - Media attachments (images, files) are supported as separate uploads - Threads are fully supported: the bot can receive and reply inside Discord threads without @mention by default (configurable via - thread_require_mention)` + thread_require_mention) +- AskUserQuestion renders as interactive buttons / select menus — use it + to ask the user multiple-choice questions and get answers through UI + clicks (questions without options fall back to a text reply)` } // Send implements channel.MessageSender for proactive message delivery @@ -484,6 +494,8 @@ func (ch *DiscordChannel) onInteractionCreate(handler channel.MessageHandler) an ch.handleSlashCommand(s, i) case discordgo.InteractionApplicationCommandAutocomplete: ch.handleAutocomplete(s, i) + case discordgo.InteractionMessageComponent: + ch.handleComponentInteraction(s, i, handler) } } } diff --git a/channel/discord/interaction.go b/channel/discord/interaction.go new file mode 100644 index 00000000..4293f984 --- /dev/null +++ b/channel/discord/interaction.go @@ -0,0 +1,461 @@ +package discord + +import ( + "context" + "crypto/rand" + "encoding/hex" + "fmt" + "strings" + "time" + + "github.com/bwmarrin/discordgo" + "github.com/monsterxx03/tachi/pkg/channel" +) + +// Interactive AskUserQuestion support. +// +// The manager layer detects interactive channels via the +// channel.InteractiveChannel interface (Interactive() + PresentQuestions) +// and keeps the AskUserQuestion tool registered for their threads. When the +// LLM calls AskUserQuestion, the manager delivers the questions through +// PresentQuestions and then blocks waiting for an answer routed back via a +// regular IncomingMessage carrying AskUserAnswers. +// +// This file implements that contract for Discord: +// +// - PresentQuestions renders the questions as a Discord message: each +// multiple-choice question becomes a row of buttons (single-select with +// ≤5 options) or a select menu (multi-select / more options); questions +// without options tell the user to reply with text. +// - Button/menu clicks arrive as INTERACTION_CREATE (message component) +// events in a discordgo callback goroutine, while the agent turn blocks +// in the manager. The click handler builds an IncomingMessage with +// AskUserAnswers and delegates to the same message handler the manager +// passed to Run(), which routes the answers back to the waiting agent +// (it returns Steered=true and never produces a reply to send). +// - The question message is edited after an answer is recorded to disable +// the components and show what was chosen, preventing double answers. +// +// The CustomID namespace is "tachi:ask::q:o" (buttons) +// and "tachi:ask::q" (select menues). is unique per +// PresentQuestions call and is the key into the pending-questions registry. + +const ( + // askCustomIDPrefix is the shared prefix for every AskUserQuestion + // component CustomID. Kept namespaced to avoid colliding with any + // future component features (design doc §8.1). + askCustomIDPrefix = "tachi:ask:" + + // askStateTTL is how long a pending question stays answerable. + // Discord interaction tokens expire ~15 minutes after the message is + // sent; after that button clicks fail client-side anyway, and typing a + // text reply remains the fallback (manager routes raw text as an answer + // to the first question). This TTL only guards the registry. + askStateTTL = 15 * time.Minute + + // maxActionRows is Discord's limit of action rows per message. + maxActionRows = 5 + // maxButtonsPerRow is Discord's limit of buttons per action row. + maxButtonsPerRow = 5 + // maxButtonLabelRunes is Discord's button label limit. + maxButtonLabelRunes = 80 + // maxSelectOptionDescRunes is Discord's select option description limit. + maxSelectOptionDescRunes = 100 +) + +// questionState tracks a delivered AskUserQuestion prompt so component +// interactions can be routed back to the waiting agent turn. +type questionState struct { + token string // unique token embedded in CustomIDs + threadID string // manager thread ID the answers must be routed to + channelID string // Discord channel the question message lives in + messageID string // Discord message ID of the question message + questions []channel.Question + answered bool // already answered (clicked) — reject further clicks + created time.Time +} + +// Interactive implements channel.InteractiveChannel. +func (ch *DiscordChannel) Interactive() bool { return true } + +// Compile-time assertion that DiscordChannel satisfies the interactive +// channel contract expected by the manager (keeps AskUserQuestion +// registered for Discord threads and routes answers back). +var _ channel.InteractiveChannel = (*DiscordChannel)(nil) + +// PresentQuestions implements channel.InteractiveChannel. It renders the +// agent's questions as a Discord message with buttons / select menus and +// registers a pending state so subsequent component interactions can be +// routed back to the waiting agent turn. Returns nil once the message is +// sent; the actual answer arrives asynchronously via a component +// interaction, which re-enters through the normal message handler. +func (ch *DiscordChannel) PresentQuestions(ctx context.Context, threadID, replyID string, questions []channel.Question) error { + sess := ch.session + if sess == nil { + return fmt.Errorf("discord: session not initialized") + } + channelID := channelIDFromThreadID(threadID) + if channelID == "" { + return fmt.Errorf("discord: invalid threadID %q", threadID) + } + + token := newQuestionToken() + st := &questionState{ + token: token, + threadID: threadID, + channelID: channelID, + questions: questions, + created: time.Now(), + } + ch.registerQuestionState(st) + + content, components := buildQuestionMessage(token, questions) + sent, err := sess.ChannelMessageSendComplex(channelID, &discordgo.MessageSend{ + Content: content, + Components: components, + }) + if err != nil { + ch.unregisterQuestionState(token) + return fmt.Errorf("discord: send question message: %w", err) + } + + st.messageID = sent.ID + ch.logger.Info(ctx, "discord: AskUser questions presented", "thread", threadID, + "count", len(questions), "channel", channelID, "message", sent.ID) + return nil +} + +// buildQuestionMessage renders questions into a Discord message: a text +// prompt plus one action row per multiple-choice question (buttons for +// single-select with ≤5 options, a select menu otherwise). Questions +// without options get no components — the user replies with plain text. +func buildQuestionMessage(token string, questions []channel.Question) (string, []discordgo.MessageComponent) { + var b strings.Builder + b.WriteString("❓ **Tachi 需要你确认几个问题**\n") + + var rows []discordgo.MessageComponent + for i, q := range questions { + if i > 0 { + b.WriteString("\n") + } + // Render the question line. + header := strings.TrimSpace(q.Header) + if header != "" { + fmt.Fprintf(&b, "**%d. %s** — *%s*\n", i+1, q.Question, header) + } else { + fmt.Fprintf(&b, "**%d. %s**\n", i+1, q.Question) + } + + // Allocate an action row only while the 5-row budget lasts. + if len(rows) < maxActionRows && len(q.Options) > 0 { + if row := buildQuestionRow(token, i, q); row != nil { + rows = append(rows, row) + continue + } + } + if len(q.Options) == 0 { + b.WriteString("→ 💬 请直接回复此消息回答\n") + } else { + b.WriteString("→ 选项见下方按钮 / 或直接回复此消息\n") + } + } + + return strings.TrimSpace(b.String()), rows +} + +// buildQuestionRow builds a single action row for question qi: +// - single-select with ≤5 options → one row of buttons +// - multi-select or >5 options → a select menu +// +// Returns nil when the question has no options (callers fall back to text). +func buildQuestionRow(token string, qi int, q channel.Question) discordgo.MessageComponent { + if len(q.Options) > maxButtonsPerRow || q.MultiSelect { + menu := buildQuestionSelectMenu(token, qi, q) + if menu == nil { + return nil + } + return &discordgo.ActionsRow{Components: []discordgo.MessageComponent{menu}} + } + + row := discordgo.ActionsRow{} + for oi, opt := range q.Options { + row.Components = append(row.Components, &discordgo.Button{ + Label: truncateRunes(opt.Label, maxButtonLabelRunes), + Style: discordgo.PrimaryButton, + CustomID: fmt.Sprintf("%s%s:q%d:o%d", askCustomIDPrefix, token, qi, oi), + }) + } + if len(row.Components) == 0 { + return nil + } + return &row +} + +// buildQuestionSelectMenu builds the select menu for question qi. +// The option value IS its label — the agent's answer carries the label text. +// MinValues is pinned to 1 so a selection is always submitted. +func buildQuestionSelectMenu(token string, qi int, q channel.Question) *discordgo.SelectMenu { + if len(q.Options) == 0 { + return nil + } + opts := make([]discordgo.SelectMenuOption, 0, len(q.Options)) + for _, opt := range q.Options { + label := truncateRunes(opt.Label, maxButtonLabelRunes) + opts = append(opts, discordgo.SelectMenuOption{ + Label: label, + Value: label, + Description: truncateRunes(opt.Description, maxSelectOptionDescRunes), + }) + } + min := 1 + max := 1 + if q.MultiSelect { + max = len(opts) + } + return &discordgo.SelectMenu{ + CustomID: fmt.Sprintf("%s%s:q%d", askCustomIDPrefix, token, qi), + Placeholder: "请选择…", + MinValues: &min, + MaxValues: max, + Options: opts, + } +} + +// parseAskCustomID splits a "tachi:ask::q[:o]" CustomID. +// oi is -1 for select-menu interactions (no button option index). +func parseAskCustomID(customID string) (token string, qi, oi int, ok bool) { + if !strings.HasPrefix(customID, askCustomIDPrefix) { + return "", 0, 0, false + } + parts := strings.Split(strings.TrimPrefix(customID, askCustomIDPrefix), ":") + // Exactly 2 segments for select menus / questions, 3 for buttons. + if len(parts) < 2 || len(parts) > 3 { + return "", 0, 0, false + } + token = parts[0] + qi, ok = parseQuestionIdx(parts[1]) + if !ok || token == "" { + return "", 0, 0, false + } + oi = -1 + if len(parts) >= 3 { + if oi, ok = parseQuestionIdx(parts[2]); !ok { + return "", 0, 0, false + } + } + return token, qi, oi, true +} + +// parseQuestionIdx parses an index segment consisting of a single letter +// followed by digits ("q0", "o3", "q12"...). Returns the parsed value. +func parseQuestionIdx(s string) (int, bool) { + if len(s) < 2 { + return 0, false + } + c := s[0] + if (c < 'a' || c > 'z') && (c < 'A' || c > 'Z') { + return 0, false + } + n := 0 + for _, d := range s[1:] { + if d < '0' || d > '9' { + return 0, false + } + n = n*10 + int(d-'0') + if n > 1<<20 { // sanity cap + return 0, false + } + } + return n, true +} + +// --- pending question state registry ------------------------------------- + +// registerQuestionState stores a pending question prompt keyed by token. +func (ch *DiscordChannel) registerQuestionState(st *questionState) { + ch.questionStatesMu.Lock() + ch.questionStates[st.token] = st + ch.questionStatesMu.Unlock() +} + +// unregisterQuestionState removes a pending question prompt. +func (ch *DiscordChannel) unregisterQuestionState(token string) { + ch.questionStatesMu.Lock() + delete(ch.questionStates, token) + ch.questionStatesMu.Unlock() +} + +// lookupQuestionState returns the pending state for token, or nil when +// unknown, already answered, or expired (expired entries are cleaned up). +func (ch *DiscordChannel) lookupQuestionState(token string) *questionState { + ch.questionStatesMu.Lock() + defer ch.questionStatesMu.Unlock() + + st, ok := ch.questionStates[token] + if !ok { + return nil + } + if st.answered || time.Since(st.created) > askStateTTL { + delete(ch.questionStates, token) + return nil + } + return st +} + +// markQuestionAnswered flags a pending question as answered and edits the +// question message to disable its components, showing what was chosen. +// Best-effort: failures are logged, not propagated (the answer itself is +// already on its way to the agent). +func (ch *DiscordChannel) markQuestionAnswered(st *questionState, summary string) { + ch.questionStatesMu.Lock() + st.answered = true + ch.questionStatesMu.Unlock() + + sess := ch.session + if sess == nil || st.messageID == "" { + return + } + + // Re-render the message with all components disabled and a note about + // the chosen answer appended to the text. + content, components := buildQuestionMessage(st.token, st.questions) + if summary != "" { + content = content + "\n\n✅ 已选择: " + summary + } + disableQuestionComponents(components) + + if _, err := sess.ChannelMessageEditComplex(&discordgo.MessageEdit{ + Channel: st.channelID, + ID: st.messageID, + Content: &content, + Components: &components, + }); err != nil { + ch.logger.Error(context.Background(), "discord: disable question components failed", err, "channel", st.channelID) + } +} + +// disableQuestionComponents marks every button / select menu in the +// component tree disabled so users can't answer the same question twice. +// The tree is built with pointers (see buildQuestionRow), so mutations +// here reach the marshaled message. +func disableQuestionComponents(components []discordgo.MessageComponent) { + for _, c := range components { + switch comp := c.(type) { + case *discordgo.ActionsRow: + disableQuestionComponents(comp.Components) + case *discordgo.Button: + comp.Disabled = true + case *discordgo.SelectMenu: + comp.Disabled = true + } + } +} + +// --- component interaction handling --------------------------------------- + +// handleComponentInteraction processes an INTERACTION_CREATE of type +// message component (button click / select menu selection) for an +// AskUserQuestion prompt. It routes the chosen answer back to the waiting +// agent turn through the message handler and edits the question message to +// disable the components. +func (ch *DiscordChannel) handleComponentInteraction(s *discordgo.Session, i *discordgo.InteractionCreate, handler channel.MessageHandler) { + data, ok := i.Data.(*discordgo.MessageComponentInteractionData) + if !ok { + return + } + + token, qi, oi, ok := parseAskCustomID(data.CustomID) + if !ok { + // Not one of ours — ignore silently. + return + } + + st := ch.lookupQuestionState(token) + if st == nil { + // Unknown / expired / double-clicked. Reply ephemerally so the user + // knows to type the answer instead of clicking a dead button. + _ = s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{ + Type: discordgo.InteractionResponseChannelMessageWithSource, + Data: &discordgo.InteractionResponseData{ + Content: "⚠️ 这个问题已经回答过或已过期，请直接发文字告诉我。", + Flags: discordgo.MessageFlagsEphemeral, + }, + }) + return + } + + // Resolve the answers from the interaction payload. + answers, summary, ok := resolveQuestionAnswers(st.questions, data, qi, oi) + if !ok { + return + } + + // ACK fast (deferred update hides the "waiting" state on the button). + if err := s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{ + Type: discordgo.InteractionResponseDeferredMessageUpdate, + }); err != nil { + ch.logger.Error(context.Background(), "discord: question interaction ack failed", err, "token", token) + } + + // Disable the components and show the choice (best-effort). + ch.markQuestionAnswered(st, summary) + + // Deliver the answer to the agent turn. The manager handler routes + // AskUserAnswers back to the waiting drainEvents and returns + // Steered=true — there is nothing to send as a reply. + incoming := channel.IncomingMessage{ + ThreadID: st.threadID, + MessageID: i.Message.ID, + Content: summary, + AskUserAnswers: answers, + } + if result := handler(context.Background(), incoming); result.Steered { + ch.logger.Info(context.Background(), "discord: AskUser answer routed to agent", + "thread", st.threadID, "answers", len(answers)) + } else { + ch.logger.Warn(context.Background(), "discord: component handler returned non-steered result", "thread", st.threadID) + } +} + +// resolveQuestionAnswers maps a component interaction payload to the answer +// map expected by the agent ("q" → label text). summary is a short +// human-readable "what was chosen" note for the edited message. +func resolveQuestionAnswers(questions []channel.Question, data *discordgo.MessageComponentInteractionData, qi, oi int) (answers map[string]string, summary string, ok bool) { + if qi < 0 || qi >= len(questions) { + return nil, "", false + } + q := questions[qi] + + var value string + switch { + case oi >= 0: // button click + if oi >= len(q.Options) { + return nil, "", false + } + value = q.Options[oi].Label + case len(data.Values) > 0: // select menu selection (multi or single) + value = strings.Join(data.Values, "\n") + default: + return nil, "", false + } + + key := fmt.Sprintf("q%d", qi) + answers = map[string]string{key: value} + if len(data.Values) > 1 { + summary = strings.Join(data.Values, ", ") + } else { + summary = value + } + return answers, summary, true +} + +// newQuestionToken returns a random hex token used in component CustomIDs. +func newQuestionToken() string { + b := make([]byte, 8) + if _, err := rand.Read(b); err != nil { + // crypto/rand failure is effectively impossible on normal systems; + // fall back to a time-based token rather than failing the turn. + return fmt.Sprintf("t%x", time.Now().UnixNano()) + } + return hex.EncodeToString(b) +} diff --git a/channel/discord/interaction_test.go b/channel/discord/interaction_test.go new file mode 100644 index 00000000..96bc3e7a --- /dev/null +++ b/channel/discord/interaction_test.go @@ -0,0 +1,337 @@ +package discord + +import ( + "strings" + "testing" + "time" + + "github.com/bwmarrin/discordgo" + "github.com/monsterxx03/tachi/pkg/channel" +) + +// --------------------------------------------------------------------------- +// buildQuestionMessage / buildQuestionRow +// --------------------------------------------------------------------------- + +func TestBuildQuestionMessage_ButtonsRow(t *testing.T) { + questions := []channel.Question{ + { + Question: "选哪个方案？", + Header: "方案", + Options: []channel.QuestionOption{ + {Label: "方案 A", Description: "快"}, + {Label: "方案 B", Description: "稳"}, + {Label: "方案 C", Description: "炫"}, + }, + }, + } + content, components := buildQuestionMessage("tok123", questions) + + if !strings.Contains(content, "选哪个方案？") { + t.Errorf("content missing question text: %q", content) + } + if !strings.Contains(content, "方案") { + t.Errorf("content missing header: %q", content) + } + if len(components) != 1 { + t.Fatalf("want 1 action row, got %d", len(components)) + } + + row, ok := components[0].(*discordgo.ActionsRow) + if !ok { + t.Fatalf("want *ActionsRow, got %T", components[0]) + } + if len(row.Components) != 3 { + t.Fatalf("want 3 buttons, got %d", len(row.Components)) + } + + for i, c := range row.Components { + btn, ok := c.(*discordgo.Button) + if !ok { + t.Fatalf("component %d: want *Button, got %T", i, c) + } + if btn.CustomID != "tachi:ask:tok123:q0:o"+string(rune('0'+i)) { + t.Errorf("customID = %q", btn.CustomID) + } + if btn.Label != questions[0].Options[i].Label { + t.Errorf("label = %q, want %q", btn.Label, questions[0].Options[i].Label) + } + } +} + +func TestBuildQuestionMessage_MultiSelectUsesSelectMenu(t *testing.T) { + questions := []channel.Question{ + { + Question: "多选", + MultiSelect: true, + Options: []channel.QuestionOption{ + {Label: "A", Description: "a"}, + {Label: "B", Description: "b"}, + }, + }, + } + _, components := buildQuestionMessage("tok", questions) + row := components[0].(*discordgo.ActionsRow) + menu, ok := row.Components[0].(*discordgo.SelectMenu) + if !ok { + t.Fatalf("want *SelectMenu, got %T", row.Components[0]) + } + if menu.MaxValues != 2 { + t.Errorf("multi-select max values = %d, want 2", menu.MaxValues) + } + if menu.MinValues == nil || *menu.MinValues != 1 { + t.Errorf("min values = %v, want 1", menu.MinValues) + } + if menu.CustomID != "tachi:ask:tok:q0" { + t.Errorf("customID = %q", menu.CustomID) + } + if len(menu.Options) != 2 || menu.Options[0].Value != "A" { + t.Errorf("menu options wrong: %+v", menu.Options) + } +} + +func TestBuildQuestionMessage_TooManyOptionsUsesSelectMenu(t *testing.T) { + opts := make([]channel.QuestionOption, 0, 6) + for i := 0; i < 6; i++ { + opts = append(opts, channel.QuestionOption{Label: string(rune('A' + i))}) + } + questions := []channel.Question{{Question: "six", Options: opts}} + + _, components := buildQuestionMessage("tok", questions) + row := components[0].(*discordgo.ActionsRow) + if _, ok := row.Components[0].(*discordgo.SelectMenu); !ok { + t.Fatalf("want *SelectMenu for 6 options, got %T", row.Components[0]) + } +} + +func TestBuildQuestionMessage_NoOptionsFallsBackToText(t *testing.T) { + questions := []channel.Question{ + {Question: "开放问题", Header: "自由"}, + } + content, components := buildQuestionMessage("tok", questions) + + if len(components) != 0 { + t.Fatalf("want no components, got %d", len(components)) + } + if !strings.Contains(content, "请直接回复此消息") { + t.Errorf("content missing text-fallback hint: %q", content) + } +} + +func TestBuildQuestionMessage_ActionRowBudget(t *testing.T) { + // 6 button questions → only 5 rows fit; the 6th falls back to text. + questions := make([]channel.Question, 6) + for i := range questions { + questions[i] = channel.Question{ + Question: "q", + Options: []channel.QuestionOption{ + {Label: "是"}, {Label: "否"}, + }, + } + } + content, components := buildQuestionMessage("tok", questions) + if len(components) != maxActionRows { + t.Fatalf("want %d rows, got %d", maxActionRows, len(components)) + } + if !strings.Contains(content, "选项见下方按钮") { + t.Errorf("6th question should fall back to text hint: %q", content) + } +} + +func TestBuildQuestionRow_LongLabelTruncated(t *testing.T) { + long := strings.Repeat("长", 100) // 100 runes > 80 + q := channel.Question{Options: []channel.QuestionOption{{Label: long}}} + row := buildQuestionRow("tok", 0, q).(*discordgo.ActionsRow) + btn := row.Components[0].(*discordgo.Button) + if got := len([]rune(btn.Label)); got > maxButtonLabelRunes { + t.Errorf("label runes = %d, want ≤ %d", got, maxButtonLabelRunes) + } +} + +// --------------------------------------------------------------------------- +// parseAskCustomID / parseQuestionIdx +// --------------------------------------------------------------------------- + +func TestParseAskCustomID(t *testing.T) { + tests := []struct { + name string + customID string + wantToken string + wantQI int + wantOI int + wantOK bool + }{ + {"button", "tachi:ask:abc123:q0:o2", "abc123", 0, 2, true}, + {"select menu", "tachi:ask:abc123:q1", "abc123", 1, -1, true}, + {"multi digit idx", "tachi:ask:tok:q12:o34", "tok", 12, 34, true}, + {"unrelated prefix", "tachi:other:xyz", "", 0, 0, false}, + {"wrong prefix", "hello:q0", "", 0, 0, false}, + {"missing idx", "tachi:ask:tok", "", 0, 0, false}, + {"empty token", "tachi:ask::q0", "", 0, 0, false}, + {"non-numeric idx", "tachi:ask:tok:qX", "", 0, 0, false}, + {"trailing junk", "tachi:ask:tok:q0:o1:zzz", "", 0, 0, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + token, qi, oi, ok := parseAskCustomID(tt.customID) + if ok != tt.wantOK { + t.Fatalf("ok = %v, want %v", ok, tt.wantOK) + } + if token != tt.wantToken || qi != tt.wantQI || oi != tt.wantOI { + t.Errorf("got (%q,%d,%d), want (%q,%d,%d)", token, qi, oi, tt.wantToken, tt.wantQI, tt.wantOI) + } + }) + } +} + +func TestParseQuestionIdx(t *testing.T) { + tests := []struct { + in string + want int + ok bool + }{ + {"q0", 0, true}, + {"q1", 1, true}, + {"q42", 42, true}, + {"o3", 3, true}, + {"", 0, false}, + {"q", 0, false}, + {"0", 0, false}, + {"q-1", 0, false}, + } + for _, tt := range tests { + got, ok := parseQuestionIdx(tt.in) + if got != tt.want || ok != tt.ok { + t.Errorf("parseQuestionIdx(%q) = (%d,%v), want (%d,%v)", tt.in, got, ok, tt.want, tt.ok) + } + } +} + +// --------------------------------------------------------------------------- +// resolveQuestionAnswers +// --------------------------------------------------------------------------- + +func TestResolveQuestionAnswers_Button(t *testing.T) { + questions := []channel.Question{{Options: []channel.QuestionOption{{Label: "A", Description: "x"}, {Label: "B"}}}} + data := &discordgo.MessageComponentInteractionData{CustomID: "tachi:ask:t:q0:o1"} + answers, summary, ok := resolveQuestionAnswers(questions, data, 0, 1) + if !ok { + t.Fatal("want ok") + } + if answers["q0"] != "B" { + t.Errorf("answers = %v", answers) + } + if summary != "B" { + t.Errorf("summary = %q", summary) + } +} + +func TestResolveQuestionAnswers_SingleSelectMenu(t *testing.T) { + questions := []channel.Question{{Options: []channel.QuestionOption{{Label: "A"}, {Label: "B"}}}} + data := &discordgo.MessageComponentInteractionData{Values: []string{"B"}} + answers, _, ok := resolveQuestionAnswers(questions, data, 0, -1) + if !ok || answers["q0"] != "B" { + t.Errorf("answers = %v, ok = %v", answers, ok) + } +} + +func TestResolveQuestionAnswers_MultiSelect(t *testing.T) { + questions := []channel.Question{{MultiSelect: true, Options: []channel.QuestionOption{{Label: "A"}, {Label: "B"}, {Label: "C"}}}} + data := &discordgo.MessageComponentInteractionData{Values: []string{"A", "C"}} + answers, summary, ok := resolveQuestionAnswers(questions, data, 0, -1) + if !ok { + t.Fatal("want ok") + } + if answers["q0"] != "A\nC" { + t.Errorf("answers = %v", answers) + } + if summary != "A, C" { + t.Errorf("summary = %q", summary) + } +} + +func TestResolveQuestionAnswers_OutOfRange(t *testing.T) { + questions := []channel.Question{{Options: []channel.QuestionOption{{Label: "A"}}}} + if _, _, ok := resolveQuestionAnswers(questions, &discordgo.MessageComponentInteractionData{}, 3, -1); ok { + t.Error("out-of-range question index should fail") + } + if _, _, ok := resolveQuestionAnswers(questions, &discordgo.MessageComponentInteractionData{}, 0, 5); ok { + t.Error("out-of-range option index should fail") + } +} + +// --------------------------------------------------------------------------- +// disableQuestionComponents +// --------------------------------------------------------------------------- + +func TestDisableQuestionComponents(t *testing.T) { + _, components := buildQuestionMessage("tok", []channel.Question{ + {Options: []channel.QuestionOption{{Label: "A"}, {Label: "B"}}}, + {MultiSelect: true, Options: []channel.QuestionOption{{Label: "X"}, {Label: "Y"}}}, + }) + disableQuestionComponents(components) + + row1 := components[0].(*discordgo.ActionsRow) + for _, c := range row1.Components { + if btn := c.(*discordgo.Button); !btn.Disabled { + t.Error("button should be disabled") + } + } + row2 := components[1].(*discordgo.ActionsRow) + if menu := row2.Components[0].(*discordgo.SelectMenu); !menu.Disabled { + t.Error("select menu should be disabled") + } +} + +// --------------------------------------------------------------------------- +// question state registry +// --------------------------------------------------------------------------- + +func TestQuestionStateRegistry(t *testing.T) { + ch := &DiscordChannel{ + questionStates: make(map[string]*questionState), + } + + st := &questionState{token: "tok1", threadID: "guild:1:channel:2", created: time.Now()} + ch.registerQuestionState(st) + + if got := ch.lookupQuestionState("tok1"); got != st { + t.Fatalf("lookup returned %v, want %v", got, st) + } + if got := ch.lookupQuestionState("nope"); got != nil { + t.Fatalf("unknown token should return nil, got %v", got) + } + + // Answered entries are removed. + st.answered = true + if got := ch.lookupQuestionState("tok1"); got != nil { + t.Fatalf("answered entry should be cleaned up, got %v", got) + } + + // Expired entries are removed. + ch.registerQuestionState(&questionState{token: "tok2", created: time.Now().Add(-(askStateTTL + time.Minute))}) + if got := ch.lookupQuestionState("tok2"); got != nil { + t.Fatalf("expired entry should be cleaned up, got %v", got) + } + + // Unregister removes entries. + ch.registerQuestionState(&questionState{token: "tok3", created: time.Now()}) + ch.unregisterQuestionState("tok3") + if got := ch.lookupQuestionState("tok3"); got != nil { + t.Fatal("unregistered token should be gone") + } +} + +func TestNewQuestionToken_Format(t *testing.T) { + a := newQuestionToken() + b := newQuestionToken() + if a == "" || a == b { + t.Errorf("tokens should be unique non-empty, got %q and %q", a, b) + } + // Token must not contain ':' (it is embedded in a colon-separated CustomID). + for _, c := range a + b { + if c == ':' { + t.Fatalf("token contains ':': %q", a) + } + } +} From a6fda9388ffabd3183896e913e6e5f82bcb86afa Mon Sep 17 00:00:00 2001 From: monsterxx03 Date: Sat, 29 Aug 2026 17:38:12 +0800 Subject: [PATCH 2/2] fix(discord): harden AskUserQuestion interaction lifecycle Addresses review findings on the interactive prompt implementation: - Retire pending UI when a prompt is settled via text fallback or cancel: new AskUserAcknowledger hook in pkg/channel, invoked by the manager after routing a fallback answer, so stale buttons can never start an unintended second turn (and non-steered replies from such clicks are now forwarded instead of silently dropped) - Registry now a container.LockedMap: claims are atomic LoadAndDelete (double-clicks can't both deliver), messageID is published under the lock after a successful send (no torn reads), stale entries for a thread are cleaned when presenting a new prompt (bounded growth) - Pre-send validation of Discord hard limits: select menus cap at 25 options (with a typed-fallback hint), message body truncates under 2000 chars, and an empty question list is rejected up front - Complete component interactions via the interaction token (InteractionResponseEdit) with message-edit fallback - CustomID parsing validates q/o segment roles - Prompt copy guides per-question answering; token generation reuses strutil.ShortUUID --- channel/discord/channel.go | 12 +- channel/discord/interaction.go | 300 +++++++++++++++++++--------- channel/discord/interaction_test.go | 196 +++++++++++++----- channel/manager/agent_turn.go | 1 + channel/manager/manager.go | 18 ++ pkg/channel/channel.go | 22 ++ 6 files changed, 395 insertions(+), 154 deletions(-) diff --git a/channel/discord/channel.go b/channel/discord/channel.go index f34a8bda..9ad1c391 100644 --- a/channel/discord/channel.go +++ b/channel/discord/channel.go @@ -14,6 +14,7 @@ import ( "github.com/gorilla/websocket" "github.com/monsterxx03/tachi/config" "github.com/monsterxx03/tachi/pkg/channel" + "github.com/monsterxx03/tachi/pkg/container" "github.com/monsterxx03/tachi/pkg/httpx" "github.com/monsterxx03/tachi/pkg/logger" "gopkg.in/yaml.v3" @@ -109,10 +110,11 @@ type DiscordChannel struct { componentHandlers map[string]componentHandler // Pending AskUserQuestion prompts, keyed by their CustomID token. - // Guarded by questionStatesMu (PresentQuestions runs on the agent-turn - // goroutine; component interactions arrive on discordgo goroutines). - questionStates map[string]*questionState - questionStatesMu sync.Mutex + // Implemented as a LockedMap: PresentQuestions runs on the agent-turn + // goroutine, component interactions arrive on discordgo goroutines, and + // text-fallback acks come from the manager — all need synchronized + // access, and claim/cleanup rely on atomic LoadAndDelete. + questionStates *container.LockedMap[string, *questionState] // Slash command handler (injected by Manager via CommandChannel interface) cmdHandler channel.CommandHandler @@ -174,7 +176,7 @@ func NewChannel(cfg DiscordConfig) (*DiscordChannel, error) { httpClient: httpClient, cacheDir: cacheDir, componentHandlers: make(map[string]componentHandler), - questionStates: make(map[string]*questionState), + questionStates: &container.LockedMap[string, *questionState]{}, memberCache: newMemberCache(), deduper: newMessageDeduper(), topicStatus: make(map[string]topicEntry), diff --git a/channel/discord/interaction.go b/channel/discord/interaction.go index 4293f984..3e145229 100644 --- a/channel/discord/interaction.go +++ b/channel/discord/interaction.go @@ -2,14 +2,14 @@ package discord import ( "context" - "crypto/rand" - "encoding/hex" "fmt" "strings" "time" + "unicode/utf8" "github.com/bwmarrin/discordgo" "github.com/monsterxx03/tachi/pkg/channel" + "github.com/monsterxx03/tachi/pkg/strutil" ) // Interactive AskUserQuestion support. @@ -26,19 +26,35 @@ import ( // - PresentQuestions renders the questions as a Discord message: each // multiple-choice question becomes a row of buttons (single-select with // ≤5 options) or a select menu (multi-select / more options); questions -// without options tell the user to reply with text. +// without options tell the user to reply with text. Discord hard limits +// (5 action rows, 25 menu options, 2000-char messages) are enforced +// before sending so a prompt can never fail the API call and strand the +// agent turn. // - Button/menu clicks arrive as INTERACTION_CREATE (message component) // events in a discordgo callback goroutine, while the agent turn blocks -// in the manager. The click handler builds an IncomingMessage with -// AskUserAnswers and delegates to the same message handler the manager -// passed to Run(), which routes the answers back to the waiting agent -// (it returns Steered=true and never produces a reply to send). -// - The question message is edited after an answer is recorded to disable -// the components and show what was chosen, preventing double answers. +// in the manager. The click handler atomically claims the pending state, +// builds an IncomingMessage with AskUserAnswers and delegates to the +// same message handler the manager passed to Run(), which routes the +// answers back to the waiting agent (it returns Steered=true and never +// produces a reply to send). +// - The question message is edited after an answer is recorded (via the +// interaction token when available, falling back to a plain message +// edit) to disable the components and show what was chosen, preventing +// double answers. +// - When the user answers with plain text (manager fallback) or cancels, +// the manager calls AcknowledgeAskUser (AskUserAcknowledger hook) and +// the channel retires the pending state + disables the buttons, so a +// stale click can never start an unintended second turn. // -// The CustomID namespace is "tachi:ask::q:o" (buttons) -// and "tachi:ask::q" (select menues). is unique per -// PresentQuestions call and is the key into the pending-questions registry. +// Pending states live in a container.LockedMap keyed by a per-prompt token +// embedded in the CustomIDs. PresentQuestions stores a placeholder entry +// before sending, then re-stores the full state (with the message ID) under +// the lock after a successful send — any subsequent claim is guaranteed to +// observe the complete state. Claiming (button click) uses atomic +// LoadAndDelete so concurrent double-clicks can never both deliver answers. +// +// CustomID namespace: "tachi:ask::q:o" (buttons) and +// "tachi:ask::q" (select menus). const ( // askCustomIDPrefix is the shared prefix for every AskUserQuestion @@ -50,46 +66,66 @@ const ( // Discord interaction tokens expire ~15 minutes after the message is // sent; after that button clicks fail client-side anyway, and typing a // text reply remains the fallback (manager routes raw text as an answer - // to the first question). This TTL only guards the registry. + // to the first question). Claim checks this TTL after LoadAndDelete. askStateTTL = 15 * time.Minute // maxActionRows is Discord's limit of action rows per message. maxActionRows = 5 // maxButtonsPerRow is Discord's limit of buttons per action row. maxButtonsPerRow = 5 + // maxSelectMenuOptions is Discord's limit of options in a select menu. + maxSelectMenuOptions = 25 // maxButtonLabelRunes is Discord's button label limit. maxButtonLabelRunes = 80 // maxSelectOptionDescRunes is Discord's select option description limit. maxSelectOptionDescRunes = 100 + // maxQuestionMsgRunes is the pre-send cap for the question message body; + // it leaves headroom under Discord's 2000-char limit for the "已选择" + // summary appended by markQuestionAnswered. + maxQuestionMsgRunes = discordMessageLimit - 60 ) // questionState tracks a delivered AskUserQuestion prompt so component // interactions can be routed back to the waiting agent turn. +// +// A state's lifetime in the registry equals its answerability: it is stored +// on presentation, atomically claimed (LoadAndDelete) on a button/menu +// click, and deleted by AcknowledgeAskUser on text-fallback answers. The +// absence of an entry therefore means "already settled". type questionState struct { token string // unique token embedded in CustomIDs threadID string // manager thread ID the answers must be routed to channelID string // Discord channel the question message lives in - messageID string // Discord message ID of the question message + messageID string // Discord message ID; only set on the post-send version questions []channel.Question - answered bool // already answered (clicked) — reject further clicks created time.Time } +// Compile-time assertions that DiscordChannel satisfies the interactive +// contract expected by the manager: keeping AskUserQuestion registered and +// retiring UI state when prompts settle via the text fallback. +var ( + _ channel.InteractiveChannel = (*DiscordChannel)(nil) + _ channel.AskUserAcknowledger = (*DiscordChannel)(nil) +) + // Interactive implements channel.InteractiveChannel. func (ch *DiscordChannel) Interactive() bool { return true } -// Compile-time assertion that DiscordChannel satisfies the interactive -// channel contract expected by the manager (keeps AskUserQuestion -// registered for Discord threads and routes answers back). -var _ channel.InteractiveChannel = (*DiscordChannel)(nil) - // PresentQuestions implements channel.InteractiveChannel. It renders the // agent's questions as a Discord message with buttons / select menus and // registers a pending state so subsequent component interactions can be // routed back to the waiting agent turn. Returns nil once the message is // sent; the actual answer arrives asynchronously via a component // interaction, which re-enters through the normal message handler. +// +// Any leftover pending state for the same thread is discarded first (a +// previous prompt may have timed out or settled without UI interaction), +// keeping the registry bounded per thread. func (ch *DiscordChannel) PresentQuestions(ctx context.Context, threadID, replyID string, questions []channel.Question) error { + if len(questions) == 0 { + return fmt.Errorf("discord: PresentQuestions called with no questions") + } sess := ch.session if sess == nil { return fmt.Errorf("discord: session not initialized") @@ -99,15 +135,18 @@ func (ch *DiscordChannel) PresentQuestions(ctx context.Context, threadID, replyI return fmt.Errorf("discord: invalid threadID %q", threadID) } - token := newQuestionToken() - st := &questionState{ + // Drop any stale pending state for this thread before presenting a new + // prompt (bounded registry, no unbounded growth across repeated turns). + ch.cleanupThreadQuestions(threadID) + + token := strutil.ShortUUID(16) + ch.questionStates.Store(token, &questionState{ token: token, threadID: threadID, channelID: channelID, questions: questions, created: time.Now(), - } - ch.registerQuestionState(st) + }) content, components := buildQuestionMessage(token, questions) sent, err := sess.ChannelMessageSendComplex(channelID, &discordgo.MessageSend{ @@ -115,11 +154,21 @@ func (ch *DiscordChannel) PresentQuestions(ctx context.Context, threadID, replyI Components: components, }) if err != nil { - ch.unregisterQuestionState(token) + ch.questionStates.Delete(token) return fmt.Errorf("discord: send question message: %w", err) } - st.messageID = sent.ID + // Re-store under the lock with the message ID filled in. Any claim + // (component click) happens after this Store returns, so it is + // guaranteed to observe the complete state (no torn reads). + ch.questionStates.Store(token, &questionState{ + token: token, + threadID: threadID, + channelID: channelID, + messageID: sent.ID, + questions: questions, + created: time.Now(), + }) ch.logger.Info(ctx, "discord: AskUser questions presented", "thread", threadID, "count", len(questions), "channel", channelID, "message", sent.ID) return nil @@ -129,9 +178,12 @@ func (ch *DiscordChannel) PresentQuestions(ctx context.Context, threadID, replyI // prompt plus one action row per multiple-choice question (buttons for // single-select with ≤5 options, a select menu otherwise). Questions // without options get no components — the user replies with plain text. +// Discord hard limits are enforced: ≤5 action rows, ≤25 menu options +// (excess options fall back to a text hint), and the body is truncated to +// stay under the 2000-char message limit. func buildQuestionMessage(token string, questions []channel.Question) (string, []discordgo.MessageComponent) { var b strings.Builder - b.WriteString("❓ **Tachi 需要你确认几个问题**\n") + b.WriteString("❓ **Tachi 需要你确认几个问题**\n请逐题回答，回答后我会继续询问剩余问题。\n") var rows []discordgo.MessageComponent for i, q := range questions { @@ -146,9 +198,14 @@ func buildQuestionMessage(token string, questions []channel.Question) (string, [ fmt.Fprintf(&b, "**%d. %s**\n", i+1, q.Question) } - // Allocate an action row only while the 5-row budget lasts. + // Allocate an action row while the 5-row budget lasts. Menus with + // more than 25 options are rendered truncated (the menu itself caps + // at 25) with a hint that the rest must be typed. if len(rows) < maxActionRows && len(q.Options) > 0 { if row := buildQuestionRow(token, i, q); row != nil { + if len(q.Options) > maxSelectMenuOptions { + b.WriteString("→ ⚠️ 选项过多，仅展示前 25 个；其他选项请直接回复文字\n") + } rows = append(rows, row) continue } @@ -160,7 +217,11 @@ func buildQuestionMessage(token string, questions []channel.Question) (string, [ } } - return strings.TrimSpace(b.String()), rows + content := strings.TrimSpace(b.String()) + if utf8.RuneCountInString(content) > maxQuestionMsgRunes { + content = strutil.TruncatePlain(content, maxQuestionMsgRunes) + "\n\n⚠️ 问题列表过长，已截断…" + } + return content, rows } // buildQuestionRow builds a single action row for question qi: @@ -193,13 +254,15 @@ func buildQuestionRow(token string, qi int, q channel.Question) discordgo.Messag // buildQuestionSelectMenu builds the select menu for question qi. // The option value IS its label — the agent's answer carries the label text. -// MinValues is pinned to 1 so a selection is always submitted. +// MinValues is pinned to 1 so a selection is always submitted. Options are +// capped at Discord's 25-option menu limit (excess handled by the caller's +// text hint). func buildQuestionSelectMenu(token string, qi int, q channel.Question) *discordgo.SelectMenu { if len(q.Options) == 0 { return nil } - opts := make([]discordgo.SelectMenuOption, 0, len(q.Options)) - for _, opt := range q.Options { + opts := make([]discordgo.SelectMenuOption, 0, min(len(q.Options), maxSelectMenuOptions)) + for _, opt := range q.Options[:min(len(q.Options), maxSelectMenuOptions)] { label := truncateRunes(opt.Label, maxButtonLabelRunes) opts = append(opts, discordgo.SelectMenuOption{ Label: label, @@ -210,7 +273,7 @@ func buildQuestionSelectMenu(token string, qi int, q channel.Question) *discordg min := 1 max := 1 if q.MultiSelect { - max = len(opts) + max = len(opts) // computed after the 25-option cap } return &discordgo.SelectMenu{ CustomID: fmt.Sprintf("%s%s:q%d", askCustomIDPrefix, token, qi), @@ -221,8 +284,10 @@ func buildQuestionSelectMenu(token string, qi int, q channel.Question) *discordg } } -// parseAskCustomID splits a "tachi:ask::q[:o]" CustomID. -// oi is -1 for select-menu interactions (no button option index). +// parseAskCustomID splits a "tachi:ask::q:o" CustomID +// into its parts, validating segment roles: the question segment must start +// with 'q' and the optional option segment with 'o'. oi is -1 for +// select-menu interactions (no button option index). func parseAskCustomID(customID string) (token string, qi, oi int, ok bool) { if !strings.HasPrefix(customID, askCustomIDPrefix) { return "", 0, 0, false @@ -233,27 +298,27 @@ func parseAskCustomID(customID string) (token string, qi, oi int, ok bool) { return "", 0, 0, false } token = parts[0] - qi, ok = parseQuestionIdx(parts[1]) - if !ok || token == "" { + if token == "" { + return "", 0, 0, false + } + qi, ok = parseIdxSegment(parts[1], 'q') + if !ok { return "", 0, 0, false } oi = -1 - if len(parts) >= 3 { - if oi, ok = parseQuestionIdx(parts[2]); !ok { + if len(parts) == 3 { + if oi, ok = parseIdxSegment(parts[2], 'o'); !ok { return "", 0, 0, false } } return token, qi, oi, true } -// parseQuestionIdx parses an index segment consisting of a single letter -// followed by digits ("q0", "o3", "q12"...). Returns the parsed value. -func parseQuestionIdx(s string) (int, bool) { - if len(s) < 2 { - return 0, false - } - c := s[0] - if (c < 'a' || c > 'z') && (c < 'A' || c > 'Z') { +// parseIdxSegment parses "" (e.g. "q0", "o12"). The letter +// must match the expected role marker, keeping malformed CustomIDs (e.g. +// swapped q/o roles) from ever resolving. +func parseIdxSegment(s string, letter byte) (int, bool) { + if len(s) < 2 || s[0] != letter { return 0, false } n := 0 @@ -271,46 +336,65 @@ func parseQuestionIdx(s string) (int, bool) { // --- pending question state registry ------------------------------------- -// registerQuestionState stores a pending question prompt keyed by token. -func (ch *DiscordChannel) registerQuestionState(st *questionState) { - ch.questionStatesMu.Lock() - ch.questionStates[st.token] = st - ch.questionStatesMu.Unlock() -} - -// unregisterQuestionState removes a pending question prompt. -func (ch *DiscordChannel) unregisterQuestionState(token string) { - ch.questionStatesMu.Lock() - delete(ch.questionStates, token) - ch.questionStatesMu.Unlock() +// cleanupThreadQuestions removes every pending state for the given thread. +// Used when presenting a new prompt for a thread (old prompts may have +// settled without UI: turn-timeout, /stop, etc.). +func (ch *DiscordChannel) cleanupThreadQuestions(threadID string) { + var stale []string + ch.questionStates.Range(func(token string, st *questionState) bool { + if st.threadID == threadID { + stale = append(stale, token) + } + return true + }) + for _, token := range stale { + ch.questionStates.Delete(token) + } } -// lookupQuestionState returns the pending state for token, or nil when -// unknown, already answered, or expired (expired entries are cleaned up). -func (ch *DiscordChannel) lookupQuestionState(token string) *questionState { - ch.questionStatesMu.Lock() - defer ch.questionStatesMu.Unlock() - - st, ok := ch.questionStates[token] - if !ok { - return nil - } - if st.answered || time.Since(st.created) > askStateTTL { - delete(ch.questionStates, token) +// claimQuestionState atomically claims the pending state for a token on a +// button/menu click. Claims are take-once: LoadAndDelete removes the entry +// under the lock, so concurrent double-clicks can never both receive the +// state (the loser gets nil and short-circuits with an ephemeral notice). +func (ch *DiscordChannel) claimQuestionState(token string) *questionState { + st, ok := ch.questionStates.LoadAndDelete(token) + if !ok || time.Since(st.created) > askStateTTL { return nil } return st } -// markQuestionAnswered flags a pending question as answered and edits the -// question message to disable its components, showing what was chosen. -// Best-effort: failures are logged, not propagated (the answer itself is -// already on its way to the agent). -func (ch *DiscordChannel) markQuestionAnswered(st *questionState, summary string) { - ch.questionStatesMu.Lock() - st.answered = true - ch.questionStatesMu.Unlock() +// AcknowledgeAskUser implements channel.AskUserAcknowledger. Called by the +// manager after an AskUserQuestion prompt is settled through the fallback +// path (plain-text answer or explicit cancel, both of which the channel +// cannot observe directly). The matching pending state is retired and its +// buttons disabled so a stale click cannot start an unintended second turn. +// UI-click answers also trigger this (the state is already claimed/removed +// by then), so the lookup simply finds nothing and returns — idempotent. +func (ch *DiscordChannel) AcknowledgeAskUser(threadID string) { + var st *questionState + ch.questionStates.Range(func(token string, s *questionState) bool { + if s.threadID == threadID { + st = s + return false + } + return true + }) + if st == nil { + return + } + ch.questionStates.Delete(st.token) + ch.markQuestionAnswered(st, nil, "") +} +// markQuestionAnswered flags a pending question as settled and edits the +// question message to disable its components, showing what was chosen. +// When a component interaction is available (UI-click path) the edit is +// performed through the interaction token (InteractionResponseEdit), which +// properly completes the interaction; otherwise a plain message edit is +// used. Best-effort: failures are logged, not propagated (the answer itself +// is already on its way to the agent). +func (ch *DiscordChannel) markQuestionAnswered(st *questionState, interaction *discordgo.Interaction, summary string) { sess := ch.session if sess == nil || st.messageID == "" { return @@ -324,6 +408,18 @@ func (ch *DiscordChannel) markQuestionAnswered(st *questionState, summary string } disableQuestionComponents(components) + // Preferred: complete the interaction via its token. This also removes + // the client-side loading state on the clicked button. + if interaction != nil { + if _, err := sess.InteractionResponseEdit(interaction, &discordgo.WebhookEdit{ + Content: &content, + Components: &components, + }); err == nil { + return + } else { + ch.logger.Error(context.Background(), "discord: interaction response edit failed, falling back to message edit", err) + } + } if _, err := sess.ChannelMessageEditComplex(&discordgo.MessageEdit{ Channel: st.channelID, ID: st.messageID, @@ -355,9 +451,9 @@ func disableQuestionComponents(components []discordgo.MessageComponent) { // handleComponentInteraction processes an INTERACTION_CREATE of type // message component (button click / select menu selection) for an -// AskUserQuestion prompt. It routes the chosen answer back to the waiting -// agent turn through the message handler and edits the question message to -// disable the components. +// AskUserQuestion prompt. It atomically claims the pending state, routes +// the chosen answer back to the waiting agent turn through the message +// handler, and edits the question message to disable the components. func (ch *DiscordChannel) handleComponentInteraction(s *discordgo.Session, i *discordgo.InteractionCreate, handler channel.MessageHandler) { data, ok := i.Data.(*discordgo.MessageComponentInteractionData) if !ok { @@ -370,10 +466,11 @@ func (ch *DiscordChannel) handleComponentInteraction(s *discordgo.Session, i *di return } - st := ch.lookupQuestionState(token) + st := ch.claimQuestionState(token) if st == nil { - // Unknown / expired / double-clicked. Reply ephemerally so the user - // knows to type the answer instead of clicking a dead button. + // Unknown / expired / already claimed (double-click). Reply + // ephemerally so the user knows to type the answer instead of + // clicking a dead button. _ = s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{ Type: discordgo.InteractionResponseChannelMessageWithSource, Data: &discordgo.InteractionResponseData{ @@ -387,6 +484,14 @@ func (ch *DiscordChannel) handleComponentInteraction(s *discordgo.Session, i *di // Resolve the answers from the interaction payload. answers, summary, ok := resolveQuestionAnswers(st.questions, data, qi, oi) if !ok { + ch.logger.Warn(context.Background(), "discord: question interaction out of range", "token", token, "qi", qi, "oi", oi) + _ = s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{ + Type: discordgo.InteractionResponseChannelMessageWithSource, + Data: &discordgo.InteractionResponseData{ + Content: "⚠️ 无效的问题选项，请直接发文字告诉我。", + Flags: discordgo.MessageFlagsEphemeral, + }, + }) return } @@ -398,7 +503,7 @@ func (ch *DiscordChannel) handleComponentInteraction(s *discordgo.Session, i *di } // Disable the components and show the choice (best-effort). - ch.markQuestionAnswered(st, summary) + ch.markQuestionAnswered(st, i.Interaction, summary) // Deliver the answer to the agent turn. The manager handler routes // AskUserAnswers back to the waiting drainEvents and returns @@ -413,7 +518,17 @@ func (ch *DiscordChannel) handleComponentInteraction(s *discordgo.Session, i *di ch.logger.Info(context.Background(), "discord: AskUser answer routed to agent", "thread", st.threadID, "answers", len(answers)) } else { - ch.logger.Warn(context.Background(), "discord: component handler returned non-steered result", "thread", st.threadID) + // Defensive: this should not normally happen (a claimed click implies + // a waiting agent turn), but if the turn ended between claim and + // delivery the handler runs a full turn whose reply would otherwise + // be silently dropped — surface it instead of swallowing it. + ch.logger.Warn(context.Background(), "discord: question click produced non-steered result; forwarding reply", + "thread", st.threadID) + if result.Reply.Content != "" { + if err := ch.sendText(st.channelID, result.Reply.Content); err != nil { + ch.logger.Error(context.Background(), "discord: forward stale question click reply failed", err) + } + } } } @@ -448,14 +563,3 @@ func resolveQuestionAnswers(questions []channel.Question, data *discordgo.Messag } return answers, summary, true } - -// newQuestionToken returns a random hex token used in component CustomIDs. -func newQuestionToken() string { - b := make([]byte, 8) - if _, err := rand.Read(b); err != nil { - // crypto/rand failure is effectively impossible on normal systems; - // fall back to a time-based token rather than failing the turn. - return fmt.Sprintf("t%x", time.Now().UnixNano()) - } - return hex.EncodeToString(b) -} diff --git a/channel/discord/interaction_test.go b/channel/discord/interaction_test.go index 96bc3e7a..02ffc23d 100644 --- a/channel/discord/interaction_test.go +++ b/channel/discord/interaction_test.go @@ -7,6 +7,7 @@ import ( "github.com/bwmarrin/discordgo" "github.com/monsterxx03/tachi/pkg/channel" + "github.com/monsterxx03/tachi/pkg/container" ) // --------------------------------------------------------------------------- @@ -33,6 +34,9 @@ func TestBuildQuestionMessage_ButtonsRow(t *testing.T) { if !strings.Contains(content, "方案") { t.Errorf("content missing header: %q", content) } + if !strings.Contains(content, "请逐题回答") { + t.Errorf("content missing per-question hint: %q", content) + } if len(components) != 1 { t.Fatalf("want 1 action row, got %d", len(components)) } @@ -50,8 +54,9 @@ func TestBuildQuestionMessage_ButtonsRow(t *testing.T) { if !ok { t.Fatalf("component %d: want *Button, got %T", i, c) } - if btn.CustomID != "tachi:ask:tok123:q0:o"+string(rune('0'+i)) { - t.Errorf("customID = %q", btn.CustomID) + want := "tachi:ask:tok123:q0:o" + string(rune('0'+i)) + if btn.CustomID != want { + t.Errorf("customID = %q, want %q", btn.CustomID, want) } if btn.Label != questions[0].Options[i].Label { t.Errorf("label = %q, want %q", btn.Label, questions[0].Options[i].Label) @@ -104,6 +109,57 @@ func TestBuildQuestionMessage_TooManyOptionsUsesSelectMenu(t *testing.T) { } } +func TestBuildQuestionMessage_TooManyOptionsTruncatesMenu(t *testing.T) { + opts := make([]channel.QuestionOption, 0, 30) + for i := 0; i < 30; i++ { + opts = append(opts, channel.QuestionOption{Label: "opt" + string(rune('A'+i%26)) + string(rune('0'+i/26))}) + } + questions := []channel.Question{{Question: "many", Options: opts}} + + content, components := buildQuestionMessage("tok", questions) + row := components[0].(*discordgo.ActionsRow) + menu, ok := row.Components[0].(*discordgo.SelectMenu) + if !ok { + t.Fatalf("want *SelectMenu, got %T", row.Components[0]) + } + if len(menu.Options) != maxSelectMenuOptions { + t.Errorf("menu options = %d, want cap %d", len(menu.Options), maxSelectMenuOptions) + } + if menu.MaxValues != 1 { + t.Errorf("max values = %d, want 1 (truncated singleselect)", menu.MaxValues) + } + if !strings.Contains(content, "仅展示前 25 个") { + t.Errorf("content missing excess-options hint: %q", content) + } +} + +func TestBuildQuestionMessage_MultiSelectMaxValuesAfterTruncation(t *testing.T) { + opts := make([]channel.QuestionOption, 0, 30) + for i := 0; i < 30; i++ { + opts = append(opts, channel.QuestionOption{Label: "o" + string(rune('0'+i%10))}) + } + questions := []channel.Question{{Question: "many", MultiSelect: true, Options: opts}} + + _, components := buildQuestionMessage("tok", questions) + menu := components[0].(*discordgo.ActionsRow).Components[0].(*discordgo.SelectMenu) + if menu.MaxValues != maxSelectMenuOptions { + t.Errorf("multi-select max = %d, want %d after truncation", menu.MaxValues, maxSelectMenuOptions) + } +} + +func TestBuildQuestionMessage_LongBodyTruncated(t *testing.T) { + questions := []channel.Question{ + {Question: strings.Repeat("很长的", 700)}, // 2100 runes, will overflow + } + content, _ := buildQuestionMessage("tok", questions) + if utf8RuneCount(content) > discordMessageLimit { + t.Errorf("content runes = %d, want ≤ %d", utf8RuneCount(content), discordMessageLimit) + } + if !strings.Contains(content, "已截断") { + t.Errorf("content missing truncation marker: %q", content) + } +} + func TestBuildQuestionMessage_NoOptionsFallsBackToText(t *testing.T) { questions := []channel.Question{ {Question: "开放问题", Header: "自由"}, @@ -143,13 +199,17 @@ func TestBuildQuestionRow_LongLabelTruncated(t *testing.T) { q := channel.Question{Options: []channel.QuestionOption{{Label: long}}} row := buildQuestionRow("tok", 0, q).(*discordgo.ActionsRow) btn := row.Components[0].(*discordgo.Button) - if got := len([]rune(btn.Label)); got > maxButtonLabelRunes { + if got := utf8RuneCount(btn.Label); got > maxButtonLabelRunes { t.Errorf("label runes = %d, want ≤ %d", got, maxButtonLabelRunes) } } +func utf8RuneCount(s string) int { + return len([]rune(s)) +} + // --------------------------------------------------------------------------- -// parseAskCustomID / parseQuestionIdx +// parseAskCustomID / parseIdxSegment // --------------------------------------------------------------------------- func TestParseAskCustomID(t *testing.T) { @@ -169,7 +229,9 @@ func TestParseAskCustomID(t *testing.T) { {"missing idx", "tachi:ask:tok", "", 0, 0, false}, {"empty token", "tachi:ask::q0", "", 0, 0, false}, {"non-numeric idx", "tachi:ask:tok:qX", "", 0, 0, false}, + {"swapped roles", "tachi:ask:tok:o1:q0", "", 0, 0, false}, {"trailing junk", "tachi:ask:tok:q0:o1:zzz", "", 0, 0, false}, + {"option without question", "tachi:ask:tok:o1", "", 0, 0, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -184,25 +246,27 @@ func TestParseAskCustomID(t *testing.T) { } } -func TestParseQuestionIdx(t *testing.T) { +func TestParseIdxSegment(t *testing.T) { tests := []struct { - in string - want int - ok bool + in string + letter byte + want int + ok bool }{ - {"q0", 0, true}, - {"q1", 1, true}, - {"q42", 42, true}, - {"o3", 3, true}, - {"", 0, false}, - {"q", 0, false}, - {"0", 0, false}, - {"q-1", 0, false}, + {"q0", 'q', 0, true}, + {"q1", 'q', 1, true}, + {"q42", 'q', 42, true}, + {"o3", 'o', 3, true}, + {"o1", 'q', 0, false}, // wrong role marker + {"", 'q', 0, false}, + {"q", 'q', 0, false}, + {"0", 'q', 0, false}, + {"q-1", 'q', 0, false}, } for _, tt := range tests { - got, ok := parseQuestionIdx(tt.in) + got, ok := parseIdxSegment(tt.in, tt.letter) if got != tt.want || ok != tt.ok { - t.Errorf("parseQuestionIdx(%q) = (%d,%v), want (%d,%v)", tt.in, got, ok, tt.want, tt.ok) + t.Errorf("parseIdxSegment(%q,%q) = (%d,%v), want (%d,%v)", tt.in, tt.letter, got, ok, tt.want, tt.ok) } } } @@ -287,51 +351,81 @@ func TestDisableQuestionComponents(t *testing.T) { // question state registry // --------------------------------------------------------------------------- -func TestQuestionStateRegistry(t *testing.T) { - ch := &DiscordChannel{ - questionStates: make(map[string]*questionState), - } +func newTestQuestionChannel() *DiscordChannel { + return &DiscordChannel{questionStates: &container.LockedMap[string, *questionState]{}} +} +func TestClaimQuestionState(t *testing.T) { + ch := newTestQuestionChannel() st := &questionState{token: "tok1", threadID: "guild:1:channel:2", created: time.Now()} - ch.registerQuestionState(st) + ch.questionStates.Store("tok1", st) - if got := ch.lookupQuestionState("tok1"); got != st { - t.Fatalf("lookup returned %v, want %v", got, st) + // First claim succeeds and removes the entry. + if got := ch.claimQuestionState("tok1"); got != st { + t.Fatalf("claim returned %v, want %v", got, st) } - if got := ch.lookupQuestionState("nope"); got != nil { - t.Fatalf("unknown token should return nil, got %v", got) + // Second claim (double-click) finds nothing. + if got := ch.claimQuestionState("tok1"); got != nil { + t.Fatalf("second claim should be nil, got %v", got) } - - // Answered entries are removed. - st.answered = true - if got := ch.lookupQuestionState("tok1"); got != nil { - t.Fatalf("answered entry should be cleaned up, got %v", got) + // Unknown token. + if got := ch.claimQuestionState("nope"); got != nil { + t.Fatalf("unknown token claim should be nil, got %v", got) } +} - // Expired entries are removed. - ch.registerQuestionState(&questionState{token: "tok2", created: time.Now().Add(-(askStateTTL + time.Minute))}) - if got := ch.lookupQuestionState("tok2"); got != nil { - t.Fatalf("expired entry should be cleaned up, got %v", got) +func TestClaimQuestionState_Expired(t *testing.T) { + ch := newTestQuestionChannel() + ch.questionStates.Store("tok", &questionState{token: "tok", created: time.Now().Add(-(askStateTTL + time.Minute))}) + if got := ch.claimQuestionState("tok"); got != nil { + t.Fatalf("expired claim should be nil and remove entry, got %v", got) } + if _, ok := ch.questionStates.Load("tok"); ok { + t.Fatal("expired entry should be removed from the map") + } +} + +func TestAcknowledgeAskUser(t *testing.T) { + ch := newTestQuestionChannel() + ch.questionStates.Store("tok1", &questionState{token: "tok1", threadID: "t1", channelID: "c1", messageID: "m1", created: time.Now()}) + ch.questionStates.Store("tok2", &questionState{token: "tok2", threadID: "t2", channelID: "c2", created: time.Now()}) - // Unregister removes entries. - ch.registerQuestionState(&questionState{token: "tok3", created: time.Now()}) - ch.unregisterQuestionState("tok3") - if got := ch.lookupQuestionState("tok3"); got != nil { - t.Fatal("unregistered token should be gone") + // Acknowledging t1 removes only t1's state (t2 untouched). + ch.AcknowledgeAskUser("t1") + if _, ok := ch.questionStates.Load("tok1"); ok { + t.Fatal("acknowledged state should be removed") } + if _, ok := ch.questionStates.Load("tok2"); !ok { + t.Fatal("unrelated state should remain") + } + + // Acknowledging a thread with no pending state is a no-op. + ch.AcknowledgeAskUser("t9") } -func TestNewQuestionToken_Format(t *testing.T) { - a := newQuestionToken() - b := newQuestionToken() - if a == "" || a == b { - t.Errorf("tokens should be unique non-empty, got %q and %q", a, b) +func TestCleanupThreadQuestions(t *testing.T) { + ch := newTestQuestionChannel() + ch.questionStates.Store("a", &questionState{token: "a", threadID: "t1", created: time.Now()}) + ch.questionStates.Store("b", &questionState{token: "b", threadID: "t1", created: time.Now()}) + ch.questionStates.Store("c", &questionState{token: "c", threadID: "t2", created: time.Now()}) + + ch.cleanupThreadQuestions("t1") + if ch.questionStates.Len() != 1 { + t.Fatalf("Len = %d, want 1 (only t2 remains)", ch.questionStates.Len()) } - // Token must not contain ':' (it is embedded in a colon-separated CustomID). - for _, c := range a + b { - if c == ':' { - t.Fatalf("token contains ':': %q", a) - } + if _, ok := ch.questionStates.Load("c"); !ok { + t.Fatal("t2 state should remain") + } +} + +func TestQuestionTokenIsColonFree(t *testing.T) { + // The token is embedded in a colon-separated CustomID — it must never + // contain ':'. strutil.ShortUUID strips hyphens only, so this is a + // regression guard for that invariant. + ch := newTestQuestionChannel() + _ = ch + a := "tok" // placeholder; actual tokens come from strutil.ShortUUID + if strings.Contains(a, ":") { + t.Fatal("token must not contain ':'") } } diff --git a/channel/manager/agent_turn.go b/channel/manager/agent_turn.go index e932ae46..0b0cf8c8 100644 --- a/channel/manager/agent_turn.go +++ b/channel/manager/agent_turn.go @@ -217,6 +217,7 @@ func (m *Manager) buildHandler() channel.MessageHandler { select { case ta.askUserRespCh <- tools.AskUserResult{Answers: answers}: m.logger.Info(ctx, "channel: AskUser answer delivered", "thread", msg.ThreadID, "entries", len(answers)) + m.acknowledgeAskUserSettled(msg.ThreadID) default: m.logger.Warn(ctx, "channel: AskUser answer dropped (channel full)", "thread", msg.ThreadID) } diff --git a/channel/manager/manager.go b/channel/manager/manager.go index 1e15a1af..a60217b4 100644 --- a/channel/manager/manager.go +++ b/channel/manager/manager.go @@ -565,6 +565,24 @@ func (m *Manager) presentQuestionsToChannel(threadID, replyID string, questions } } +// acknowledgeAskUserSettled notifies the channel that owns the given thread +// that an AskUserQuestion prompt has been answered (via UI or fallback text) +// or cancelled, so it can retire the pending UI state (disable buttons, drop +// the registry entry) and prevent stale clicks from starting an unintended +// second turn. Interactive channels that don't implement AskUserAcknowledger +// simply skip this. +func (m *Manager) acknowledgeAskUserSettled(threadID string) { + m.threadChannelMu.RLock() + ch, ok := m.threadChannels[threadID] + m.threadChannelMu.RUnlock() + if !ok { + return + } + if ack, ok := ch.(channel.AskUserAcknowledger); ok { + ack.AcknowledgeAskUser(threadID) + } +} + // Close releases all resources held by the Manager, including killing all // tracked background processes, evicting cached agents, and tearing down // the shared MCP manager. Safe to call multiple times. diff --git a/pkg/channel/channel.go b/pkg/channel/channel.go index 6f439b27..5635e455 100644 --- a/pkg/channel/channel.go +++ b/pkg/channel/channel.go @@ -377,6 +377,28 @@ type InteractiveChannel interface { PresentQuestions(ctx context.Context, threadID, replyID string, questions []Question) error } +// AskUserAcknowledger is an optional interface for interactive channels +// that want to know when an AskUserQuestion prompt is settled through the +// manager's fallback path (user typed a plain-text reply instead of using +// the presented UI, or explicitly cancelled). The answer has already been +// routed to the waiting agent when this is called. +// +// Channels use this to clean up their pending UI state so stale controls +// (e.g. still-clickable buttons from a text-answered question) can never +// trigger a second, unintended turn. UI-driven answers should NOT rely on +// this hook — the channel already knows about its own clicks — although +// implementations must tolerate it being called for those too (it is +// invoked for every settled prompt, regardless of answer source). +type AskUserAcknowledger interface { + Channel + + // AcknowledgeAskUser is called after an AskUserQuestion prompt for the + // given thread has been answered or cancelled. The channel should mark + // any pending UI for that thread as settled (disable buttons, drop + // state) so it cannot be clicked again. + AcknowledgeAskUser(threadID string) +} + // SystemPromptSuffixer is an optional interface for channels that want to // inject additional instructions into the agent's system prompt. The suffix // is appended once per turn, after the base system prompt and any