From 0a4b4617264d582ac58e33aee534fc15641605e5 Mon Sep 17 00:00:00 2001 From: LiuWei Date: Mon, 19 Oct 2026 16:22:58 +0800 Subject: [PATCH] fix(core): drop AskUserQuestion option taps that no longer match a question MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Platforms forward an AskUserQuestion option tap to the engine as a plain "askq::" message. When that tap arrives after the question was already answered (for example by a chat message), there is no pending question, so the tap fell through to the normal message path: on a busy session it was queued ("Message received — will process after the current task finishes") and later sent to the agent as the literal text "askq:0:2". A tap while a tool permission was pending got the allow/deny hint, and a tap on an earlier question's card in a multi-question prompt answered the current question with the wrong option. The engine now recognises option taps and, when they do not match the question being asked, tells the user the question is no longer active instead of passing the tap on. Free text that merely starts with "askq:" is unaffected. --- core/askq_stale_callback_test.go | 208 +++++++++++++++++++++++++++++++ core/engine.go | 52 ++++++++ core/i18n.go | 8 ++ 3 files changed, 268 insertions(+) create mode 100644 core/askq_stale_callback_test.go diff --git a/core/askq_stale_callback_test.go b/core/askq_stale_callback_test.go new file mode 100644 index 000000000..388ded934 --- /dev/null +++ b/core/askq_stale_callback_test.go @@ -0,0 +1,208 @@ +package core + +import ( + "context" + "fmt" + "sync" + "testing" + "time" +) + +// askqJourneySession is a busy agent session the test drives event by event, +// recording both prompts and AskUserQuestion answers sent to the agent. +type askqJourneySession struct { + *receiptAgentSession + answerMu sync.Mutex + answers []PermissionResult +} + +func (s *askqJourneySession) RespondPermission(_ string, result PermissionResult) error { + s.answerMu.Lock() + defer s.answerMu.Unlock() + s.answers = append(s.answers, result) + return nil +} + +func (s *askqJourneySession) answerCount() int { + s.answerMu.Lock() + defer s.answerMu.Unlock() + return len(s.answers) +} + +func (s *askqJourneySession) prompts() []string { + s.sendMu.Lock() + defer s.sendMu.Unlock() + return append([]string(nil), s.sendCalls...) +} + +type askqJourneyAgent struct { + stubAgent + session *askqJourneySession +} + +func (a *askqJourneyAgent) StartSession(context.Context, string) (AgentSession, error) { + return a.session, nil +} + +// CUJ: the agent asks a question, the user answers it in chat, then taps the +// question card's option button while the turn is still running. The late +// tap must not reach the agent as a prompt. Before the fix it was queued as +// literal "askq:0:2" text and sent to the agent once the turn ended. +func TestCUJ_C7_StaleAskQuestionClickNotSentToAgent(t *testing.T) { + p := &stubPlatformEngine{n: "test"} + session := &askqJourneySession{ + receiptAgentSession: &receiptAgentSession{queuingAgentSession: newQueuingSession("askq-session")}, + } + e := NewEngine("askq", &askqJourneyAgent{session: session}, []Platform{p}, t.TempDir()+"/sessions.json", LangEnglish) + t.Cleanup(func() { _ = e.Stop() }) + env := &cujEnv{t: t, engine: e, plat: p} + send := func(content string) { + e.ReceiveMessage(p, &Message{ + SessionKey: "test:asker", Platform: "test", UserID: "asker", UserName: "asker", + MessageID: "m-" + content, Content: content, ReplyCtx: "ctx", + }) + } + + // User action 1: start a task; the agent asks a question mid-turn. + send("do the task") + env.waitFor("task sent to agent", 3*time.Second, func() bool { return len(session.prompts()) == 1 }) + session.events <- Event{Type: EventPermissionRequest, RequestID: "req-q", ToolName: "AskUserQuestion", + ToolInputRaw: map[string]any{}, Questions: testQuestions()} + env.waitFor("question shown", 3*time.Second, func() bool { return env.sentContains("Which database?") }) + + // User action 2: answer in chat. The question is resolved, the turn goes on. + send("PostgreSQL please") + env.waitFor("answer delivered", 3*time.Second, func() bool { return session.answerCount() == 1 }) + + // User action 3: tap the card's option button after the question is gone. + p.clearSent() + send("askq:0:2") + env.waitFor("stale notice", 3*time.Second, func() bool { + return env.sentContains(e.i18n.T(MsgAskQuestionStale)) + }) + if env.sentContains(e.i18n.T(MsgMessageQueued)) { + t.Fatalf("stale option tap was queued for the agent; sent: %v", p.getSent()) + } + + // User action 4: the turn ends and the user sends the next task. A queued + // tap would be drained first and sent to the agent before this one. + session.events <- Event{Type: EventResult, Content: "task done", Done: true} + env.waitFor("turn result", 3*time.Second, func() bool { return env.sentContains("task done") }) + send("next task") + env.waitFor("next task sent", 3*time.Second, func() bool { return len(session.prompts()) >= 2 }) + if got, want := fmt.Sprint(session.prompts()), fmt.Sprint([]string{"do the task", "next task"}); got != want { + t.Fatalf("prompts sent to agent = %s, want %s", got, want) + } + if n := session.answerCount(); n != 1 { + t.Fatalf("AskUserQuestion answers sent = %d, want 1", n) + } +} + +func TestParseAskQuestionCallback(t *testing.T) { + cases := []struct { + in string + qIdx int + hasQIdx bool + ok bool + }{ + {"askq:0:2", 0, true, true}, + {"askq:3:1", 3, true, true}, + {" askq:1:1 ", 1, true, true}, + {"askq:2", 0, false, true}, // legacy askq:optIdx + {"askq:", 0, false, false}, + {"askq: use the second one", 0, false, false}, + {"askq:0:x", 0, false, false}, + {"askq:-1:2", 0, false, false}, + {"askq:1:2:3", 0, false, false}, + {"2", 0, false, false}, + {"PostgreSQL", 0, false, false}, + } + for _, c := range cases { + qIdx, hasQIdx, ok := parseAskQuestionCallback(c.in) + if qIdx != c.qIdx || hasQIdx != c.hasQIdx || ok != c.ok { + t.Errorf("parseAskQuestionCallback(%q) = (%d, %v, %v), want (%d, %v, %v)", + c.in, qIdx, hasQIdx, ok, c.qIdx, c.hasQIdx, c.ok) + } + } +} + +func TestHandlePendingPermission_StaleAskQuestionCallback(t *testing.T) { + stale := func(e *Engine, p *stubPlatformEngine) bool { + return fmt.Sprint(p.getSent()) == fmt.Sprint([]string{e.i18n.T(MsgAskQuestionStale)}) + } + + t.Run("no interactive state (e.g. after restart)", func(t *testing.T) { + e := newTestEngine() + p := &stubPlatformEngine{n: "test"} + msg := &Message{SessionKey: "sk", Content: "askq:0:1", ReplyCtx: "ctx"} + if !e.handlePendingPermission(p, msg, "askq:0:1", "ghost") { + t.Fatal("stale option tap fell through to the normal message path") + } + if !stale(e, p) { + t.Fatalf("sent = %v, want only the stale-question notice", p.getSent()) + } + }) + + t.Run("tool permission pending, not a question", func(t *testing.T) { + e := newTestEngine() + p := &stubPlatformEngine{n: "test"} + rec := &recordingAgentSession{} + pending := &pendingPermission{RequestID: "req-bash", ToolName: "Bash", Resolved: make(chan struct{})} + state := &interactiveState{agentSession: rec, platform: p, replyCtx: "ctx", pending: pending} + e.interactiveMu.Lock() + e.interactiveStates["ws:sk"] = state + e.interactiveMu.Unlock() + + msg := &Message{SessionKey: "sk", Content: "askq:0:1", ReplyCtx: "ctx"} + if !e.handlePendingPermission(p, msg, "askq:0:1", "ws:sk") { + t.Fatal("expected the tap to be handled") + } + if rec.calls != 0 { + t.Fatalf("RespondPermission calls = %d, want 0", rec.calls) + } + if state.pending != pending { + t.Fatal("the tool permission request must stay pending") + } + if !stale(e, p) { + t.Fatalf("sent = %v, want only the stale-question notice", p.getSent()) + } + }) + + t.Run("tap on an earlier question's card", func(t *testing.T) { + e := newTestEngine() + p := &stubPlatformEngine{n: "test"} + rec := &recordingAgentSession{} + pending := &pendingPermission{ + RequestID: "req-q", ToolName: "AskUserQuestion", ToolInput: map[string]any{}, + Questions: testMultiQuestions(), Answers: map[int]string{0: "PostgreSQL"}, + CurrentQuestion: 1, Resolved: make(chan struct{}), + } + state := &interactiveState{agentSession: rec, platform: p, replyCtx: "ctx", pending: pending} + e.interactiveMu.Lock() + e.interactiveStates["ws:sk"] = state + e.interactiveMu.Unlock() + + msg := &Message{SessionKey: "sk", ReplyCtx: "ctx"} + if !e.handlePendingPermission(p, msg, "askq:0:2", "ws:sk") { + t.Fatal("expected the tap to be handled") + } + if rec.calls != 0 || pending.Answers[1] != "" { + t.Fatalf("tap on question 0's card answered question 1 (calls=%d, answers=%v)", rec.calls, pending.Answers) + } + if !stale(e, p) { + t.Fatalf("sent = %v, want only the stale-question notice", p.getSent()) + } + + // The current question's card still answers it. + if !e.handlePendingPermission(p, msg, "askq:1:2", "ws:sk") { + t.Fatal("expected the current card's tap to be handled") + } + if rec.calls != 1 { + t.Fatalf("RespondPermission calls = %d, want 1", rec.calls) + } + answers, _ := rec.lastResult.UpdatedInput["answers"].(map[string]any) + if answers["Which framework?"] != "Echo" || answers["Which database?"] != "PostgreSQL" { + t.Fatalf("answers = %v", answers) + } + }) +} diff --git a/core/engine.go b/core/engine.go index ed3cc0386..817c95c0a 100644 --- a/core/engine.go +++ b/core/engine.go @@ -3612,10 +3612,28 @@ func (e *Engine) handlePendingPermission(p Platform, msg *Message, content strin "session", msg.SessionKey, "content", content) return true } + // Same for an AskUserQuestion option click that arrives after its + // question was answered some other way (e.g. by a chat message), or + // after a restart: forwarding "askq:0:1" as user input would queue + // it for the agent as literal text. + if _, _, ok := parseAskQuestionCallback(content); ok { + e.dropStaleAskQuestionCallback(p, msg, content) + return true + } return false } found: + // An option click from an AskUserQuestion card is never a permission + // decision, and a click on an earlier question's card must not answer + // the question being asked now. + if qIdx, hasQIdx, ok := parseAskQuestionCallback(content); ok { + if len(pending.Questions) == 0 || (hasQIdx && qIdx != pending.CurrentQuestion) { + e.dropStaleAskQuestionCallback(p, msg, content) + return true + } + } + // AskUserQuestion: interpret user response as an answer, not a permission decision if len(pending.Questions) > 0 { // Reject empty or whitespace-only content: some platforms echo delivery @@ -3726,6 +3744,40 @@ func (e *Engine) lookupPending(iKey string) (*interactiveState, *pendingPermissi return state, pending } +// parseAskQuestionCallback recognizes the value an AskUserQuestion option +// button sends back: "askq:qIdx:optIdx", or the legacy "askq:optIdx". +// hasQIdx reports whether the question index was present. ok is false for +// anything else, including free text that merely starts with "askq:". +func parseAskQuestionCallback(content string) (qIdx int, hasQIdx bool, ok bool) { + rest, found := strings.CutPrefix(strings.TrimSpace(content), "askq:") + if !found { + return 0, false, false + } + parts := strings.Split(rest, ":") + for _, part := range parts { + if n, err := strconv.Atoi(part); err != nil || n < 0 { + return 0, false, false + } + } + switch len(parts) { + case 1: + return 0, false, true + case 2: + qIdx, _ = strconv.Atoi(parts[0]) + return qIdx, true, true + } + return 0, false, false +} + +// dropStaleAskQuestionCallback tells the user that an option click arrived +// for a question that is no longer waiting for an answer, instead of passing +// the click on to the agent. +func (e *Engine) dropStaleAskQuestionCallback(p Platform, msg *Message, content string) { + slog.Info("dropping stale ask-question callback", + "session", msg.SessionKey, "content", content) + e.reply(p, msg.ReplyCtx, e.i18n.T(MsgAskQuestionStale)) +} + // resolveAskQuestionAnswer converts user input into answer text. // It handles button callbacks ("askq:qIdx:optIdx"), numeric selections ("1", "1,3"), and free text. func (e *Engine) resolveAskQuestionAnswer(q UserQuestion, input string) string { diff --git a/core/i18n.go b/core/i18n.go index 4b6f2ff72..24a6222de 100644 --- a/core/i18n.go +++ b/core/i18n.go @@ -457,6 +457,7 @@ const ( MsgAskQuestionMulti MsgKey = "ask_question_multi" MsgAskQuestionPrompt MsgKey = "ask_question_prompt" MsgAskQuestionAnswered MsgKey = "ask_question_answered" + MsgAskQuestionStale MsgKey = "ask_question_stale" MsgCommandsTitle MsgKey = "commands_title" MsgCommandsEmpty MsgKey = "commands_empty" @@ -2816,6 +2817,13 @@ var messages = map[MsgKey]map[Language]string{ LangJapanese: "回答済み", LangSpanish: "Respondido", }, + MsgAskQuestionStale: { + LangEnglish: "⚠️ That question was already answered or is no longer active, so this answer was not sent to the agent.", + LangChinese: "⚠️ 这个问题已经回答过了或已失效，这次的回答没有发给 agent。", + LangTraditionalChinese: "⚠️ 這個問題已經回答過了或已失效，這次的回答沒有發給 agent。", + LangJapanese: "⚠️ この質問は回答済みか無効になっているため、この回答はエージェントに送信されませんでした。", + LangSpanish: "⚠️ Esa pregunta ya se respondió o ya no está activa, así que esta respuesta no se envió al agente.", + }, MsgCommandsTitle: { LangEnglish: "🔧 **Custom Commands** (%d)\n\n", LangChinese: "🔧 **自定义命令** (%d)\n\n",