From 020c05fe65f8588080a9da2785ac26d4fe04067f Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 13:59:54 +0200 Subject: [PATCH] messenger, telegram: show the operator at once what a tap did, and change the question to its outcome (hq issue 372) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At 13:30 on 2026-10-10 the operator approved two questions on Telegram and saw nothing happen: the channel acknowledged each tap with no words, before the router had judged it, and the edit that removed the buttons kept the question's title, adding only a line under it that rings nothing. The router now answers a chosen option in reply to the tap, once the ask's end is recorded and before the question is edited: "You chose Approve: recorded, and claude-code was told." (or, when the bus did not take the warrant, that it is told when it does). A refusal already replied with its reason. Telegram holds a tap's acknowledgement for the router's word and shows it as the tap's own toast; a word with a body (a refusal's reason, Details) also stays in the chat. Without a word in 8 seconds the toast claims nothing, and the late word goes to the chat. A secret is never a toast. An ask's own message, once answered, carries its outcome as its title: "You chose Approve at 13:30 on Telegram · ", then whether the asker was told. The asks protocol carries no result back from the asker, so nothing more is claimed of what it did. --- modules/messenger/README.md | 6 + .../messenger/cmd/messenger/answers_test.go | 33 +++- modules/messenger/cmd/messenger/asks.go | 70 +++++++- .../messenger/cmd/messenger/feedback_test.go | 152 +++++++++++++++++ modules/messenger/cmd/messenger/holder.go | 21 ++- modules/messenger/cmd/messenger/selfact.go | 2 +- modules/telegram/README.md | 7 + modules/telegram/cmd/telegram/channel.go | 79 ++++++++- modules/telegram/cmd/telegram/channel_test.go | 156 +++++++++++++++++- 9 files changed, 498 insertions(+), 28 deletions(-) create mode 100644 modules/messenger/cmd/messenger/feedback_test.go diff --git a/modules/messenger/README.md b/modules/messenger/README.md index 6ddf21d8..9907a96d 100644 --- a/modules/messenger/README.md +++ b/modules/messenger/README.md @@ -35,6 +35,12 @@ channel's client and no channel's secret. the kind proves its sender; the kind's capabilities meet the level; an approval comes from an account linked an hour or more. The ask is then closed by compare-and-set — a second answer loses — and the warrant published to the asker alone. A warrant the bus did not take is published again every minute. +- **What the operator sees when they answer** (issue 372): first, in reply to the answer where it was given, + that it was recorded and whether its asker was told ("You chose Approve: recorded, and claude-code was + told.") — said only once the ask's end is recorded — or, refused, why; then the ask's message edited to how + it ended, as its title ("You chose Approve at 13:30 on Telegram · "), with no answers left on it. + What the asker then did is not said: the asks protocol carries no result back to the router, so the + message says only that the asker was told and acts on it itself. - **An ask never defaults**: at its expiry it ends `expired` and its asker does what it said it would. - **Linking** (`/start` to the bot): a six-digit code is shown, with the account's name and number, on a channel whose words stay private (the desk) for ten minutes; the operator sends it to the bot, and the diff --git a/modules/messenger/cmd/messenger/answers_test.go b/modules/messenger/cmd/messenger/answers_test.go index 2a5eed39..4f667a10 100644 --- a/modules/messenger/cmd/messenger/answers_test.go +++ b/modules/messenger/cmd/messenger/answers_test.go @@ -365,10 +365,24 @@ func TestAnApproveFromTheLinkedAccountIsAWarrantForTheAskerAlone(t *testing.T) { for _, m := range ch.edits { edited = m } - if len(edited.Actions) != 0 || !strings.Contains(edited.Body, "Release: chosen at") { - t.Errorf("%s still offers %q: %q", ch.name, labelsOf(edited), edited.Body) + // The question's title is how it ended, read at a glance; then what the router knows of the asker (issue 372). + if len(edited.Actions) != 0 || edited.Title != "You chose Release at 12:00 on Telegram · Delivery of hq held for 36 hours" || + !strings.HasPrefix(edited.Body, "Told to mesh-delivery, which acts on it itself.\n") { + t.Errorf("%s still offers %q, or does not say how it ended: %q / %q", ch.name, labelsOf(edited), edited.Title, edited.Body) } } + // The tap is confirmed where it was made, in reply to it, once the warrant stood — and before the question + // was edited, so the confirmation is what the operator sees first. + var confirmed []Message + for _, m := range tg.sends { + if m.Reply == choiceOf(release, "42", c.now()).ID { + confirmed = append(confirmed, m) + } + } + if len(confirmed) != 1 || confirmed[0].Title != "You chose Release: recorded, and mesh-delivery was told." || + confirmed[0].Body != "" || !confirmed[0].Quiet { + t.Errorf("the tap was confirmed with %+v", confirmed) + } // One warrant: the same answer again, and another answer after it, are refused and said. if out, _ := h.Chosen("telegram", choiceOf(release, "42", c.now())); out != "chosen" { t.Errorf("a redelivery of the same answer was acted on again: %s", out) @@ -603,8 +617,9 @@ func TestAnAskExpiresAndNeverDefaults(t *testing.T) { for _, m := range tg.edits { edited = m } - if len(edited.Actions) != 0 || !strings.Contains(edited.Body, "No longer answerable") { - t.Errorf("the copy still offers answers: %+v", edited) + if len(edited.Actions) != 0 || !strings.HasPrefix(edited.Title, "No longer answerable: nobody answered in time · Delivery of hq") || + !strings.HasPrefix(edited.Body, "The delivery stays held.") { + t.Errorf("the copy still offers answers, or does not say how it ended: %+v", edited) } } @@ -794,9 +809,13 @@ func TestUnlinkingIsAskedOfTheLinkedAccountAndToldToItFirst(t *testing.T) { if len(store.ids) != 0 { t.Fatal("the account's own approval did not unlink it") } - notice := tg.sends[before] - if notice.Title != "This account is no longer linked as you" || tg.to[before] != "42" { - t.Errorf("the account was not told first, by name: %q to %q", notice.Title, tg.to[before]) + // The tap is confirmed where it was made (issue 372), then the account is told, by name, before it goes. + if confirmed := tg.sends[before]; confirmed.Title != "You chose Unlink: recorded." || confirmed.Reply == "" { + t.Errorf("the tap was not confirmed in reply to it: %+v", confirmed) + } + notice := tg.sends[before+1] + if notice.Title != "This account is no longer linked as you" || tg.to[before+1] != "42" { + t.Errorf("the account was not told first, by name: %q to %q", notice.Title, tg.to[before+1]) } if last := dt.sends[len(dt.sends)-1]; !strings.Contains(last.Title, "No Telegram account is linked as you") { t.Errorf("the desk was not told: %q", last.Title) diff --git a/modules/messenger/cmd/messenger/asks.go b/modules/messenger/cmd/messenger/asks.go index 25c51d6a..37c396fa 100644 --- a/modules/messenger/cmd/messenger/asks.go +++ b/modules/messenger/cmd/messenger/asks.go @@ -415,7 +415,7 @@ func (h *Holder) TakeAsk(verb, asker string, body json.RawMessage) error { h.Logf("[messenger] a cancel from %s names no ask", asker) return nil } - return h.closeAsk(askKey(asker, c.ID), asks.OutcomeCancelled, "its asker took it back", nil, "") + return h.closeAsk(askKey(asker, c.ID), asks.OutcomeCancelled, "its asker took it back", nil, "", nil) } var a asks.Ask if err := json.Unmarshal(body, &a); err != nil { @@ -469,7 +469,7 @@ func (h *Holder) TakeAsk(verb, asker string, body json.RawMessage) error { return nil } if replaced != "" { - if err := h.closeAsk(replaced, asks.OutcomeReplaced, "a newer ask about the same thing replaced it", nil, ""); err != nil && !errors.Is(err, ErrChanged) { + if err := h.closeAsk(replaced, asks.OutcomeReplaced, "a newer ask about the same thing replaced it", nil, "", nil); err != nil && !errors.Is(err, ErrChanged) { return err } } @@ -596,8 +596,10 @@ func (h *Holder) tell(w asks.Warrant) error { } // closeAsk ends an open ask by compare-and-set, tells its asker, and edits every copy. A second close -// loses: ErrChanged when another outcome was first. -func (h *Holder) closeAsk(key string, outcome asks.Outcome, words string, w *asks.Warrant, used string) error { +// loses: ErrChanged when another outcome was first. confirm, when given, is called once the end is recorded +// and the asker told or not (told), before any copy is edited: the answer's confirmation where it was given +// comes first, and never before the record stands (issue 372). +func (h *Holder) closeAsk(key string, outcome asks.Outcome, words string, w *asks.Warrant, used string, confirm func(told bool)) error { e := h.askOf(key) if e == nil { return nil @@ -643,13 +645,20 @@ func (h *Holder) closeAsk(key string, outcome asks.Outcome, words string, w *ask if rec.Asker == SelfAsker { // The router's own ask: it does what was decided, and tells nobody else (selfact.go). h.markTold(key) + if confirm != nil { + confirm(true) + } h.showClosed(rec) h.selfAct(rec, *w) return nil } - if h.tell(*w) == nil { + told := h.tell(*w) == nil + if told { h.markTold(key) } + if confirm != nil { + confirm(told) + } h.showClosed(rec) return nil } @@ -680,12 +689,14 @@ func (h *Holder) showClosed(rec AskRecord) { if r == nil { return } + told := h.toldLine(rec) // read before the lock: it reads the ask's record h.mu.Lock() + // The first line is how it ended, which an ask's own message shows as its title (sayOn); the rest follows. switch { case rec.Warrant != nil && rec.Warrant.Outcome == asks.OutcomeChosen: - r.Answered = rec.Warrant.Label + ": chosen at " + rec.Closed.In(h.zone()).Format("15:04") + " on " + Display(rec.Warrant.By.Kind) + r.Answered = chosenLine(rec.Warrant, rec.Closed.In(h.zone())) + "\n" + told case rec.State == string(asks.OutcomeExpired): - r.Answered = "No longer answerable: nobody answered in time. " + capital(rec.Ask.OnExpiry) + r.Answered = "No longer answerable: nobody answered in time.\n" + strings.TrimSuffix(capital(rec.Ask.OnExpiry), ".") + "." case rec.State == string(asks.OutcomeReplaced): r.Answered = "" default: @@ -704,6 +715,46 @@ func (h *Holder) showClosed(rec AskRecord) { h.persist(r) } +// chosenLine is how a chosen ask ended, in the operator's words: "You chose Approve at 13:30 on Telegram." +func chosenLine(w *asks.Warrant, at time.Time) string { + who, where := "Somebody", "" + if w.By != nil { + if w.By.Who == asks.Operator { + who = "You" + } + where = " on " + Display(w.By.Kind) + } + return who + " chose " + w.Label + " at " + at.Format("15:04") + where + "." +} + +// toldLine is what the router knows of the asker after an answer: whether it was told. The asker says nothing +// back of what it then did (the asks protocol carries no result to the router), so nothing more is claimed. +func (h *Holder) toldLine(rec AskRecord) string { + if rec.Asker == SelfAsker { + return "The router does it itself." + } + if e := h.askOf(askKey(rec.Asker, rec.Ask.ID)); e != nil && e.rec.Told { + return "Told to " + rec.Asker + ", which acts on it itself." + } + return "Told to " + rec.Asker + " as soon as the bus takes it; it acts on it itself." +} + +// confirmChoice says, where an answer was given and in reply to it, that it was recorded: on Telegram the +// tap's own acknowledgement (issue 372). Called only once the ask's end is recorded. +func (h *Holder) confirmChoice(kind string, c asks.Choice, label, asker string, told bool) { + words := "You chose " + label + ": recorded" + switch { + case asker == SelfAsker: + words += "." + case told: + words += ", and " + asker + " was told." + default: + words += "; " + asker + " is told as soon as the bus takes it." + } + err := h.showOn(kind, Message{Title: words, Quiet: true, Reply: c.ID}) + h.record(Sent{At: h.Now(), Channel: kind, Key: "(answer)", What: "confirmed " + label, Outcome: outcomeOf(err)}) +} + // sweepAsks ends asks past their expiry, and tells again an asker not yet told. Called every minute. func (h *Holder) sweepAsks(now time.Time) { h.mu.Lock() @@ -720,7 +771,7 @@ func (h *Holder) sweepAsks(now time.Time) { sort.Strings(expired) for _, k := range expired { e := h.askOf(k) - if err := h.closeAsk(k, asks.OutcomeExpired, "nobody answered in time: "+e.rec.Ask.OnExpiry, nil, ""); err != nil && !errors.Is(err, ErrChanged) { + if err := h.closeAsk(k, asks.OutcomeExpired, "nobody answered in time: "+e.rec.Ask.OnExpiry, nil, "", nil); err != nil && !errors.Is(err, ErrChanged) { h.Logf("[messenger] could not end the expired ask %s: %v", k, err) } } @@ -892,7 +943,8 @@ func (h *Holder) judge(kind string, c asks.Choice, key string, root func(KindInf Option: o.ID, Label: o.Label, Level: o.Level, By: &person, Channel: info.Module, Proofs: proofs, At: now, // The warrant answers this ask as it was taken and shown, with what each option binds (ADR 0259 §6). AskDigest: rec.Ask.Digest()} - if err := h.closeAsk(key, asks.OutcomeChosen, "", w, c.Ticket); err != nil { + confirm := func(told bool) { h.confirmChoice(kind, c, o.Label, rec.Asker, told) } + if err := h.closeAsk(key, asks.OutcomeChosen, "", w, c.Ticket, confirm); err != nil { if errors.Is(err, ErrChanged) { return h.refuseChoice(kind, c, "the question was already answered") } diff --git a/modules/messenger/cmd/messenger/feedback_test.go b/modules/messenger/cmd/messenger/feedback_test.go new file mode 100644 index 00000000..45851af6 --- /dev/null +++ b/modules/messenger/cmd/messenger/feedback_test.go @@ -0,0 +1,152 @@ +package main + +// What the operator sees when they answer a question (issue 372): at once, where they answered, that it was +// recorded — never before it was — or why it was not; then the question itself says how it ended, with no +// answers left on it. + +import ( + "errors" + "strings" + "testing" + "time" +) + +// inOrder is a channel that keeps what it was given in one line, sends and edits alike. +type inOrder struct { + *fakeChannel + log *[]string +} + +func (o inOrder) Send(m Message) (string, error) { + *o.log = append(*o.log, "send "+m.Title) + return o.fakeChannel.Send(m) +} + +func (o inOrder) Edit(id string, m Message) error { + *o.log = append(*o.log, "edit "+m.Title) + return o.fakeChannel.Edit(id, m) +} + +// failingAsks is the router's state refusing to record an ask's end. +type failingAsks struct { + *memAsks + err error +} + +func (f failingAsks) UpdateAsk(k string, r AskRecord, rev uint64) (uint64, error) { + return 0, f.err +} + +func TestAnApprovalIsConfirmedWhereItWasGivenAndThenTheQuestionSaysHowItEnded(t *testing.T) { + h, tg, _, c, store, _ := newRouter(t) + var log []string + h.Away = inOrder{tg, &log} + link(h, store, c, "42", c.now().Add(-2*time.Hour)) + submit(t, h, "mesh-delivery", heldAsk(c, "d-1")) + settle(h, c) + log = nil + ch := choiceOf(ticketFor(tg.sends[0], "Release"), "42", c.now()) + if out, err := h.Chosen("telegram", ch); err != nil || out != "chosen" { + t.Fatalf("%s %v", out, err) + } + want := []string{ + "send You chose Release: recorded, and mesh-delivery was told.", + "edit You chose Release at 12:00 on Telegram · Delivery of hq held for 36 hours", + } + if strings.Join(log, "\n") != strings.Join(want, "\n") { + t.Fatalf("the operator saw, in order:\n%s\nwant:\n%s", strings.Join(log, "\n"), strings.Join(want, "\n")) + } + if confirmed := tg.sends[len(tg.sends)-1]; confirmed.Reply != ch.ID || confirmed.Body != "" { + t.Errorf("the confirmation is not a reply of one line to the tap: %+v", confirmed) + } + // The same tap redelivered is acted on once: nothing more is said, and the question is edited once. + if out, _ := h.Chosen("telegram", ch); out != "chosen" || len(log) != 2 { + t.Errorf("a redelivered tap was said again: %s %q", out, log) + } +} + +// A Decline is confirmed as what it is, in the option's own word. +func TestADeclineIsConfirmedInItsOwnWord(t *testing.T) { + h, tg, _, c, store, _ := newRouter(t) + link(h, store, c, "42", c.now().Add(-2*time.Hour)) + submit(t, h, "mesh-delivery", heldAsk(c, "d-1")) + settle(h, c) + if out, err := h.Chosen("telegram", choiceOf(ticketFor(tg.sends[0], "Stop"), "42", c.now())); err != nil || out != "chosen" { + t.Fatalf("%s %v", out, err) + } + if got := tg.sends[len(tg.sends)-1].Title; got != "You chose Stop: recorded, and mesh-delivery was told." { + t.Errorf("confirmed as %q", got) + } +} + +// The confirmation never claims what did not happen: an end that could not be recorded is said as refused, +// with why, and nothing says "recorded"; a warrant the bus did not take is said as not yet told. +func TestTheConfirmationNeverClaimsWhatDidNotHappen(t *testing.T) { + t.Run("not recorded", func(t *testing.T) { + h, tg, _, c, store, w := newRouter(t) + link(h, store, c, "42", c.now().Add(-2*time.Hour)) + submit(t, h, "mesh-delivery", heldAsk(c, "d-1")) + settle(h, c) + h.Asks = failingAsks{store, errors.New("the bucket is away")} + before := len(tg.sends) + out, err := h.Chosen("telegram", choiceOf(ticketFor(tg.sends[0], "Release"), "42", c.now())) + if err == nil || out != "refused" { + t.Fatalf("%s %v", out, err) + } + for _, m := range tg.sends[before:] { + if strings.Contains(m.Title+m.Body, "recorded,") || strings.Contains(m.Title, "You chose") { + t.Errorf("an answer not recorded was confirmed: %+v", m) + } + } + last := tg.sends[len(tg.sends)-1] + if last.Title != "That answer was not taken" || !strings.Contains(last.Body, "could not be recorded") || last.Reply == "" { + t.Errorf("the refusal does not say why, in reply to the tap: %+v", last) + } + if len(w.of("mesh-delivery")) != 0 || len(tg.edits) != 0 { + t.Error("an answer not recorded was told or shown as the question's end") + } + }) + t.Run("not told", func(t *testing.T) { + h, tg, _, c, store, _ := newRouter(t) + link(h, store, c, "42", c.now().Add(-2*time.Hour)) + submit(t, h, "mesh-delivery", heldAsk(c, "d-1")) + settle(h, c) + h.Publish = func(subject string, body any, id string) error { return errors.New("the bus is away") } + if out, err := h.Chosen("telegram", choiceOf(ticketFor(tg.sends[0], "Release"), "42", c.now())); err != nil || out != "chosen" { + t.Fatalf("%s %v", out, err) + } + if got := tg.sends[len(tg.sends)-1].Title; got != "You chose Release: recorded; mesh-delivery is told as soon as the bus takes it." { + t.Errorf("confirmed as %q", got) + } + var edited Message + for _, m := range tg.edits { + edited = m + } + if !strings.HasPrefix(edited.Body, "Told to mesh-delivery as soon as the bus takes it") { + t.Errorf("the question claims its asker was told: %q", edited.Body) + } + }) +} + +// A refused answer says why, in reply to the tap, with nothing claimed done. +func TestARefusedAnswerSaysWhyInReplyToTheTap(t *testing.T) { + h, tg, _, c, store, _ := newRouter(t) + link(h, store, c, "42", c.now().Add(-2*time.Hour)) + submit(t, h, "mesh-delivery", heldAsk(c, "d-1")) + settle(h, c) + release := ticketFor(tg.sends[0], "Release") + _, _ = h.Chosen("telegram", choiceOf(release, "42", c.now())) + again := choiceOf(release, "42", c.now()) + again.ID = "tg-tap-again" + if out, _ := h.Chosen("telegram", again); out != "refused" { + t.Fatalf("a used ticket was taken: %s", out) + } + last := tg.sends[len(tg.sends)-1] + if last.Reply != "tg-tap-again" || last.Title != "That answer was not taken" || + !strings.HasSuffix(last.Body, "Nothing was done.") || len(strings.TrimSuffix(last.Body, "Nothing was done.")) < 10 { + t.Errorf("the refusal: %+v", last) + } + if _, ok := Check(last.Text()); !ok { + t.Error("the refusal carries what may not leave") + } +} diff --git a/modules/messenger/cmd/messenger/holder.go b/modules/messenger/cmd/messenger/holder.go index 9351544b..aa5b8f62 100644 --- a/modules/messenger/cmd/messenger/holder.go +++ b/modules/messenger/cmd/messenger/holder.go @@ -639,7 +639,7 @@ func (h *Holder) sayOn(r *Record, what, kind string) Message { return m } if r.Answered != "" { - m.Body = strings.TrimSpace(r.Answered + "\n" + m.Body) + m = h.answeredWords(r, m) } if kind == "" { return m @@ -675,6 +675,25 @@ func (h *Holder) sayOn(r *Record, what, kind string) Message { return m } +// answeredWords is a message that says what came of its question. An ask's own message is the question +// itself, so its title becomes how it ended — "You chose Approve at 13:30 on Telegram · " — read at +// a glance on the phone, where the edit rings nothing (issue 372); the rest of what came of it, the +// explanation and when it was asked follow. A condition's message keeps its own title, and says it first. +func (h *Holder) answeredWords(r *Record, m Message) Message { + if r.Origin == "ask" { + first, rest, _ := strings.Cut(r.Answered, "\n") + headline, explanation := wordsOf(r) + out := Message{Title: strings.TrimSuffix(first, ".") + " · " + headline, Urgent: m.Urgent, Quiet: m.Quiet, + Body: strings.TrimSpace(rest + "\n" + explanation + "\nAsked " + when(r.Raised, h.Now(), h.zone()) + ".")} + if _, ok := Check(out.Text()); ok { + return out + } + // Its words refused (they are the asker's): the outcome is still said, before what may leave. + } + m.Body = strings.TrimSpace(r.Answered + "\n" + m.Body) + return m +} + // askLines are what an ask's message says of the ask itself: who asks, each option's effect, and its expiry. func (h *Holder) askLines(rec *AskRecord) []string { asker := rec.Asker diff --git a/modules/messenger/cmd/messenger/selfact.go b/modules/messenger/cmd/messenger/selfact.go index bba67109..b3502420 100644 --- a/modules/messenger/cmd/messenger/selfact.go +++ b/modules/messenger/cmd/messenger/selfact.go @@ -302,7 +302,7 @@ func (h *Holder) endSelfAsks(why string) int { sort.Strings(keys) n := 0 for _, k := range keys { - if err := h.closeAsk(k, asks.OutcomeCancelled, why, nil, ""); err == nil { + if err := h.closeAsk(k, asks.OutcomeCancelled, why, nil, "", nil); err == nil { n++ } } diff --git a/modules/telegram/README.md b/modules/telegram/README.md index 20846196..46fc615e 100644 --- a/modules/telegram/README.md +++ b/modules/telegram/README.md @@ -11,6 +11,13 @@ as the operator. - **Says what the operator chose** on `intake` under its own kind: a tapped button (`choice`), with the sender as Telegram authenticated it; `/start` (`link`); a message it could not deliver (`failed`); and whether it can send (`standing`). Only a private chat with the account itself counts. +- **A tap is acknowledged on the phone with the router's word on it** (issue 372): the router answers a + choice with a message in reply to it — "You chose Approve: recorded, and claude-code was told.", or why it + was not taken — and this channel shows that as the tap's own acknowledgement, the short line Telegram + shows over the chat. A word with more to it (a refusal's reason, Details) is also said in the chat, where it + stays. Nothing is acknowledged before the router's word, so the phone never says "recorded" for what was + not; without a word in 8 seconds the tap is acknowledged with words that claim nothing, and the word when it + comes is said in the chat. A secret is never an acknowledgement. - **A code** (six digits, the one shown on the desk when linking) is asked of the router as a proof, by request and reply: never in an event, never kept, and deleted from the chat at once. - **Judges nothing.** Whether the sender is the operator, and what an answer may do, is the router's. diff --git a/modules/telegram/cmd/telegram/channel.go b/modules/telegram/cmd/telegram/channel.go index 3f7fba54..31a97cf5 100644 --- a/modules/telegram/cmd/telegram/channel.go +++ b/modules/telegram/cmd/telegram/channel.go @@ -11,8 +11,10 @@ package main // clearing arrives without sound, an urgent message or a question rings (D4). A rate limit is waited out // as told (D5); a refusal is a permanent failure, said to the router, never retried; an edit to the same // text is a success (D6). A message that cannot be delivered after three tries is said as failed. -// - **What arrives** is long-polled: outbound only. A tapped button is acknowledged at once (the phone's -// spinner stops) and said as a `choice` with the sender the service authenticated; only a private chat +// - **What arrives** is long-polled: outbound only. A tapped button is said as a `choice` with the sender +// the service authenticated, and acknowledged on the phone with the router's word on it — what was +// recorded, or why it was not — which comes as a reply to the tap; without one in TapWait, with words that +// claim nothing, and the word when it comes is said in the chat (issue 372); only a private chat // with the account itself counts, and only on a message showing the words this channel put there (another // holder of the token may have changed them). `/start` is said as a `link`; six digits are asked as a `code` proof by // request and reply, never in an event, and deleted from the chat. Anything else is answered with one @@ -50,6 +52,13 @@ const ( PollWait = 50 * time.Second // StandingEvery is how often the channel says whether it can send, at least. StandingEvery = 5 * time.Minute + // TapWait is how long a tap's acknowledgement waits for the router's word on it (issue 372): the phone + // shows its spinner meanwhile, and Telegram takes an acknowledgement only for a short while. + TapWait = 8 * time.Second + // TapPending is a tap's acknowledgement when the router said nothing within TapWait: it claims nothing. + TapPending = "Passed to the mesh, which has not decided yet. The question changes when it has." + // ToastLength is what a tap's acknowledgement shows (Telegram's own bound). + ToastLength = 200 // FixedLine is what anything the bot does not take is answered with. FixedLine = "This bot carries the mesh's messages and takes their buttons. To link this account as the " + "operator, send /start." @@ -100,9 +109,12 @@ type Channel struct { Now func() time.Time Logf func(string, ...any) Sleep func(time.Duration) + // After calls f once d has passed (time.AfterFunc); a test calls it when it says. + After func(d time.Duration, f func()) mu sync.Mutex - replyTo map[string]int64 // an arrival's id to the chat it came from, for the router's reply + replyTo map[string]int64 // an arrival's id to the chat it came from, for the router's reply + taps map[string]string // a tap's arrival id to its callback, until it is acknowledged lastPoll time.Time pollErr string meAt time.Time @@ -131,6 +143,12 @@ func (c *Channel) init() { if c.Sleep == nil { c.Sleep = time.Sleep } + if c.taps == nil { + c.taps = map[string]string{} + } + if c.After == nil { + c.After = func(d time.Duration, f func()) { time.AfterFunc(d, f) } + } } // text is a message as the chat shows it, cut at MaxLength and the cut said (D1). @@ -195,6 +213,15 @@ func (c *Channel) Take(subject string, body json.RawMessage) error { c.Logf("[telegram] work on %s could not be read; taken and dropped", subject) return nil } + // The router's word on a tap, said where the tap was made: the tap's own acknowledgement (issue 372). A + // word of one line is that alone; one with more to it is also said in the chat, where it stays. + if verb == asks.Send && m.Reply != "" && !m.Secret && c.settleTap(m.Reply, toast(m)) && strings.TrimSpace(m.Body) == "" { + c.mu.Lock() + c.lastSent, c.lastErr = c.Now(), "" + c.delivered++ + c.mu.Unlock() + return nil + } chat, err := c.chatFor(m) if err != nil { c.fail(m.Handle, err, true) @@ -301,15 +328,22 @@ func (c *Channel) Handle(u Update) { c.Logf("[telegram] a tap on a message not showing what this channel put there was refused") return } - // At once, whatever comes of it: the phone shows a spinner until then. - c.answerTap(cb.ID, "") + // Acknowledged with the router's word on it, which comes as a reply to this tap (Take): what was recorded, + // or why it was refused. Never before: an acknowledgement claims nothing the router has not said. If the + // router says nothing within TapWait, the tap is acknowledged with words that claim nothing, and its word + // when it comes is said in the chat (issue 372). id := "tg-tap-" + cb.ID c.remember(id, cb.Message.Chat.ID) + c.mu.Lock() + c.taps[id] = cb.ID + c.mu.Unlock() ch := asks.Choice{ID: id, Ticket: cb.Data, Sender: senderOf(cb.From), At: now} if err := c.Publish(asks.IntakeSubject(asks.Chosen, Kind), ch, id); err != nil { c.Logf("[telegram] a tap could not be said to the router: %v", err) - _ = c.Bot.AnswerTap(cb.ID, "The mesh could not take this answer now. Try again in a minute.") + c.settleTap(id, "The mesh could not take this answer now. Nothing was done. Try again in a minute.") + return } + c.After(TapWait, func() { c.settleTap(id, TapPending) }) case u.Message != nil: m := u.Message if !private(m.From, m.Chat) { @@ -345,6 +379,37 @@ func (c *Channel) Handle(u Update) { } } +// settleTap acknowledges a tap still waiting, once, with words: true when it was waiting and Telegram took +// the acknowledgement. A tap already acknowledged, or one Telegram no longer takes, is false: the words are +// then for the chat. +func (c *Channel) settleTap(arrival, words string) bool { + c.mu.Lock() + cb, waiting := c.taps[arrival] + delete(c.taps, arrival) + c.mu.Unlock() + if !waiting { + return false + } + if err := c.Bot.AnswerTap(cb, words); err != nil { + c.Logf("[telegram] a tap could not be answered: %v", err) + return false + } + return true +} + +// toast is a message as a tap's acknowledgement shows it: its title and the first line of its body, cut at +// ToastLength. +func toast(m asks.Message) string { + t := strings.TrimSpace(m.Title) + if first, _, _ := strings.Cut(strings.TrimSpace(m.Body), "\n"); first != "" { + t += ": " + first + } + if utf8.RuneCountInString(t) > ToastLength { + t = string([]rune(t)[:ToastLength-1]) + "…" + } + return t +} + func (c *Channel) answerTap(id, words string) { if err := c.Bot.AnswerTap(id, words); err != nil { c.Logf("[telegram] a tap could not be answered: %v", err) @@ -481,7 +546,7 @@ func (c *Channel) Status() map[string]any { out["rules"] = []string{ "it shows what the router sends and says what was chosen; whether the sender is the operator is the router's to judge, from the account linked there (novox/hq ADR 0259)", "a button carries only the router's one-time ticket", - "only a private chat with the account itself counts; a tap is acknowledged at once", + "only a private chat with the account itself counts; a tap is acknowledged with the router's word on it — what was recorded, or why not — or, without one in 8 seconds, with words that claim nothing", "/start asks to link this account; the code shown on the desk is checked by request and reply, never in an event, and deleted from the chat", "updates are long-polled: outbound only, no route into the mesh", "up to 4096 characters arrive whole; longer is cut and the cut said", diff --git a/modules/telegram/cmd/telegram/channel_test.go b/modules/telegram/cmd/telegram/channel_test.go index 5d6437cc..5b518471 100644 --- a/modules/telegram/cmd/telegram/channel_test.go +++ b/modules/telegram/cmd/telegram/channel_test.go @@ -2,6 +2,8 @@ package main import ( "encoding/json" + "errors" + "fmt" "net/http" "net/http/httptest" "os" @@ -11,6 +13,7 @@ import ( "sync" "testing" "time" + "unicode/utf8" "git.novox.be/novox/mesh-sdk/go/asks" ) @@ -121,6 +124,7 @@ type said struct { mu sync.Mutex by map[string][]json.RawMessage proved []json.RawMessage + later []func() } func (s *said) of(subject string) []json.RawMessage { @@ -160,10 +164,26 @@ func newChannel(t *testing.T, token string) (*Channel, *fakeAPI, *said, *memStor } return json.RawMessage(`{"accepted":false,"words":"That code is not right."}`), nil }} + c.After = func(d time.Duration, fn func()) { + s.mu.Lock() + s.later = append(s.later, fn) + s.mu.Unlock() + } c.init() return c, f, s, st } +// waited runs what was set to happen later: the router said nothing in time. +func (s *said) waited() { + s.mu.Lock() + later := s.later + s.later = nil + s.mu.Unlock() + for _, fn := range later { + fn() + } +} + func work(verb string, m asks.Message) (string, json.RawMessage) { raw, _ := json.Marshal(m) return asks.ChannelSubject(verb, Kind), raw @@ -274,9 +294,6 @@ func TestATapIsAcknowledgedAtOnceAndSaidWithItsAuthenticatedSender(t *testing.T) shown := showQuestion(t, c, f) c.Handle(Update{UpdateID: 1, Callback: &Callback{ID: "cb1", Data: "tkt", From: User{ID: 42, FirstName: "Jo"}, Message: &Msg{MessageID: shown, Chat: Chat{ID: 42, Type: "private"}, Text: "Release it?\nThe delivery is held."}}}) - if len(f.of("answerCallbackQuery")) != 1 { - t.Error("the tap was not acknowledged") - } got := s.of(asks.IntakeSubject(asks.Chosen, Kind)) var ch asks.Choice if len(got) != 1 || json.Unmarshal(got[0], &ch) != nil || ch.Ticket != "tkt" || @@ -430,3 +447,136 @@ func TestATapOnAMessageShowingOtherWordsIsRefused(t *testing.T) { t.Error("what an edit showed was not kept") } } + +// tapOn taps the question showQuestion put there, as the operator. +func tapOn(c *Channel, shown int64, id string) { + c.Handle(Update{UpdateID: 1, Callback: &Callback{ID: id, Data: "tkt", From: User{ID: 42, FirstName: "Jo"}, + Message: &Msg{MessageID: shown, Chat: Chat{ID: 42, Type: "private"}, Text: "Release it?\nThe delivery is held."}}}) +} + +// The operator taps Approve and sees at once what the router recorded (issue 372): the tap is acknowledged +// with the router's word on it, which comes as a reply to the tap — never before it, so the phone never says +// "recorded" for what the router did not record — and a word of one line goes nowhere else. +func TestATapIsAcknowledgedWithTheRoutersWordOnIt(t *testing.T) { + c, f, s, _ := newChannel(t, goodToken) + shown := showQuestion(t, c, f) + tapOn(c, shown, "cb1") + if got := f.of("answerCallbackQuery"); len(got) != 0 { + t.Fatalf("the tap was acknowledged before the router said anything: %v", got) + } + sentBefore := len(f.of("sendMessage")) + subject, body := work(asks.Send, asks.Message{Handle: "r1", Title: "You chose Release: recorded, and mesh-delivery was told.", + Silent: true, Reply: "tg-tap-cb1", To: "42"}) + if err := c.Take(subject, body); err != nil { + t.Fatal(err) + } + got := f.of("answerCallbackQuery") + if len(got) != 1 || got[0].Body["callback_query_id"] != "cb1" || + got[0].Body["text"] != "You chose Release: recorded, and mesh-delivery was told." { + t.Fatalf("the tap was acknowledged with %v", got) + } + if len(f.of("sendMessage")) != sentBefore { + t.Error("a word of one line was also sent to the chat") + } + // The wait running out after the word changes nothing: a tap is acknowledged once. + s.waited() + if len(f.of("answerCallbackQuery")) != 1 { + t.Error("the tap was acknowledged twice") + } + // The same word again, once the tap is acknowledged, is said in the chat. + if err := c.Take(subject, body); err != nil { + t.Fatal(err) + } + if len(f.of("sendMessage")) != sentBefore+1 { + t.Error("a word for a tap already acknowledged was lost") + } +} + +// A refused tap says why on the phone, and the refusal stays in the chat (issue 372). +func TestARefusedTapSaysWhyOnThePhoneAndInTheChat(t *testing.T) { + c, f, _, _ := newChannel(t, goodToken) + shown := showQuestion(t, c, f) + tapOn(c, shown, "cb1") + sentBefore := len(f.of("sendMessage")) + subject, body := work(asks.Send, asks.Message{Handle: "r1", Title: "That answer was not taken", + Body: "The question was already answered or has ended. Nothing was done.", Silent: true, Reply: "tg-tap-cb1", To: "42"}) + if err := c.Take(subject, body); err != nil { + t.Fatal(err) + } + got := f.of("answerCallbackQuery") + if len(got) != 1 || got[0].Body["text"] != "That answer was not taken: The question was already answered or has ended. Nothing was done." { + t.Fatalf("the tap was acknowledged with %v", got) + } + if len(f.of("sendMessage")) != sentBefore+1 { + t.Error("the refusal was not kept in the chat") + } +} + +// Without the router's word in time, the tap is acknowledged with words that claim nothing, and the word when +// it comes is said in the chat. A tap Telegram no longer takes an acknowledgement for is the same. +func TestATapTheRouterIsSlowOnClaimsNothingAndItsWordComesInTheChat(t *testing.T) { + c, f, s, _ := newChannel(t, goodToken) + shown := showQuestion(t, c, f) + tapOn(c, shown, "cb1") + s.waited() + got := f.of("answerCallbackQuery") + if len(got) != 1 || got[0].Body["text"] != TapPending || strings.Contains(TapPending, "record") { + t.Fatalf("a slow router's tap was acknowledged with %v", got) + } + sentBefore := len(f.of("sendMessage")) + subject, body := work(asks.Send, asks.Message{Handle: "r1", Title: "You chose Release: recorded.", Reply: "tg-tap-cb1", To: "42"}) + if err := c.Take(subject, body); err != nil { + t.Fatal(err) + } + if sent := f.of("sendMessage"); len(sent) != sentBefore+1 || sent[len(sent)-1].Body["text"] != "You chose Release: recorded." { + t.Errorf("the late word was not said in the chat: %v", sent) + } + // Telegram refuses an acknowledgement that came too late: the word goes to the chat instead. + f.answer["answerCallbackQuery"] = func(map[string]any) (int, string) { + return 400, `{"ok":false,"error_code":400,"description":"Bad Request: query is too old and response timeout expired or query ID is invalid"}` + } + tapOn(c, shown, "cb2") + subject, body = work(asks.Send, asks.Message{Handle: "r2", Title: "You chose Release: recorded.", Reply: "tg-tap-cb2", To: "42"}) + if err := c.Take(subject, body); err != nil { + t.Fatal(err) + } + if len(f.of("sendMessage")) != sentBefore+2 { + t.Error("a word Telegram would not take as the acknowledgement was lost") + } +} + +// A tap the router cannot be told of is acknowledged at once with that, claiming nothing done; a secret +// (a link's code) is never shown as an acknowledgement. +func TestATapNotSaidToTheRouterIsSaidSoAndASecretIsNeverAToast(t *testing.T) { + c, f, _, _ := newChannel(t, goodToken) + shown := showQuestion(t, c, f) + c.Publish = func(string, any, string) error { return errors.New("the bus is away") } + tapOn(c, shown, "cb1") + got := f.of("answerCallbackQuery") + if len(got) != 1 || !strings.Contains(got[0].Body["text"].(string), "Nothing was done") { + t.Fatalf("acknowledged with %v", got) + } + c.Publish = func(string, any, string) error { return nil } + tapOn(c, shown, "cb2") + subject, body := work(asks.Send, asks.Message{Handle: "r1", Title: "Your code", Body: "123456", Secret: true, + Reply: "tg-tap-cb2", To: "42"}) + if err := c.Take(subject, body); err != nil { + t.Fatal(err) + } + for _, a := range f.of("answerCallbackQuery") { + if strings.Contains(fmt.Sprint(a.Body["text"]), "123456") { + t.Fatal("a secret was shown as a tap's acknowledgement") + } + } +} + +// The toast is the title and the first line of the body, cut at Telegram's bound. +func TestAToastIsCutAtTelegramsBound(t *testing.T) { + long := toast(asks.Message{Title: strings.Repeat("a", 300)}) + if utf8.RuneCountInString(long) != ToastLength || !strings.HasSuffix(long, "…") { + t.Errorf("a long toast is %d runes", utf8.RuneCountInString(long)) + } + if got := toast(asks.Message{Title: "Details: x", Body: "first\nsecond"}); got != "Details: x: first" { + t.Errorf("toast %q", got) + } +}