amadan

eleven / messenger Public

bot: fix the first real-world bumps in conversational bot updates (#812)

Live on chat.paulca.com, "switch to the Irish Times" and "what about The
Journal?" both failed their dry runs, for three separate reasons:

- The fetch allowlist broke on the apex->www redirect: discovery finds no
  advertised feed on irishtimes.com, falls back to the homepage — whose
  301 to www.irishtimes.com then fails the redirect re-check, so every
  fetch dies. The allowlist now tolerates the www dance (feed discovery
  already did). While in the matcher: prefix matching is now
  boundary-aware — a bare-host entry ("https://irishtimes.com") no longer
  matches a longer lookalike host ("https://irishtimes.com.evil.com"),
  which matters because the model picks fetch URLs.

- Task runs share a 4096-token budget with adaptive thinking, and a long
  feed digest can spend it all thinking — truncating before any text (The
  Journal's "came back empty"). An empty result now retries once with
  thinking off, which has the whole budget for the answer.

- A failed UPDATE spoke create-path copy ("Couldn't set up… Nothing was
  created."), reading like the bot was gone. It now says "Couldn't
  update…" / "I couldn't make that change — … Your bot is unchanged and
  still running as before." And sources-down is classified before empty,
  so the member (and the log) hear the real reason.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 830adc257ce32c1c0256156fc2eb2d27cb10ea78 parent b4ae394
8 files changed, +350 −20
  • bot_charterupdate_test.go +136 −0
  • changelog/2026-07-13-bot-update-fixes.md +5 −0
  • internal/bot/botbuild.go +21 −8
  • internal/bot/chartersched.go +6 −3
  • internal/bot/fetch.go +46 −4
  • internal/bot/fetch_test.go +53 −0
  • internal/bot/task.go +24 −5
  • internal/bot/task_test.go +59 −0
diff --git a/bot_charterupdate_test.go b/bot_charterupdate_test.go
new file mode 100644
index 0000000..aadf87f
--- /dev/null
+++ b/bot_charterupdate_test.go
@@ -0,0 +1,136 @@
+package main
+
+// End-to-end test of a FAILED conversational bot update (internal/bot:
+// botbuild.go): the owner asks, in the bot's own chat, to switch it to a
+// source that turns out to be unreadable. The dry-run gate must refuse the
+// change — and say so in update terms ("your bot is unchanged"), never the
+// create-path "nothing was created", which reads like the bot is gone. The
+// stored charter must be untouched.
+
+import (
+ "bytes"
+ "context"
+ "encoding/json"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+ "time"
+
+ "lchat/internal/bot"
+ "lchat/internal/e2ee"
+)
+
+func TestCharterUpdateFailureLeavesBotUntouched(t *testing.T) {
+ t.Setenv("LCHAT_BOT_ALLOW_PRIVATE_FETCH", "1")
+
+ // The would-be new source: unreadable (403 to everything).
+ blocked := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.WriteHeader(http.StatusForbidden)
+ }))
+ defer blocked.Close()
+
+ fakeAI := scriptedAnthropic(t,
+ // Turn 1 — the owner's ask in the bot's chat: an update via create_bot
+ // with the bot id from the brain's charter-home context.
+ []map[string]any{
+ {"type": "text", "text": "Switching it over now."},
+ {"type": "tool_use", "name": "create_bot", "input": map[string]any{
+ "bot_id": "bot-home01", "name": "home-news", "title": "Home News", "glyph": "🏠",
+ "every": "1h",
+ "sources": []any{blocked.URL},
+ "prompt": "Fetch the new source and summarise it.",
+ }},
+ },
+ // The dry run: one fetch (403s), then an apology.
+ []map[string]any{{"type": "tool_use", "id": "tu_1", "name": "fetch", "input": map[string]any{"url": blocked.URL}}},
+ []map[string]any{{"type": "text", "text": "I couldn't reach the new source."}},
+ )
+ t.Setenv("ANTHROPIC_BASE_URL", fakeAI.URL)
+
+ ts, s := newTestServer(t)
+ owner := provisionNamed(t, ts.URL, "Owner", false)
+ eleven := provisionNamed(t, ts.URL, "Eleven", true)
+ ownerUID, _ := s.db.deviceTokenUser(owner.Token)
+ instKey := grantInstanceKey(t, ts.URL, owner, eleven, s)
+ threadID, threadKey := createThreadKind(t, ts.URL, s, "group", owner, eleven)
+
+ farSlot := time.Now().UTC().Add(12 * time.Hour).Format("15:04")
+ charter := map[string]any{
+ "v": 1, "name": "home-news", "title": "Home News", "glyph": "🏠",
+ "schedule": map[string]any{"at": farSlot},
+ "task": map[string]any{"prompt": "Write the household news."},
+ "thread": threadID, "owner": ownerUID,
+ }
+ blob, err := e2ee.EncryptJSON(instKey, charter)
+ if err != nil {
+ t.Fatal(err)
+ }
+ body, _ := json.Marshal(map[string]string{"token": eleven.Token, "blob": blob, "owner_id": ownerUID})
+ req, _ := http.NewRequest(http.MethodPut, ts.URL+"/api/member-bots/bot-home01", bytes.NewReader(body))
+ req.Header.Set("Content-Type", "application/json")
+ if resp, err := http.DefaultClient.Do(req); err != nil || resp.StatusCode != http.StatusOK {
+ t.Fatalf("seed charter: %v %d", err, resp.StatusCode)
+ }
+
+ dir := t.TempDir()
+ sessionPath := dir + "/eleven.session.json"
+ if err := eleven.Save(sessionPath); err != nil {
+ t.Fatal(err)
+ }
+ ready := make(chan struct{})
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+ eng := &bot.Engine{Base: ts.URL, Session: eleven, SessionPath: sessionPath,
+ Recipe: bot.DefaultRecipe(), AIKey: "test-key", Charters: true,
+ Ready: func() { close(ready) }, Logf: t.Logf}
+ go eng.Run(ctx)
+ awaitReady(t, ready)
+
+ postAsMember(t, ts.URL, threadID, threadKey, owner, ownerUID, "Switch it to the blocked site please.")
+
+ // The failure speaks update, not create: the member's bot still exists and
+ // still runs — the copy must say so, and the breadcrumb must say "update".
+ waitFor(t, "the update-failure explanation", func() bool {
+ for _, rp := range botReplies(t, s, threadID, threadKey, "Eleven") {
+ if strings.Contains(rp.Text, "Your bot is unchanged and still running as before.") &&
+ strings.Contains(rp.Text, "I couldn't make that change") {
+ return true
+ }
+ }
+ return false
+ })
+ var sawUpdateCrumb bool
+ for _, rp := range botReplies(t, s, threadID, threadKey, "Eleven") {
+ if strings.Contains(rp.Text, "Couldn't update Home News") {
+ sawUpdateCrumb = true
+ }
+ if strings.Contains(rp.Text, "Nothing was created") || strings.Contains(rp.Text, "Couldn't set up") {
+ t.Fatalf("update failure used create-path copy: %q", rp.Text)
+ }
+ }
+ if !sawUpdateCrumb {
+ t.Fatal("breadcrumb should read 'Couldn't update …'")
+ }
+
+ // The stored charter is byte-for-byte the seeded one: same single row,
+ // original prompt, no schedule/source change.
+ bots, _ := s.db.listMemberBots()
+ if len(bots) != 1 {
+ t.Fatalf("want the one original charter, got %d", len(bots))
+ }
+ var c struct {
+ Task struct {
+ Prompt string `json:"prompt"`
+ Fetch *struct {
+ Allow []string `json:"allow"`
+ } `json:"fetch"`
+ } `json:"task"`
+ }
+ if err := e2ee.DecryptJSON(instKey, bots[0].Blob, &c); err != nil {
+ t.Fatal(err)
+ }
+ if c.Task.Prompt != "Write the household news." || c.Task.Fetch != nil {
+ t.Fatalf("charter changed by a failed update: %+v", c.Task)
+ }
+}
diff --git a/changelog/2026-07-13-bot-update-fixes.md b/changelog/2026-07-13-bot-update-fixes.md
new file mode 100644
index 0000000..1b6f581
--- /dev/null
+++ b/changelog/2026-07-13-bot-update-fixes.md
@@ -0,0 +1,5 @@
+🤖 Changing a bot is more dependable. Switching a bot to a site that redirects
+to its www address works now (that broke "switch to the Irish Times"); a
+change that fails its test run says clearly that your bot is unchanged and
+still running — not "nothing was created"; and bots recover automatically
+from a rare case where an edition came out blank.
diff --git a/internal/bot/botbuild.go b/internal/bot/botbuild.go
index c31ffba..0d2cf0e 100644
--- a/internal/bot/botbuild.go
+++ b/internal/bot/botbuild.go
@@ -268,11 +268,19 @@ func (r *Runtime) runBotBuild(tc *ThreadCtx, req botBuildReq, requester, request
status.set(breadcrumb)
r.postReply(tc.ID, tc.key, Reply{Text: message, Actions: actions})
}
+ // Failure copy speaks the member's situation: a failed CREATE made
+ // nothing; a failed UPDATE changed nothing — the bot they have keeps
+ // running exactly as it was, and saying "nothing was created" there reads
+ // like the bot is gone. The vars flip once the update path loads the
+ // existing charter (fail is a closure, so later calls see the flip).
+ doing := "set up"
+ cantIntro := "I couldn't set that bot up — "
+ nothing := "Nothing was created."
fail := func(format string, args ...any) {
msg := fmt.Sprintf(format, args...)
r.logf("bot build %q: %s", req.Name, msg)
- settle(fmt.Sprintf("❌ Couldn't set up %s %s.", req.Title, req.Glyph),
- "I couldn't set that bot up — "+msg+" Feel free to ask me to try again.")
+ settle(fmt.Sprintf("❌ Couldn't %s %s %s.", doing, req.Title, req.Glyph),
+ cantIntro+msg+" Feel free to ask me to try again.")
}
if requesterUID == "" {
@@ -315,6 +323,9 @@ func (r *Runtime) runBotBuild(tc *ThreadCtx, req botBuildReq, requester, request
default:
existing = c
req.Name = c.Name
+ doing = "update"
+ cantIntro = "I couldn't make that change — "
+ nothing = "Your bot is unchanged and still running as before."
}
}
if req.BotID == "" {
@@ -350,20 +361,22 @@ func (r *Runtime) runBotBuild(tc *ThreadCtx, req botBuildReq, requester, request
}
// THE DRY RUN IS THE GATE: run the task once, now. No result → no bot.
+ // sourcesDown is checked FIRST: an all-sources-failed run that also came
+ // back textless should name the real problem, not "empty".
phase <- "running a first edition as a test"
sample, err := r.execTask(charter.Title, &charter.Task)
if err != nil {
- fail(fmt.Sprintf("the test run failed (%v). Nothing was created.", err))
- return
- }
- if strings.TrimSpace(sample.text) == "" {
- fail("the test run came back empty — the sources may not be readable. Nothing was created.")
+ fail(fmt.Sprintf("the test run failed (%v). %s", err, nothing))
return
}
if sample.sourcesDown() {
// The model reached NONE of the sources: the "edition" is an apology,
// not proof. Same gate as empty — never mint a bot that's born failing.
- fail("the test run couldn't read any of its sources — they may be down or blocking readers right now. Nothing was created.")
+ fail("the test run couldn't read any of its sources — they may be down or blocking readers right now. " + nothing)
+ return
+ }
+ if strings.TrimSpace(sample.text) == "" {
+ fail("the test run came back empty — the sources may not be readable. " + nothing)
return
}
diff --git a/internal/bot/chartersched.go b/internal/bot/chartersched.go
index 6cc5208..8d17870 100644
--- a/internal/bot/chartersched.go
+++ b/internal/bot/chartersched.go
@@ -248,12 +248,15 @@ func (r *Runtime) charterTick(selfID string, selfKey []byte, tp *tickPayload) {
case err != nil:
r.logf("charters: %s: %v", c.Name, err)
r.charterRunFailed(selfID, selfKey, tp.Bot, c, key, taskResult{})
- case res.text == "":
- r.logf("charters: %s: empty result — nothing posted", c.Name)
- r.charterRunFailed(selfID, selfKey, tp.Bot, c, key, taskResult{})
case res.sourcesDown():
+ // Before the empty check: an all-sources-failed run that also came
+ // back textless should log (and, on the first failure, explain) the
+ // real problem.
r.logf("charters: %s: reached none of its sources (%d fetches)", c.Name, res.fetches)
r.charterRunFailed(selfID, selfKey, tp.Bot, c, key, res)
+ case res.text == "":
+ r.logf("charters: %s: empty result — nothing posted", c.Name)
+ r.charterRunFailed(selfID, selfKey, tp.Bot, c, key, taskResult{})
default:
r.clearCharterFails(tp.Bot)
r.postTaskResult(c.Thread, key, c.Title, &c.Task, res.text, res.card)
diff --git a/internal/bot/fetch.go b/internal/bot/fetch.go
index f88b485..1c6d243 100644
--- a/internal/bot/fetch.go
+++ b/internal/bot/fetch.go
@@ -34,7 +34,10 @@ func allowPrivateFetch() bool { return os.Getenv("LCHAT_BOT_ALLOW_PRIVATE_FETCH"
// checkFetchURL enforces scheme + allowlist. Allow entries are URL prefixes
// ("https://www.rte.ie/feeds/"); a redirect target is re-checked with the same
-// rule, so a fetch can never wander off the list.
+// rule, so a fetch can never wander off the list. The apex↔www dance counts
+// as the same site (feed discovery already treats it so): a homepage-fallback
+// entry like "https://irishtimes.com" must survive the site's own 301 to
+// www.irishtimes.com, or every fetch dies on its first redirect.
func checkFetchURL(raw string, allow []string) error {
u, err := url.Parse(raw)
if err != nil {
@@ -47,14 +50,53 @@ func checkFetchURL(raw string, allow []string) error {
if u.Scheme != "https" && u.Scheme != scheme {
return fmt.Errorf("only https urls may be fetched")
}
- for _, a := range allow {
- if strings.HasPrefix(raw, a) {
- return nil
+ for _, candidate := range []string{raw, toggleWWW(u)} {
+ if candidate == "" {
+ continue
+ }
+ for _, a := range allow {
+ if allowPrefixMatch(candidate, a) {
+ return nil
+ }
}
}
return fmt.Errorf("url not in this bot's allowlist")
}
+// allowPrefixMatch reports whether a URL falls under one allow entry. Not a
+// bare HasPrefix: a bare-host entry ("https://irishtimes.com") must not match
+// a LONGER host ("https://irishtimes.com.evil.com" — and the MODEL picks
+// fetch URLs, so the boundary is load-bearing). Unless the entry already ends
+// inside its path or query, the match must stop at a URL boundary character.
+func allowPrefixMatch(raw, entry string) bool {
+ if !strings.HasPrefix(raw, entry) {
+ return false
+ }
+ if len(raw) == len(entry) || strings.HasSuffix(entry, "/") || strings.Contains(entry, "?") {
+ return true
+ }
+ switch raw[len(entry)] {
+ case '/', '?', '#':
+ return true
+ }
+ return false
+}
+
+// toggleWWW is u with its host's leading "www." flipped (added or removed) —
+// the same-site variant the allowlist tolerates. "" when there's no host.
+func toggleWWW(u *url.URL) string {
+ if u.Host == "" {
+ return ""
+ }
+ v := *u
+ if strings.HasPrefix(v.Host, "www.") {
+ v.Host = strings.TrimPrefix(v.Host, "www.")
+ } else {
+ v.Host = "www." + v.Host
+ }
+ return v.String()
+}
+
// privateIP reports whether an address must not be dialled: loopback,
// link-local, RFC1918/ULA — the standard SSRF surface. Checked in the dialer's
// Control hook, i.e. against the address actually being connected to (after
diff --git a/internal/bot/fetch_test.go b/internal/bot/fetch_test.go
index 30f6849..ce858ed 100644
--- a/internal/bot/fetch_test.go
+++ b/internal/bot/fetch_test.go
@@ -25,6 +25,59 @@ func TestCheckFetchURL(t *testing.T) {
}
}
+// TestCheckFetchURLWWW pins the apex↔www tolerance: a homepage-fallback allow
+// entry survives the site's own redirect to (or from) www — the live failure
+// that broke "switch to the Irish Times" (irishtimes.com 301s to www).
+func TestCheckFetchURLWWW(t *testing.T) {
+ cases := []struct {
+ raw string
+ allow string
+ ok bool
+ }{
+ {"https://www.irishtimes.com/", "https://irishtimes.com", true},
+ {"https://irishtimes.com/news", "https://www.irishtimes.com", true},
+ {"https://www.irishtimes.com/news?page=2", "https://irishtimes.com", true},
+ // www tolerance is not subdomain tolerance.
+ {"https://api.irishtimes.com/", "https://irishtimes.com", false},
+ {"https://wwwx.irishtimes.com/", "https://irishtimes.com", false},
+ }
+ for _, c := range cases {
+ err := checkFetchURL(c.raw, []string{c.allow})
+ if (err == nil) != c.ok {
+ t.Errorf("checkFetchURL(%q, allow %q) = %v, want ok=%v", c.raw, c.allow, err, c.ok)
+ }
+ }
+}
+
+// TestAllowPrefixMatchBoundary pins the boundary rule: a bare-host entry must
+// not match a LONGER host that merely starts with the same bytes — the model
+// picks fetch URLs, so this edge is load-bearing.
+func TestAllowPrefixMatchBoundary(t *testing.T) {
+ cases := []struct {
+ raw string
+ entry string
+ ok bool
+ }{
+ {"https://irishtimes.com", "https://irishtimes.com", true},
+ {"https://irishtimes.com/", "https://irishtimes.com", true},
+ {"https://irishtimes.com/news", "https://irishtimes.com", true},
+ {"https://irishtimes.com?utm=x", "https://irishtimes.com", true},
+ {"https://irishtimes.com.evil.com/", "https://irishtimes.com", false},
+ {"https://irishtimes.community/", "https://irishtimes.com", false},
+ {"https://irishtimes.com:8443/", "https://irishtimes.com", false},
+ // Entries ending in a slash or already inside a query keep plain
+ // prefix semantics.
+ {"https://www.rte.ie/feeds/rss/news.xml", "https://www.rte.ie/feeds/", true},
+ {"https://a.com/feed?index=/news/&limit=100", "https://a.com/feed?index=/news/", true},
+ {"https://a.com/feedsecret", "https://a.com/feeds", false},
+ }
+ for _, c := range cases {
+ if got := allowPrefixMatch(c.raw, c.entry); got != c.ok {
+ t.Errorf("allowPrefixMatch(%q, %q) = %v, want %v", c.raw, c.entry, got, c.ok)
+ }
+ }
+}
+
// TestFetchBlocksPrivateAddresses pins the SSRF guard: without the test-only
// escape hatch, a loopback URL — even one on the allowlist — must not connect.
func TestFetchBlocksPrivateAddresses(t *testing.T) {
diff --git a/internal/bot/task.go b/internal/bot/task.go
index 98bd56c..a7707be 100644
--- a/internal/bot/task.go
+++ b/internal/bot/task.go
@@ -154,17 +154,35 @@ func (res *taskResult) sourcesDown() bool { return res.fetches > 0 && res.fetchO
// scheduler) and the builder's dry run (botbuild.go), which captures the
// output as the gate + first post instead of a separate delivery.
func (r *Runtime) execTask(persona string, t *TaskSpec) (taskResult, error) {
- var res taskResult
+ res, stop, err := r.execTaskOnce(persona, t, true)
+ if err != nil || res.text != "" {
+ return res, err
+ }
+ // An empty final is nearly always adaptive thinking having spent the
+ // whole token budget before any text was written (stop_reason
+ // "max_tokens" — seen live on a long feed digest). A thinking-free
+ // retry has the full budget for the answer: the same trade modelReply's
+ // forced-chip retry makes.
+ r.logf("task: empty result (stop_reason=%s) — retrying without thinking", stop)
+ res, _, err = r.execTaskOnce(persona, t, false)
+ return res, err
+}
+
+func (r *Runtime) execTaskOnce(persona string, t *TaskSpec, think bool) (res taskResult, stop string, err error) {
if r.ai == nil {
// Fail closed but not silent: a scheduled task that can never run is a
// misconfiguration the owner should see once per firing, in the log.
- return res, fmt.Errorf("no model available (ANTHROPIC_API_KEY unset)")
+ return res, "", fmt.Errorf("no model available (ANTHROPIC_API_KEY unset)")
}
model := firstOf(t.Model, r.model, defaultModel)
maxCalls := t.MaxToolCalls
if maxCalls == 0 {
maxCalls = defaultMaxToolCalls
}
+ var thinking map[string]any
+ if think {
+ thinking = adaptiveThinking(model)
+ }
system := []anthBlock{{Type: "text", Text: r.taskSystemPrompt(persona, t), CacheControl: &anthCache{Type: "ephemeral"}}}
messages := []anthMessage{{
@@ -178,13 +196,14 @@ func (r *Runtime) execTask(persona string, t *TaskSpec) (taskResult, error) {
Model: model,
MaxTokens: 4096,
System: system,
- Thinking: adaptiveThinking(model),
+ Thinking: thinking,
Tools: r.taskTools(t),
Messages: messages,
})
if err != nil {
- return res, fmt.Errorf("model: %w", err)
+ return res, stop, fmt.Errorf("model: %w", err)
}
+ stop = resp.StopReason
uses := toolUses(resp.Content)
if len(uses) == 0 {
res.text = strings.TrimSpace(resp.text())
@@ -206,7 +225,7 @@ func (r *Runtime) execTask(persona string, t *TaskSpec) (taskResult, error) {
}
messages = append(messages, anthMessage{Role: "user", Content: results})
}
- return res, nil
+ return res, stop, nil
}
// runTaskTool executes one tool_use block. Errors become the result text —
diff --git a/internal/bot/task_test.go b/internal/bot/task_test.go
new file mode 100644
index 0000000..90a52a2
--- /dev/null
+++ b/internal/bot/task_test.go
@@ -0,0 +1,59 @@
+package bot
+
+import (
+ "encoding/json"
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "sync"
+ "testing"
+)
+
+// TestExecTaskRetriesWithoutThinking pins the empty-result fallback: adaptive
+// thinking can spend the whole 4096-token budget before any text is written
+// (stop_reason max_tokens — seen live on a long feed digest), and the run
+// must retry once with thinking off instead of reporting an empty edition.
+func TestExecTaskRetriesWithoutThinking(t *testing.T) {
+ var mu sync.Mutex
+ var bodies []string
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ b, _ := io.ReadAll(r.Body)
+ mu.Lock()
+ bodies = append(bodies, string(b))
+ call := len(bodies)
+ mu.Unlock()
+ if call == 1 {
+ // The whole budget went on thinking: no text, truncated.
+ json.NewEncoder(w).Encode(map[string]any{
+ "content": []map[string]any{{"type": "thinking", "thinking": "hmm…", "signature": "sig"}},
+ "stop_reason": "max_tokens",
+ })
+ return
+ }
+ json.NewEncoder(w).Encode(map[string]any{
+ "content": []map[string]any{{"type": "text", "text": "the edition"}},
+ "stop_reason": "end_turn",
+ })
+ }))
+ defer srv.Close()
+ t.Setenv("ANTHROPIC_BASE_URL", srv.URL)
+
+ r := &Runtime{Session: &Session{Name: "Eleven"}, ai: newAnthropicClient("test-key"), Logf: t.Logf}
+ res, err := r.execTask("Test Bot", &TaskSpec{Prompt: "write the edition"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if res.text != "the edition" {
+ t.Fatalf("text = %q, want the retry's edition", res.text)
+ }
+ if len(bodies) != 2 {
+ t.Fatalf("want 2 model calls (attempt + thinking-free retry), got %d", len(bodies))
+ }
+ if !strings.Contains(bodies[0], `"thinking":{"type":"adaptive"}`) {
+ t.Error("first attempt should run with adaptive thinking")
+ }
+ if strings.Contains(bodies[1], `"thinking":{"type":"adaptive"}`) {
+ t.Error("the retry must run with thinking off")
+ }
+}