Batch 16: Shards, Slack output and shards panel

familiar-gateway/internal/adapter/shardapi/shardapi.go

@@ -14,8 +14,11 @@
 //   - ignore model / tools / temperature / max_tokens / user in the
 //     body — the shard is the capability envelope, the caller cannot
 //     expand it;
-//   - never run the router, the Familiar tiered prompt store, the
-//     preamble path, or pre-execution tool orchestration;
+//   - never run the Familiar tiered prompt store, the preamble path,
+//     or pre-execution tool orchestration; a shard pinned to a model
+//     or tier skips the classifier too, while an unpinned one is
+//     classified like a trusted turn (its web_search is granted by its
+//     allowlist either way, not by the classifier);
 //   - skip memory retrieval entirely (shards see the shard prompt +
 //     prior session turns, nothing else);
 //   - tag all downstream writes (pipeline commit, extracted facts,
@@ -34,7 +37,6 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
-	"fmt"
 	"io"
 	"log"
 	"net/http"
@@ -566,11 +568,18 @@ func (h *Handler) handleStreaming(
 	// the final JSON; routing metadata is out of band (logs).
 	_, info, err := h.pipe.HandleShardStream(ctx, sess, userMsg, overrides, onChunk, nil, nil)
 	if err != nil {
+		// Same as the non-streaming path: the detail goes to the log,
+		// the caller gets a generic error, as an error event (the
+		// OpenAI shape) and no "stop" finish. The error text was sent as
+		// reply content (upstream URLs, model ids) and the stream then
+		// finished "stop", reading as a successful completion.
 		log.Printf("[shards] invoke failed: shard=%s err=%v", shardID, err)
-		send(choice{
-			Index: 0,
-			Delta: &invokeMessage{Content: fmt.Sprintf("\n\n[error: %v]", err)},
-		})
+		mu.Lock()
+		_, _ = io.WriteString(w, `data: {"error":{"message":"invocation failed","type":"server_error"}}`+"\n\n")
+		_, _ = io.WriteString(w, "data: [DONE]\n\n")
+		flusher.Flush()
+		mu.Unlock()
+		return
 	}
 
 	finish := "stop"

familiar-gateway/internal/adapter/shardapi/shardapi_test.go

@@ -688,3 +688,26 @@ func TestInvoke_PipelineErrorReturns500(t *testing.T) {
 // grows or shrinks; no-op otherwise.
 var _ = fmt.Sprintf
 var _ = io.Discard
+
+// A failed streamed invocation ends with an error event, not the
+// internal error text as reply content followed by a "stop" finish
+// (which reads as a successful completion and leaked upstream URLs).
+func TestInvoke_StreamingErrorIsAnErrorEvent(t *testing.T) {
+	_, _, pipe, h := buildFixtures(t)
+	pipe.err = errors.New(`LLM completion: Post "http://127.0.0.1:8081/v1/chat/completions": connection refused`)
+	body := `{"messages":[{"role":"user","content":"hi"}],"stream":true}`
+	rr := doInvoke(t, h, testShard, testEmail, testToken, body)
+	out := rr.Body.String()
+	if strings.Contains(out, "127.0.0.1") || strings.Contains(out, "connection refused") {
+		t.Errorf("internal error text reached the caller: %q", out)
+	}
+	if !strings.Contains(out, `"error":{"message":"invocation failed"`) {
+		t.Errorf("no error event: %q", out)
+	}
+	if strings.Contains(out, `"finish_reason":"stop"`) {
+		t.Errorf("a failed stream finished as stop: %q", out)
+	}
+	if !strings.HasSuffix(strings.TrimSpace(out), "data: [DONE]") {
+		t.Errorf("stream not terminated: %q", out)
+	}
+}

familiar-gateway/internal/adapter/slack/format.go

@@ -2,6 +2,7 @@ package slack
 
 import (
 	"regexp"
+	"strconv"
 	"strings"
 )
 
@@ -14,9 +15,13 @@ import (
 //   - --- / *** / ___ rule → a unicode divider line
 //   - "- item" / "+ item" → "•  item"
 //   - **bold** → *bold*
-//   - [text](url) → <url|text>
+//   - [text](url) and <https://…> autolinks → <url|text> / <url>
 //   - <html> tags → stripped
-//   - backtick `code` and ``` ```code``` fences pass through untouched
+//   - backtick `code` and ``` ```code``` fences pass through, escaped
+//   - &, < and > are escaped everywhere, so text can't form a Slack
+//     control sequence (<!channel>, <@U…>): content a tool fetched
+//     could make the bot ping a whole channel. Only the links built
+//     here keep their <…>.
 //
 // Code spans and fenced blocks are carved out first so the bold/link
 // transformations don't accidentally rewrite content the user wanted
@@ -35,14 +40,21 @@ func toMrkdwn(s string) string {
 	b.Grow(len(s))
 	for _, seg := range segments {
 		if seg.code {
-			b.WriteString(seg.text)
+			b.WriteString(slackEscape(seg.text))
 			continue
 		}
 		b.WriteString(transformPlain(seg.text))
 	}
 	return b.String()
 }
 
+// slackEscape escapes the three characters Slack gives meaning to.
+func slackEscape(s string) string {
+	return slackEscaper.Replace(s)
+}
+
+var slackEscaper = strings.NewReplacer("&", "&amp;", "<", "&lt;", ">", "&gt;")
+
 type codeSeg struct {
 	text string
 	code bool // true = do not transform
@@ -117,20 +129,44 @@ var linkPattern = regexp.MustCompile(`\[([^\]]+)\]\(([^)\s]+)\)`)
 // htmlTagPattern matches simple HTML tags for stripping.
 var htmlTagPattern = regexp.MustCompile(`<(/?[a-zA-Z][a-zA-Z0-9]*)\b[^>]*>`)
 
+// autolinkPattern matches a CommonMark autolink, <scheme://…> or
+// <mailto:…>. It has to go before the tag strip, which took
+// "<https://x>" for a tag named https and deleted the URL.
+var autolinkPattern = regexp.MustCompile(`<((?:https?|mailto):[^\s<>]+)>`)
+
 func transformPlain(s string) string {
 	// Order matters:
-	//  1. strip HTML before link rewriting so generated <url|text>
-	//     isn't re-stripped as a tag;
-	//  2. headings/rules/bullets are line-anchored and run before the
+	//  1. links (markdown and autolinks) become placeholders first, so
+	//     the tag strip and the escaping leave them alone; they're
+	//     restored as Slack links at the end, their text escaped;
+	//  2. strip HTML, then escape what's left of &, < and >;
+	//  3. headings/rules/bullets are line-anchored and run before the
 	//     inline bold pass — heading text may itself contain **bold**,
 	//     which the bold pass then handles;
-	//  3. the hr pattern (---) must run before bullets so a "---" line
+	//  4. the hr pattern (---) must run before bullets so a "---" line
 	//     isn't seen as a "-" bullet.
+	var links []string
+	hold := func(link string) string {
+		links = append(links, link)
+		return "\x00" + strconv.Itoa(len(links)-1) + "\x00"
+	}
+	s = linkPattern.ReplaceAllStringFunc(s, func(m string) string {
+		sub := linkPattern.FindStringSubmatch(m)
+		return hold("<" + slackEscape(sub[2]) + "|" + slackEscape(sub[1]) + ">")
+	})
+	s = autolinkPattern.ReplaceAllStringFunc(s, func(m string) string {
+		return hold("<" + slackEscape(autolinkPattern.FindStringSubmatch(m)[1]) + ">")
+	})
 	s = htmlTagPattern.ReplaceAllString(s, "")
+	s = slackEscape(s)
 	s = hrPattern.ReplaceAllString(s, mrkdwnRule)
 	s = headingPattern.ReplaceAllString(s, "*$1*")
 	s = bulletPattern.ReplaceAllString(s, "$1•  ")
 	s = boldPattern.ReplaceAllString(s, "*$1*")
-	s = linkPattern.ReplaceAllString(s, "<$2|$1>")
-	return s
+	return placeholderPattern.ReplaceAllStringFunc(s, func(m string) string {
+		i, _ := strconv.Atoi(strings.Trim(m, "\x00"))
+		return links[i]
+	})
 }
+
+var placeholderPattern = regexp.MustCompile("\x00[0-9]+\x00")

familiar-gateway/internal/adapter/slack/safety_test.go

@@ -0,0 +1,94 @@
+package slack
+
+import (
+	"context"
+	"strings"
+	"testing"
+	"unicode/utf8"
+)
+
+// Text can't form Slack control sequences: a fetched page telling the
+// model to print "<!channel>" made the bot ping the channel. Autolinks
+// survive (the tag strip deleted them); the links the converter builds
+// keep their markup.
+func TestToMrkdwn_EscapesAndKeepsLinks(t *testing.T) {
+	cases := []struct{ in, want string }{
+		{"See <https://example.com/docs> for details", "See <https://example.com/docs> for details"},
+		{"Hey <!channel> and <@U12345>", "Hey &lt;!channel&gt; and &lt;@U12345&gt;"},
+		{"Tom & Jerry <b>bold</b>", "Tom &amp; Jerry bold"},
+		{"[the docs](https://x.test/a?b=1&c=2)", "<https://x.test/a?b=1&amp;c=2|the docs>"},
+		{"code: `<!here>`", "code: `&lt;!here&gt;`"},
+		{"**bold** <mailto:a@b.test>", "*bold* <mailto:a@b.test>"},
+	}
+	for _, c := range cases {
+		if got := toMrkdwn(c.in); got != c.want {
+			t.Errorf("toMrkdwn(%q) = %q, want %q", c.in, got, c.want)
+		}
+	}
+}
+
+// Splitting never halves a character or a link, prefers spaces, and
+// keeps a code fence rendering as code on both sides.
+func TestSplitMessage_RunesLinksAndFences(t *testing.T) {
+	// No newline or space: the cut lands on a rune boundary.
+	s := strings.Repeat("a", 9) + "é" + strings.Repeat("b", 10)
+	for _, c := range splitMessage(s, 10) {
+		if !utf8.ValidString(c) {
+			t.Errorf("chunk %q is not valid UTF-8", c)
+		}
+	}
+	// A space before the limit wins over a raw cut.
+	chunks := splitMessage("hello world again", 13)
+	if chunks[0] != "hello world " {
+		t.Errorf("chunk[0] = %q, want a cut at the space", chunks[0])
+	}
+	// Not inside a link (one that fits in a chunk).
+	chunks = splitMessage("aaaaaaaaaa<https://x.test|t>tail", 20)
+	for _, c := range chunks {
+		if strings.Count(c, "<") != strings.Count(c, ">") {
+			t.Errorf("a link was cut: %q", chunks)
+		}
+	}
+	// A fence cut in two is closed and reopened.
+	code := "```\n" + strings.Repeat("line of code\n", 20) + "```"
+	chunks = splitMessage(code, 100)
+	if len(chunks) < 2 {
+		t.Fatalf("expected several chunks, got %d", len(chunks))
+	}
+	for _, c := range chunks {
+		if strings.Count(c, "```")%2 != 0 {
+			t.Errorf("chunk leaves a fence open: %q", c)
+		}
+		if len(c) > 100 {
+			t.Errorf("chunk of %d bytes, over the limit", len(c))
+		}
+	}
+}
+
+// A long proactive post goes as several posts: Slack truncates text
+// past 40,000 characters, and nothing split it.
+func TestSendProactive_SplitsLongPosts(t *testing.T) {
+	api := &fakeSlackAPI{membersPage: [][]string{{}}}
+	srv := api.server(t)
+	s, err := NewSender("xoxb-test", srv.URL+"/api/")
+	if err != nil {
+		t.Fatal(err)
+	}
+	long := strings.Repeat("A research digest paragraph.\n", 3000) // ~87KB
+	if err := s.SendProactive(context.Background(), "C_TEAM", long); err != nil {
+		t.Fatal(err)
+	}
+	if len(api.texts) < 3 {
+		t.Fatalf("posted %d messages, want the digest split", len(api.texts))
+	}
+	total := 0
+	for _, txt := range api.texts {
+		if len(txt) > proactiveMaxLen {
+			t.Errorf("a post of %d bytes", len(txt))
+		}
+		total += len(txt)
+	}
+	if total < len(long)-10 {
+		t.Errorf("posted %d of %d bytes", total, len(long))
+	}
+}

familiar-gateway/internal/adapter/slack/sender.go

@@ -62,10 +62,11 @@ func (s *Sender) SendDM(ctx context.Context, slackUserID, text string) error {
 }
 
 // SendProactive posts `text` to `channelID`. This is the method the
-// scheduled-actions Slack deliverer calls. Long messages are sent as
-// a single post — the slack-go client splits at the API level if
-// needed. We do not use the 4k truncation the adapter applies to
-// interactive replies because proactive briefings are commonly long.
+// scheduled-actions Slack deliverer calls. Proactive briefings are
+// commonly long: past proactiveMaxLen they go as several posts (split
+// like interactive replies, at a larger size). They were sent as one
+// post on the belief that slack-go splits long messages; it doesn't,
+// and Slack truncates text past 40,000 characters without an error.
 func (s *Sender) SendProactive(ctx context.Context, channelID, text string) error {
 	if channelID == "" {
 		return fmt.Errorf("slack sender: empty channel_id")
@@ -78,16 +79,22 @@ func (s *Sender) SendProactive(ctx context.Context, channelID, text string) erro
 	// / "**" / "---" shows literally. Convert before posting — the
 	// interactive adapter already does this on its own path.
 	text = toMrkdwn(text)
-	_, _, err := s.api.PostMessageContext(ctx, channelID,
-		slack.MsgOptionText(text, false),
-		slack.MsgOptionDisableLinkUnfurl(),
-	)
-	if err != nil {
-		return fmt.Errorf("slack post to %s: %w", channelID, err)
+	for _, chunk := range splitMessage(text, proactiveMaxLen) {
+		_, _, err := s.api.PostMessageContext(ctx, channelID,
+			slack.MsgOptionText(chunk, false),
+			slack.MsgOptionDisableLinkUnfurl(),
+		)
+		if err != nil {
+			return fmt.Errorf("slack post to %s: %w", channelID, err)
+		}
 	}
 	return nil
 }
 
+// proactiveMaxLen is the largest proactive post, in bytes: under
+// Slack's 40,000-character limit whatever the characters.
+const proactiveMaxLen = 35000
+
 // IsUserOrDMID reports a Slack id that names a person (U…, W…) or a DM
 // (D…) rather than a channel. chat.postMessage accepts a user id as
 // the channel and DMs that user from the bot.

familiar-gateway/internal/adapter/slack/sender_policy_test.go

@@ -14,6 +14,7 @@ import (
 type fakeSlackAPI struct {
 	mu          sync.Mutex
 	posted      []string
+	texts       []string
 	membersErr  string // e.g. "missing_scope"
 	membersPage [][]string
 }
@@ -44,6 +45,7 @@ func (f *fakeSlackAPI) server(t *testing.T) *httptest.Server {
 		_ = r.ParseForm()
 		f.mu.Lock()
 		f.posted = append(f.posted, r.Form.Get("channel"))
+		f.texts = append(f.texts, r.Form.Get("text"))
 		f.mu.Unlock()
 		_ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "channel": r.Form.Get("channel"), "ts": "1.2"})
 	})