refactor(studio): always read a thread when the bot is mentioned in it

The thread_context binding setting is removed. When the kind declares
the history ability and the bot is mentioned in a thread, the host
reads the thread once for a new chat, as before. The other guards stay:
the ability check, the bound, the untrusted block, and a failure never
blocks the answer.

A binding stored by an earlier build still loads. The JSON store
ignores the unknown field. The GORM row no longer maps the
thread_context_off column; AutoMigrate does not drop it and it accepts
NULL, so reads and writes keep working. No data is migrated or removed.

Assisted-by: Claude Code:claude-sonnet-5-5
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
This commit is contained in:
Ettore Di Giacinto committed 2026-11-03 11:54:33 +00:00
1 parent 11d1f972ed
commit c8cb68b434
12 files changed
+61 -66

No files matched your search

+2 -2
View File
@@ -24,7 +24,7 @@ func connectorOut(i studiosvc.ConnectorInfo) schema.ChatsConnector {
}
return schema.ChatsConnector{ID: i.ID, Name: i.Name, Kind: i.Kind, Addon: i.AddonID, Active: i.Active, Reason: i.Reason, AgentID: i.AgentID, Enabled: i.Enabled, AcceptAny: i.AcceptAny,
Allow: allow, NotifyChat: i.NotifyChat, HasToken: i.HasToken, Created: i.Created,
ReplyMode: i.ReplyMode(), Conversation: i.ConversationMode(), WindowMinutes: i.WindowMinutes, SendFiles: i.SendsFiles(), ThreadContext: i.ReadsThread(), Options: i.Options,
ReplyMode: i.ReplyMode(), Conversation: i.ConversationMode(), WindowMinutes: i.WindowMinutes, SendFiles: i.SendsFiles(), Options: i.Options,
Files: filesOut(i.Files), FileReceiveMaxBytes: i.FileReceiveMax, FileSendMaxBytes: i.FileSendMax}
}
@@ -46,7 +46,7 @@ func failAcceptAny(c echo.Context) error {
func connectorIn(req schema.ChatsConnectorWrite) studiosvc.ConnectorInput {
return studiosvc.ConnectorInput{Kind: req.Kind, Name: req.Name, Token: req.Token, AgentID: req.AgentID, Enabled: req.Enabled,
AcceptAny: req.AcceptAny, Allow: req.Allow, NotifyChat: req.NotifyChat,
Reply: req.ReplyMode, Conversation: req.Conversation, WindowMinutes: req.WindowMinutes, SendFiles: req.SendFiles, ThreadContext: req.ThreadContext, Options: req.Options,
Reply: req.ReplyMode, Conversation: req.Conversation, WindowMinutes: req.WindowMinutes, SendFiles: req.SendFiles, Options: req.Options,
FileReceiveMax: req.FileReceiveMaxBytes, FileSendMax: req.FileSendMaxBytes}
}
+4 -8
View File
@@ -195,17 +195,13 @@ var _ = Describe("Chats connectors endpoints", func() {
Expect(s.do("alice", http.MethodPost, "/api/chats/connectors/"+id+"/senders/99/allow", nil).Code).To(Equal(http.StatusBadRequest))
})
It("stores the thread context switch: on for a new binding, kept by an update that omits it", func() {
It("has no thread_context field: the answer omits it and a request that sends it is not stored", func() {
ag := agent("alice", "Context", "test-model")
rec := create("alice", map[string]any{"name": "Context bot", "token": specToken, "agent_id": ag})
rec := create("alice", map[string]any{"name": "Context bot", "token": specToken, "agent_id": ag, "thread_context": false})
Expect(rec.Code).To(Equal(http.StatusCreated), rec.Body.String())
Expect(decodeJSON(rec)).To(HaveKeyWithValue("thread_context", true), "the default")
Expect(decodeJSON(rec)).NotTo(HaveKey("thread_context"))
id := decodeJSON(rec)["id"].(string)
rec = s.do("alice", http.MethodPut, "/api/chats/connectors/"+id, map[string]any{"name": "Context bot", "agent_id": ag, "thread_context": false})
Expect(rec.Code).To(Equal(http.StatusOK), rec.Body.String())
Expect(decodeJSON(rec)).To(HaveKeyWithValue("thread_context", false))
rec = s.do("alice", http.MethodPut, "/api/chats/connectors/"+id, map[string]any{"name": "Context bot", "agent_id": ag})
Expect(decodeJSON(rec)).To(HaveKeyWithValue("thread_context", false), "an update without the field keeps it")
Expect(decodeJSON(s.do("alice", http.MethodGet, "/api/chats/connectors/"+id, nil))).NotTo(HaveKey("thread_context"))
})
It("refuses an option that the kind does not declare", func() {
+2 -8
View File
@@ -1177,10 +1177,6 @@ type ChatsConnector struct {
// SendFiles sends the files that a turn makes to the sender, after the
// answer.
SendFiles bool `json:"send_files"`
// ThreadContext lets the bot read the earlier messages of the thread in
// which it is mentioned. It has an effect only on a platform that can
// give them.
ThreadContext bool `json:"thread_context"`
// Options are the choices made for the options of the kind, by option id.
// An option that is absent has its default (see ChatsConnectorKind).
Options map[string]string `json:"options,omitempty"`
@@ -1238,7 +1234,8 @@ type ChatsConnectorKind struct {
// Page is the add-on page that adds and edits bindings of this kind.
Page string `json:"page,omitempty"`
// ThreadHistory is true when the platform can give the earlier messages
// of a thread, so a binding of this kind has the thread_context option.
// of a thread. The host then reads them whenever the bot is mentioned in a
// thread.
ThreadHistory bool `json:"thread_history,omitempty"`
// Options are the choices the add-on lets each binding of this kind make.
Options []ChatsConnectorOption `json:"options,omitempty"`
@@ -1294,9 +1291,6 @@ type ChatsConnectorWrite struct {
// SendFiles turns the sending of a turn's files on or off. Absent means
// on for a new binding and unchanged for an update.
SendFiles *bool `json:"send_files,omitempty"`
// ThreadContext turns the reading of the earlier messages of a thread on
// or off. Absent means on for a new binding and unchanged for an update.
ThreadContext *bool `json:"thread_context,omitempty"`
// Options are the choices for the options of the kind, by option id. Absent
// means the defaults for a new binding and unchanged for an update.
Options map[string]string `json:"options,omitempty"`
@@ -70,7 +70,6 @@ func Run(newStore NewStore) {
got, _ = s.GetBinding(ctx, "modes")
Expect(got.ReplyMode()).To(Equal(connectors.ReplyProgress))
Expect(got.SendsFiles()).To(BeTrue())
Expect(got.ReadsThread()).To(BeTrue(), "a binding that says nothing reads the thread")
})
It("keeps the options of the kind", func() {
s := newStore()
@@ -85,15 +84,6 @@ func Run(newStore NewStore) {
Expect(again.Options["followThreads"]).To(Equal("never"))
Expect(connectors.Validate(connectors.Binding{Name: "n", Kind: "k", AgentID: "a", Options: map[string]string{"": "x"}})).NotTo(Succeed())
})
It("keeps the thread context switch", func() {
s := newStore()
b := binding("ctx", "u1", "Context")
b.ThreadContextOff = true
Expect(s.PutBinding(ctx, b)).To(Succeed())
got, err := s.GetBinding(ctx, "ctx")
Expect(err).NotTo(HaveOccurred())
Expect(got.ReadsThread()).To(BeFalse())
})
It("keeps the file limits an owner set for a binding", func() {
s := newStore()
b := binding("limits", "u1", "Limits")
+5 -5
View File
@@ -28,9 +28,9 @@ type bindingRow struct {
Conversation string `gorm:"size:16"`
WindowMinutes int
FilesOff bool
// ThreadContextOff is added by AutoMigrate; a row stored before it reads
// the thread.
ThreadContextOff bool
// A thread_context_off column that an earlier build added stays in the
// table, unread and unwritten: AutoMigrate never drops a column, and the
// column accepts NULL, so an insert without it works.
// Options is JSON, added by AutoMigrate.
Options string
// The columns of the file limits are added by AutoMigrate; a row stored
@@ -93,7 +93,7 @@ func NewGORM(db *gorm.DB) (Store, error) {
func (r bindingRow) binding() (Binding, error) {
b := Binding{ID: r.ID, Owner: r.Owner, Kind: r.Kind, AddonID: r.AddonID, Name: r.Name, AgentID: r.AgentID, Enabled: r.Enabled, AcceptAny: r.AcceptAny,
NotifyChat: r.NotifyChat, Reply: r.Reply, Conversation: r.Conversation, WindowMinutes: r.WindowMinutes, FilesOff: r.FilesOff, ThreadContextOff: r.ThreadContextOff,
NotifyChat: r.NotifyChat, Reply: r.Reply, Conversation: r.Conversation, WindowMinutes: r.WindowMinutes, FilesOff: r.FilesOff,
FileReceiveMax: r.FileReceiveMax, FileSendMax: r.FileSendMax,
Token: append([]byte(nil), r.Token...), Created: r.Created.UTC(), Updated: r.Updated.UTC()}
if r.Allow != "" {
@@ -122,7 +122,7 @@ func (s *gormStore) PutBinding(ctx context.Context, b Binding) error {
}
return s.db.WithContext(ctx).Save(&bindingRow{ID: b.ID, Owner: b.Owner, Kind: b.Kind, AddonID: b.AddonID, Name: b.Name, AgentID: b.AgentID, Enabled: b.Enabled,
AcceptAny: b.AcceptAny, Allow: allow, NotifyChat: b.NotifyChat, Reply: b.Reply, Conversation: b.Conversation, WindowMinutes: b.WindowMinutes,
FilesOff: b.FilesOff, ThreadContextOff: b.ThreadContextOff, Options: options, FileReceiveMax: b.FileReceiveMax, FileSendMax: b.FileSendMax,
FilesOff: b.FilesOff, Options: options, FileReceiveMax: b.FileReceiveMax, FileSendMax: b.FileSendMax,
Token: b.Token, Created: b.Created, Updated: b.Updated}).Error
}
@@ -1,6 +1,7 @@
package connectors_test
import (
"context"
"path/filepath"
. "github.com/onsi/ginkgo/v2"
@@ -32,6 +33,21 @@ var _ = Describe("GORM on SQLite", func() {
})
})
It("loads and rewrites a row that an earlier build stored with a thread_context_off column", func() {
path := filepath.Join(GinkgoT().TempDir(), "c.db")
first, _ := connectors.NewGORM(openSQLite(path))
connectorstest.Seed(first)
db := openSQLite(path)
Expect(db.Exec("ALTER TABLE studio_connectors ADD COLUMN thread_context_off numeric").Error).To(Succeed())
Expect(db.Exec("UPDATE studio_connectors SET thread_context_off = 1").Error).To(Succeed())
second, err := connectors.NewGORM(db)
Expect(err).NotTo(HaveOccurred())
connectorstest.CheckSeed(second)
b, err := second.GetBinding(context.Background(), "seed")
Expect(err).NotTo(HaveOccurred())
Expect(second.PutBinding(context.Background(), b)).To(Succeed(), "an insert or update works with the old column left in place")
})
It("survives a reopen", func() {
path := filepath.Join(GinkgoT().TempDir(), "c.db")
first, _ := connectors.NewGORM(openSQLite(path))
@@ -4,6 +4,7 @@ import (
"context"
"os"
"path/filepath"
"strings"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
@@ -43,6 +44,21 @@ var _ = Describe("the JSON store", func() {
}
})
It("loads a file that an earlier build wrote with the ThreadContextOff field", func() {
dir := GinkgoT().TempDir()
first, _ := connectors.NewJSON(dir)
connectorstest.Seed(first)
p := filepath.Join(dir, "connectors.json")
raw, err := os.ReadFile(p)
Expect(err).NotTo(HaveOccurred())
patched := strings.Replace(string(raw), `"Name":`, `"ThreadContextOff":true,"Name":`, -1)
Expect(patched).NotTo(Equal(string(raw)))
Expect(os.WriteFile(p, []byte(patched), 0o600)).To(Succeed())
second, err := connectors.NewJSON(dir)
Expect(err).NotTo(HaveOccurred())
connectorstest.CheckSeed(second)
})
It("refuses a key file that others can read", func() {
dir := GinkgoT().TempDir()
first, _ := connectors.NewJSON(dir)
-8
View File
@@ -110,10 +110,6 @@ type Binding struct {
// zero value sends them, so a binding stored before the option sends
// them too.
FilesOff bool
// ThreadContextOff keeps the host from reading the earlier messages of
// a thread when the bot is mentioned in it. The zero value reads them,
// on a platform whose kind declares the ability.
ThreadContextOff bool
// Options are the choices the owner made for the options that the kind of
// the binding declares (contract.ConnectorKind.Options), by option ID. An
// option that is not here has its default.
@@ -251,10 +247,6 @@ func (b Binding) ConversationMode() string {
// SendsFiles tells whether the files a turn makes go to the sender.
func (b Binding) SendsFiles() bool { return !b.FilesOff }
// ReadsThread tells whether the earlier messages of a thread are read when
// the bot is mentioned in it.
func (b Binding) ReadsThread() bool { return !b.ThreadContextOff }
// Allows tells whether a sender is on the allowlist. A match is by ID or, for an
// @name entry, by username, ignoring case.
func (b Binding) Allows(senderID, username string) bool {
@@ -148,15 +148,12 @@ var _ = Describe("connectors slack", func() {
waitFor("the answer to the reply", func() bool { return len(r2.answers()) == 2 })
})
It("slack mention outside a thread, or with the option off, reads nothing", func() {
off := false
r, _ := newSlackRigWith(connSay("ok"), studio.ConnectorInput{Allow: []string{"C1"}, ThreadContext: &off})
r.slack.AddThreadMessage("C1", "100.000001", "100.000001", map[string]any{"user": "U1", "text": "earlier"})
r.slack.Push(slackApp, slacktest.Mention("C1", "U1", "UBOT", "inside", "100.000001"))
waitFor("the answer", func() bool { return len(r.answers()) == 1 })
It("slack mention outside a thread gets no earlier-messages block", func() {
r, _ := newSlackRigWith(connSay("ok"), studio.ConnectorInput{Allow: []string{"C1"}})
r.slack.Push(slackApp, slacktest.Mention("C1", "U1", "UBOT", "top level", ""))
waitFor("the second answer", func() bool { return len(r.answers()) == 2 })
Expect(r.slack.Calls("conversations.replies")).To(BeZero())
waitFor("the answer", func() bool { return len(r.answers()) == 1 })
// The add-on makes the mention the root of its own thread, so the host may
// ask; the thread holds nothing but the message, so there is no block.
chats, _ := r.svc.List(context.Background(), alice, 10)
for _, c := range chats {
for _, m := range r.h.Sent(c.ID) {
@@ -131,20 +131,21 @@ var _ = Describe("the earlier messages of a thread", func() {
Expect(fake.calls()).To(HaveLen(1), "one history call per new chat")
})
It("asks nothing for a message outside a thread, or when the binding turns the reading off", func() {
It("asks nothing for a message outside a thread", func() {
setup(&contract.ConnectorHistory{Thread: true}, studio.ConnectorInput{})
m := mention("m5", "")
m.Direct = true
ingest(m)
answered(1)
Expect(fake.calls()).To(BeEmpty())
})
off := false
setup(&contract.ConnectorHistory{Thread: true}, studio.ConnectorInput{ThreadContext: &off})
It("always reads the thread when the kind declares the ability: no binding input changes it", func() {
setup(&contract.ConnectorHistory{Thread: true}, studio.ConnectorInput{Name: "Any", Enabled: true})
ingest(mention("m5", "T1"))
answered(1)
Expect(fake.calls()).To(BeEmpty())
Expect(r.h.Sent(chat())[0]).To(Equal("@bot can you retry?"))
Expect(fake.calls()).To(HaveLen(1))
Expect(r.h.Sent(chat())[0]).To(ContainSubstring("Earlier"))
})
It("never calls a kind that declares no history", func() {
-7
View File
@@ -38,10 +38,6 @@ type ConnectorInput struct {
// SendFiles sends the files a turn makes to the sender. Nil means on for
// a new binding and unchanged for an update.
SendFiles *bool
// ThreadContext lets the bot read the earlier messages of the thread in
// which it is mentioned. Nil means on for a new binding and unchanged for
// an update.
ThreadContext *bool
// Options are the choices for the options of the kind. Nil means the
// defaults for a new binding and unchanged for an update.
Options map[string]string
@@ -64,9 +60,6 @@ func (in ConnectorInput) apply(b *connectors.Binding) {
if in.Options != nil {
b.Options = maps.Clone(in.Options)
}
if in.ThreadContext != nil {
b.ThreadContextOff = !*in.ThreadContext
}
if in.FileReceiveMax != nil {
b.FileReceiveMax = *in.FileReceiveMax
}
+5 -5
View File
@@ -26,12 +26,12 @@ const (
// threadDirective reads the earlier messages of the thread of m from the
// platform and returns the block the first turn of the new chat gets, or "".
// It asks only when the message is in a thread, the binding reads threads and
// the add-on of the kind declares the ability. A failure never blocks the
// answer: it is logged and the turn goes on without context. The caller calls
// it once, when the chat of the thread has just been made.
// It asks only when the message is in a thread and the add-on of the kind
// declares the ability; no binding setting turns it off. A failure never
// blocks the answer: it is logged and the turn goes on without context. The
// caller calls it once, when the chat of the thread has just been made.
func (c *Connectors) threadDirective(ctx context.Context, b connectors.Binding, token string, m contract.ConnectorMessage) string {
if m.Thread == "" || !b.ReadsThread() {
if m.Thread == "" {
return ""
}
kinds, err := c.o.Transport.Kinds(ctx)