diff --git a/assets/wechat.png b/assets/wechat.png index 32998c122..cc88186a8 100644 Binary files a/assets/wechat.png and b/assets/wechat.png differ diff --git a/cmd/picoclaw/internal/gateway/helpers.go b/cmd/picoclaw/internal/gateway/helpers.go index 174f5db62..00b53e62c 100644 --- a/cmd/picoclaw/internal/gateway/helpers.go +++ b/cmd/picoclaw/internal/gateway/helpers.go @@ -16,6 +16,7 @@ import ( _ "github.com/sipeed/picoclaw/pkg/channels/dingtalk" _ "github.com/sipeed/picoclaw/pkg/channels/discord" _ "github.com/sipeed/picoclaw/pkg/channels/feishu" + _ "github.com/sipeed/picoclaw/pkg/channels/irc" _ "github.com/sipeed/picoclaw/pkg/channels/line" _ "github.com/sipeed/picoclaw/pkg/channels/maixcam" _ "github.com/sipeed/picoclaw/pkg/channels/onebot" diff --git a/config/config.example.json b/config/config.example.json index bb4bdddca..47372d70b 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -183,6 +183,28 @@ "ping_interval": 30, "read_timeout": 60, "allow_from": [] + }, + "irc": { + "enabled": false, + "server": "irc.libera.chat:6697", + "tls": true, + "nick": "mybot", + "user": "", + "real_name": "", + "password": "", + "nickserv_password": "", + "sasl_user": "", + "sasl_password": "", + "channels": ["#mychannel"], + "request_caps": ["server-time", "message-tags"], + "allow_from": [], + "group_trigger": { + "mention_only": true + }, + "typing": { + "enabled": false + }, + "reasoning_channel_id": "" } }, "providers": { diff --git a/docs/agent-refactor/README.md b/docs/agent-refactor/README.md new file mode 100644 index 000000000..db8575fc9 --- /dev/null +++ b/docs/agent-refactor/README.md @@ -0,0 +1,145 @@ +# Agent Refactor + +## What this directory is for + +This directory is the working area for the current Agent refactor. + +The purpose of this refactor is simple: + +the project needs a smaller, clearer, and more stable Agent model before more Agent-related behavior is added. + +The codebase already contains meaningful Agent behavior. What it still lacks is a sufficiently explicit and stable semantic boundary around that behavior. + +This refactor exists to fix that first. + +--- + +## Refactor stance + +This is a maintenance-led consolidation effort. + +It is not a general invitation to expand Agent behavior in parallel. + +During this refactor window, Agent-related work should converge on the current refactor track instead of branching into new semantics. + +That means: + +- concept clarification before feature expansion +- boundary tightening before abstraction growth +- semantic consolidation before new behavior + +--- + +## Core rule: minimum concepts only + +This refactor follows one hard rule: + +**do not introduce a new concept unless it is strictly necessary** + +More explicitly: + +- if an existing concept can be clarified, reuse it +- if an existing boundary can be made explicit, do that first +- if a behavior can be expressed without a new abstraction, do not add one +- "future flexibility" is not enough justification on its own + +The goal of this refactor is not to grow the model. + +The goal is to reduce ambiguity. + +--- + +## What is being clarified + +This refactor is currently concerned with the following questions: + +1. what an `Agent` is +2. what an `AgentLoop` is +3. what the lifecycle of `AgentLoop` is +4. what the event surface around `AgentLoop` is +5. how persona / identity is assembled +6. how capabilities are represented +7. how context boundaries and compression work +8. how subagent coordination works + +These are the current working boundaries. + +If they need to be adjusted, they should be adjusted explicitly rather than drift implicitly in code. + +--- + +## Status of this directory + +The documents here are working materials. + +They are not final or immutable. + +If current notes are incomplete, incorrectly split, or too broad, they should be revised. This directory should evolve with the refactor rather than pretending the first draft is complete. + +--- + +## Suggested document split + +This directory may eventually contain notes such as: + +- `agent-overview.md` + - what an Agent is +- `agent-loop.md` + - AgentLoop contract, lifecycle, event surface +- `persona.md` + - persona and identity assembly +- `capability.md` + - tools / skills / MCP capability semantics +- `context.md` + - context scope, history, summary, compression +- `subagent.md` + - subagent coordination rules + +These files should be added only when they help clarify the current refactor work. + +This directory should not turn into a generic architecture dump. + +--- + +## What this directory is not for + +This directory is not intended for: + +- broad speculative architecture +- future multi-node protocol design not required by the current refactor +- parallel feature planning unrelated to Agent consolidation +- adding new concepts before current ones are made clear + +If a topic does not directly help reduce ambiguity in the current Agent model, it probably does not belong here yet. + +--- + +## Relationship to implementation + +Implementation changes should not keep redefining Agent semantics implicitly. + +If a PR changes or depends on Agent semantics, those semantics should either already exist here or be clarified in a linked issue first. + +This directory is here to make implementation narrower and more disciplined. + +--- + +## Relationship to GitHub tracking + +The umbrella issue for this refactor should point here. + +The issue is the coordination surface. + +This directory is the repository-local working surface. + +--- + +## Summary + +The main question of this refactor is not: + +- what more can Agent do + +The main question is: + +- what is the smallest stable model that current Agent behavior can be organized around diff --git a/go.mod b/go.mod index 6fa3a900c..2bd5ddef9 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/h2non/filetype v1.1.3 github.com/larksuite/oapi-sdk-go/v3 v3.5.3 github.com/mdp/qrterminal/v3 v3.2.1 - github.com/modelcontextprotocol/go-sdk v1.3.0 + github.com/modelcontextprotocol/go-sdk v1.3.1 github.com/mymmrac/telego v1.6.0 github.com/open-dingtalk/dingtalk-stream-sdk-go v0.9.1 github.com/openai/openai-go/v3 v3.22.0 @@ -31,12 +31,13 @@ require ( ) require ( - filippo.io/edwards25519 v1.1.0 // indirect + filippo.io/edwards25519 v1.1.1 // indirect github.com/beeper/argo-go v1.1.2 // indirect github.com/coder/websocket v1.8.14 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/dustin/go-humanize v1.0.1 // indirect github.com/elliotchance/orderedmap/v3 v3.1.0 // indirect + github.com/ergochat/irc-go v0.5.0 // indirect github.com/gdamore/encoding v1.0.1 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/lucasb-eyer/go-colorful v1.3.0 // indirect @@ -48,6 +49,8 @@ require ( github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/rs/zerolog v1.34.0 // indirect + github.com/segmentio/asm v1.1.3 // indirect + github.com/segmentio/encoding v0.5.3 // indirect github.com/spf13/pflag v1.0.10 // indirect github.com/vektah/gqlparser/v2 v2.5.27 // indirect go.mau.fi/libsignal v0.2.1 // indirect diff --git a/go.sum b/go.sum index 060594d06..81a1cdd1e 100644 --- a/go.sum +++ b/go.sum @@ -1,6 +1,6 @@ cloud.google.com/go/compute/metadata v0.3.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k= -filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA= -filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4= +filippo.io/edwards25519 v1.1.1 h1:YpjwWWlNmGIDyXOn8zLzqiD+9TyIlPhGFG96P39uBpw= +filippo.io/edwards25519 v1.1.1/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4= github.com/DATA-DOG/go-sqlmock v1.5.2 h1:OcvFkGmslmlZibjAjaHm3L//6LiuBgolP7OputlJIzU= github.com/DATA-DOG/go-sqlmock v1.5.2/go.mod h1:88MAG/4G7SMwSE3CeA0ZKzrT5CiOU3OJ+JlNzwDqpNU= github.com/adhocore/gronx v1.19.6 h1:5KNVcoR9ACgL9HhEqCm5QXsab/gI4QDIybTAWcXDKDc= @@ -48,6 +48,8 @@ github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkp github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= github.com/elliotchance/orderedmap/v3 v3.1.0 h1:j4DJ5ObEmMBt/lcwIecKcoRxIQUEnw0L804lXYDt/pg= github.com/elliotchance/orderedmap/v3 v3.1.0/go.mod h1:G+Hc2RwaZvJMcS4JpGCOyViCnGeKf0bTYCGTO4uhjSo= +github.com/ergochat/irc-go v0.5.0 h1:woQ1RS9YbfgqPgSpPBBQeczXGIGzR0aC7dEgk469fTw= +github.com/ergochat/irc-go v0.5.0/go.mod h1:2vi7KNpIPWnReB5hmLpl92eMywQvuIeIIGdt/FQCph0= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= github.com/gdamore/encoding v1.0.1 h1:YzKZckdBL6jVt2Gc+5p82qhrGiqMdG/eNs6Wy0u3Uhw= @@ -134,8 +136,8 @@ github.com/mattn/go-sqlite3 v1.14.34 h1:3NtcvcUnFBPsuRcno8pUtupspG/GM+9nZ88zgJcp github.com/mattn/go-sqlite3 v1.14.34/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= github.com/mdp/qrterminal/v3 v3.2.1 h1:6+yQjiiOsSuXT5n9/m60E54vdgFsw0zhADHhHLrFet4= github.com/mdp/qrterminal/v3 v3.2.1/go.mod h1:jOTmXvnBsMy5xqLniO0R++Jmjs2sTm9dFSuQ5kpz/SU= -github.com/modelcontextprotocol/go-sdk v1.3.0 h1:gMfZkv3DzQF5q/DcQePo5rahEY+sguyPfXDfNBcT0Zs= -github.com/modelcontextprotocol/go-sdk v1.3.0/go.mod h1:AnQ//Qc6+4nIyyrB4cxBU7UW9VibK4iOZBeyP/rF1IE= +github.com/modelcontextprotocol/go-sdk v1.3.1 h1:TfqtNKOIWN4Z1oqmPAiWDC2Jq7K9OdJaooe0teoXASI= +github.com/modelcontextprotocol/go-sdk v1.3.1/go.mod h1:DgVX498dMD8UJlseK1S5i1T4tFz2fkBk4xogC3D15nw= github.com/mymmrac/telego v1.6.0 h1:Zc8rgyHozvd/7ZgyrigyHdAF9koHYMfilYfyB6wlFC0= github.com/mymmrac/telego v1.6.0/go.mod h1:xt6ZWA8zi8KmuzryE1ImEdl9JSwjHNpM4yhC7D8hU4Y= github.com/ncruces/go-strftime v1.0.0 h1:HMFp8mLCTPp341M/ZnA4qaf7ZlsbTc+miZjCLOFAw7w= @@ -171,6 +173,10 @@ github.com/rs/xid v1.6.0/go.mod h1:7XoLgs4eV+QndskICGsho+ADou8ySMSjJKDIan90Nz0= github.com/rs/zerolog v1.34.0 h1:k43nTLIwcTVQAncfCw4KZ2VY6ukYoZaBPNOE8txlOeY= github.com/rs/zerolog v1.34.0/go.mod h1:bJsvje4Z08ROH4Nhs5iH600c3IkWhwp44iRc54W6wYQ= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/segmentio/asm v1.1.3 h1:WM03sfUOENvvKexOLp+pCqgb/WDjsi7EK8gIsICtzhc= +github.com/segmentio/asm v1.1.3/go.mod h1:Ld3L4ZXGNcSLRg4JBsZ3//1+f/TjYl0Mzen/DQy1EJg= +github.com/segmentio/encoding v0.5.3 h1:OjMgICtcSFuNvQCdwqMCv9Tg7lEOXGwm1J5RPQccx6w= +github.com/segmentio/encoding v0.5.3/go.mod h1:HS1ZKa3kSN32ZHVZ7ZLPLXWvOVIiZtyJnO1gPH1sKt0= github.com/sergi/go-diff v1.3.1 h1:xkr+Oxo4BOQKmkn/B9eMK0g5Kg/983T9DqqPHwYqD+8= github.com/sergi/go-diff v1.3.1/go.mod h1:aMJSSKb2lpPvRNec0+w3fl7LP9IOFzdc9Pa4NFbPK1I= github.com/slack-go/slack v0.17.3 h1:zV5qO3Q+WJAQ/XwbGfNFrRMaJ5T/naqaonyPV/1TP4g= diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 19d13b2bb..9a54f5077 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -220,7 +220,7 @@ func registerSharedTools( // Spawn tool with allowlist checker if cfg.Tools.IsToolEnabled("spawn") { if cfg.Tools.IsToolEnabled("subagent") { - subagentManager := tools.NewSubagentManager(provider, agent.Model, agent.Workspace, msgBus) + subagentManager := tools.NewSubagentManager(provider, agent.Model, agent.Workspace) subagentManager.SetLLMOptions(agent.MaxTokens, agent.Temperature) spawnTool := tools.NewSpawnTool(subagentManager) currentAgentID := agentID @@ -726,9 +726,8 @@ func (al *AgentLoop) runAgentLoop( agent *AgentInstance, opts processOptions, ) (string, error) { - // 0. Record last channel for heartbeat notifications (skip internal channels) + // 0. Record last channel for heartbeat notifications (skip internal channels and cli) if opts.Channel != "" && opts.ChatID != "" { - // Don't record internal channels (cli, system, subagent) if !constants.IsInternalChannel(opts.Channel) { channelKey := fmt.Sprintf("%s:%s", opts.Channel, opts.ChatID) if err := al.RecordLastChannel(channelKey); err != nil { @@ -1056,9 +1055,12 @@ func (al *AgentLoop) runLLMIteration( "target_channel": al.targetReasoningChannelID(opts.Channel), "channel": opts.Channel, }) - // Check if no tool calls - we're done + // Check if no tool calls - then check reasoning content if any if len(response.ToolCalls) == 0 { finalContent = response.Content + if finalContent == "" && response.ReasoningContent != "" { + finalContent = response.ReasoningContent + } logger.InfoCF("agent", "LLM response without tool calls (direct answer)", map[string]any{ "agent_id": agent.ID, @@ -1144,15 +1146,47 @@ func (al *AgentLoop) runLLMIteration( "iteration": iteration, }) - // Create async callback for tools that implement AsyncExecutor - asyncCallback := func(callbackCtx context.Context, result *tools.ToolResult) { + // Create async callback for tools that implement AsyncExecutor. + // When the background work completes, this publishes the result + // as an inbound system message so processSystemMessage routes it + // back to the user via the normal agent loop. + asyncCallback := func(_ context.Context, result *tools.ToolResult) { + // Send ForUser content directly to the user (immediate feedback), + // mirroring the synchronous tool execution path. if !result.Silent && result.ForUser != "" { - logger.InfoCF("agent", "Async tool completed, agent will handle notification", - map[string]any{ - "tool": tc.Name, - "content_len": len(result.ForUser), - }) + outCtx, outCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer outCancel() + _ = al.bus.PublishOutbound(outCtx, bus.OutboundMessage{ + Channel: opts.Channel, + ChatID: opts.ChatID, + Content: result.ForUser, + }) } + + // Determine content for the agent loop (ForLLM or error). + content := result.ForLLM + if content == "" && result.Err != nil { + content = result.Err.Error() + } + if content == "" { + return + } + + logger.InfoCF("agent", "Async tool completed, publishing result", + map[string]any{ + "tool": tc.Name, + "content_len": len(content), + "channel": opts.Channel, + }) + + pubCtx, pubCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer pubCancel() + _ = al.bus.PublishInbound(pubCtx, bus.InboundMessage{ + Channel: "system", + SenderID: fmt.Sprintf("async:%s", tc.Name), + ChatID: fmt.Sprintf("%s:%s", opts.Channel, opts.ChatID), + Content: content, + }) } toolResult := agent.Tools.ExecuteWithContext( diff --git a/pkg/channels/irc/handler.go b/pkg/channels/irc/handler.go new file mode 100644 index 000000000..aca4ddd11 --- /dev/null +++ b/pkg/channels/irc/handler.go @@ -0,0 +1,154 @@ +package irc + +import ( + "fmt" + "strings" + "time" + "unicode" + + "github.com/ergochat/irc-go/ircevent" + "github.com/ergochat/irc-go/ircmsg" + + "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/identity" + "github.com/sipeed/picoclaw/pkg/logger" +) + +// onConnect is called after a successful connection (and on reconnect). +func (c *IRCChannel) onConnect(conn *ircevent.Connection) { + // NickServ auth (only if SASL is not configured) + if c.config.NickServPassword != "" && c.config.SASLUser == "" { + conn.Privmsg("NickServ", "IDENTIFY "+c.config.NickServPassword) + } + + // Join configured channels + for _, ch := range c.config.Channels { + conn.Join(ch) + logger.InfoCF("irc", "Joined IRC channel", map[string]any{ + "channel": ch, + }) + } +} + +// onPrivmsg handles incoming PRIVMSG events. +func (c *IRCChannel) onPrivmsg(conn *ircevent.Connection, e ircmsg.Message) { + if len(e.Params) < 2 { + return + } + + nick := e.Nick() + currentNick := conn.CurrentNick() + + // Ignore own messages + if strings.EqualFold(nick, currentNick) { + return + } + + target := e.Params[0] // channel name or bot's nick + content := e.Params[1] // message text + + // Determine if this is a DM or channel message + isDM := !strings.HasPrefix(target, "#") && !strings.HasPrefix(target, "&") + + var chatID string + var peer bus.Peer + + if isDM { + chatID = nick + peer = bus.Peer{Kind: "direct", ID: nick} + } else { + chatID = target + peer = bus.Peer{Kind: "group", ID: target} + } + + sender := bus.SenderInfo{ + Platform: "irc", + PlatformID: nick, + CanonicalID: identity.BuildCanonicalID("irc", nick), + Username: nick, + DisplayName: nick, + } + + if !c.IsAllowedSender(sender) { + return + } + + // For channel messages, check group trigger (mention detection) + if !isDM { + isMentioned := isBotMentioned(content, currentNick) + if isMentioned { + content = stripBotMention(content, currentNick) + } + respond, cleaned := c.ShouldRespondInGroup(isMentioned, content) + if !respond { + return + } + content = cleaned + } + + if strings.TrimSpace(content) == "" { + return + } + + messageID := fmt.Sprintf("%s-%d", nick, time.Now().UnixNano()) + + metadata := map[string]string{ + "platform": "irc", + "server": c.config.Server, + } + if !isDM { + metadata["channel"] = target + } + + c.HandleMessage(c.ctx, peer, messageID, nick, chatID, content, nil, metadata, sender) +} + +// nickMentionedAt returns the byte index where botNick is mentioned in content +// with word-boundary checks, or -1 if not found. Also checks for "nick:" / +// "nick," prefix convention. +func nickMentionedAt(content, botNick string) int { + lower := strings.ToLower(content) + lowerNick := strings.ToLower(botNick) + + // "nick:" or "nick," at start (most common IRC convention) + if strings.HasPrefix(lower, lowerNick+":") || strings.HasPrefix(lower, lowerNick+",") { + return 0 + } + + // Word-boundary match anywhere in the message + idx := strings.Index(lower, lowerNick) + if idx < 0 { + return -1 + } + runes := []rune(lower) + nickRunes := []rune(lowerNick) + endIdx := idx + len(string(nickRunes)) + before := idx == 0 || !unicode.IsLetter(runes[idx-1]) && !unicode.IsDigit(runes[idx-1]) + after := endIdx >= len(lower) || !unicode.IsLetter(rune(lower[endIdx])) && !unicode.IsDigit(rune(lower[endIdx])) + if before && after { + return idx + } + return -1 +} + +// isBotMentioned checks if the bot's nick appears in the message. +func isBotMentioned(content, botNick string) bool { + return nickMentionedAt(content, botNick) >= 0 +} + +// stripBotMention removes "nick: " or "nick, " prefix from content. +func stripBotMention(content, botNick string) string { + idx := nickMentionedAt(content, botNick) + if idx != 0 { + return content + } + lowerNick := strings.ToLower(botNick) + lower := strings.ToLower(content) + for _, sep := range []string{":", ","} { + prefix := lowerNick + sep + if strings.HasPrefix(lower, prefix) { + return strings.TrimSpace(content[len(prefix):]) + } + } + return content +} diff --git a/pkg/channels/irc/init.go b/pkg/channels/irc/init.go new file mode 100644 index 000000000..221d41b62 --- /dev/null +++ b/pkg/channels/irc/init.go @@ -0,0 +1,16 @@ +package irc + +import ( + "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/channels" + "github.com/sipeed/picoclaw/pkg/config" +) + +func init() { + channels.RegisterFactory("irc", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { + if !cfg.Channels.IRC.Enabled { + return nil, nil + } + return NewIRCChannel(cfg.Channels.IRC, b) + }) +} diff --git a/pkg/channels/irc/irc.go b/pkg/channels/irc/irc.go new file mode 100644 index 000000000..28c59b540 --- /dev/null +++ b/pkg/channels/irc/irc.go @@ -0,0 +1,194 @@ +package irc + +import ( + "context" + "crypto/tls" + "fmt" + "strings" + + "github.com/ergochat/irc-go/ircevent" + "github.com/ergochat/irc-go/ircmsg" + + "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/channels" + "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/logger" +) + +// IRCChannel implements the Channel interface for IRC servers. +type IRCChannel struct { + *channels.BaseChannel + config config.IRCConfig + conn *ircevent.Connection + ctx context.Context + cancel context.CancelFunc +} + +// NewIRCChannel creates a new IRC channel. +func NewIRCChannel(cfg config.IRCConfig, messageBus *bus.MessageBus) (*IRCChannel, error) { + if cfg.Server == "" { + return nil, fmt.Errorf("irc server is required") + } + if cfg.Nick == "" { + return nil, fmt.Errorf("irc nick is required") + } + + base := channels.NewBaseChannel("irc", cfg, messageBus, cfg.AllowFrom, + channels.WithMaxMessageLength(400), + channels.WithGroupTrigger(cfg.GroupTrigger), + channels.WithReasoningChannelID(cfg.ReasoningChannelID), + ) + + return &IRCChannel{ + BaseChannel: base, + config: cfg, + }, nil +} + +// Start connects to the IRC server and begins listening. +func (c *IRCChannel) Start(ctx context.Context) error { + logger.InfoC("irc", "Starting IRC channel") + c.ctx, c.cancel = context.WithCancel(ctx) + + user := c.config.User + if user == "" { + user = c.config.Nick + } + realName := c.config.RealName + if realName == "" { + realName = c.config.Nick + } + caps := []string(c.config.RequestCaps) + if len(caps) == 0 { + caps = []string{"server-time", "message-tags"} + } + + conn := &ircevent.Connection{ + Server: c.config.Server, + Nick: c.config.Nick, + User: user, + RealName: realName, + Password: c.config.Password, + UseTLS: c.config.TLS, + RequestCaps: caps, + QuitMessage: "Goodbye", + Debug: false, + Log: nil, + } + + if c.config.TLS { + conn.TLSConfig = &tls.Config{ + ServerName: extractHost(c.config.Server), + } + } + + // SASL auth (takes priority over NickServ) + if c.config.SASLUser != "" && c.config.SASLPassword != "" { + conn.SASLLogin = c.config.SASLUser + conn.SASLPassword = c.config.SASLPassword + } + + // Register event handlers + conn.AddConnectCallback(func(e ircmsg.Message) { + c.onConnect(conn) + }) + conn.AddCallback("PRIVMSG", func(e ircmsg.Message) { + c.onPrivmsg(conn, e) + }) + + if err := conn.Connect(); err != nil { + return fmt.Errorf("irc connect failed: %w", err) + } + + c.conn = conn + + // ircevent.Connection.Loop() handles reconnection internally. + go conn.Loop() + + c.SetRunning(true) + logger.InfoCF("irc", "IRC channel started", map[string]any{ + "server": c.config.Server, + "nick": c.config.Nick, + }) + return nil +} + +// Stop disconnects from the IRC server. +func (c *IRCChannel) Stop(ctx context.Context) error { + logger.InfoC("irc", "Stopping IRC channel") + c.SetRunning(false) + + if c.conn != nil { + c.conn.Quit() + } + if c.cancel != nil { + c.cancel() + } + + logger.InfoC("irc", "IRC channel stopped") + return nil +} + +// Send sends a message to an IRC channel or user. +func (c *IRCChannel) Send(ctx context.Context, msg bus.OutboundMessage) error { + if !c.IsRunning() { + return channels.ErrNotRunning + } + + target := msg.ChatID + if target == "" { + return fmt.Errorf("chat ID is empty: %w", channels.ErrSendFailed) + } + + if strings.TrimSpace(msg.Content) == "" { + return nil + } + + // Send each line separately (IRC is line-oriented) + lines := strings.Split(msg.Content, "\n") + for _, line := range lines { + line = strings.TrimRight(line, "\r") + if line == "" { + continue + } + c.conn.Privmsg(target, line) + } + + logger.DebugCF("irc", "Message sent", map[string]any{ + "target": target, + "lines": len(lines), + }) + return nil +} + +// StartTyping implements channels.TypingCapable using IRCv3 +typing client tag. +// Requires typing.enabled in config and server support for message-tags capability. +func (c *IRCChannel) StartTyping(ctx context.Context, chatID string) (func(), error) { + noop := func() {} + + if !c.config.Typing.Enabled || !c.IsRunning() || c.conn == nil { + return noop, nil + } + + // Check if server supports message-tags (required for TAGMSG) + if _, ok := c.conn.AcknowledgedCaps()["message-tags"]; !ok { + return noop, nil + } + + c.conn.SendWithTags(map[string]string{"+typing": "active"}, "TAGMSG", chatID) + + return func() { + if c.IsRunning() && c.conn != nil { + c.conn.SendWithTags(map[string]string{"+typing": "done"}, "TAGMSG", chatID) + } + }, nil +} + +// extractHost returns the hostname portion of a host:port string. +func extractHost(server string) string { + host, _, found := strings.Cut(server, ":") + if found { + return host + } + return server +} diff --git a/pkg/channels/irc/irc_test.go b/pkg/channels/irc/irc_test.go new file mode 100644 index 000000000..168252a4d --- /dev/null +++ b/pkg/channels/irc/irc_test.go @@ -0,0 +1,145 @@ +package irc + +import ( + "testing" + + "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/config" +) + +func TestNewIRCChannel(t *testing.T) { + msgBus := bus.NewMessageBus() + + t.Run("missing server", func(t *testing.T) { + cfg := config.IRCConfig{Nick: "bot"} + _, err := NewIRCChannel(cfg, msgBus) + if err == nil { + t.Error("expected error for missing server, got nil") + } + }) + + t.Run("missing nick", func(t *testing.T) { + cfg := config.IRCConfig{Server: "irc.example.com:6667"} + _, err := NewIRCChannel(cfg, msgBus) + if err == nil { + t.Error("expected error for missing nick, got nil") + } + }) + + t.Run("valid config", func(t *testing.T) { + cfg := config.IRCConfig{ + Server: "irc.example.com:6667", + Nick: "testbot", + Channels: []string{"#test"}, + } + ch, err := NewIRCChannel(cfg, msgBus) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ch.Name() != "irc" { + t.Errorf("Name() = %q, want %q", ch.Name(), "irc") + } + if ch.IsRunning() { + t.Error("new channel should not be running") + } + }) +} + +func TestExtractHost(t *testing.T) { + tests := []struct { + server string + want string + }{ + {"irc.libera.chat:6697", "irc.libera.chat"}, + {"localhost:6667", "localhost"}, + {"irc.example.com", "irc.example.com"}, + {"", ""}, + } + + for _, tt := range tests { + t.Run(tt.server, func(t *testing.T) { + got := extractHost(tt.server) + if got != tt.want { + t.Errorf("extractHost(%q) = %q, want %q", tt.server, got, tt.want) + } + }) + } +} + +func TestNickMentionedAt(t *testing.T) { + tests := []struct { + name string + content string + nick string + want int + }{ + {"colon prefix", "bot: hello", "bot", 0}, + {"comma prefix", "bot, hello", "bot", 0}, + {"case insensitive", "BOT: hello", "bot", 0}, + {"word boundary mid", "hey bot what's up", "bot", 4}, + {"no mention", "hello world", "bot", -1}, + {"substring mismatch", "robotics are cool", "bot", -1}, + {"nick at end", "hello bot", "bot", 6}, + {"empty content", "", "bot", -1}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := nickMentionedAt(tt.content, tt.nick) + if got != tt.want { + t.Errorf("nickMentionedAt(%q, %q) = %d, want %d", tt.content, tt.nick, got, tt.want) + } + }) + } +} + +func TestIsBotMentioned(t *testing.T) { + tests := []struct { + name string + content string + nick string + want bool + }{ + {"colon prefix", "bot: hello", "bot", true}, + {"comma prefix", "bot, hello", "bot", true}, + {"case insensitive", "BOT: hello", "bot", true}, + {"word boundary mid", "hey bot what's up", "bot", true}, + {"no mention", "hello world", "bot", false}, + {"substring mismatch", "robotics are cool", "bot", false}, + {"nick at end", "hello bot", "bot", true}, + {"empty content", "", "bot", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isBotMentioned(tt.content, tt.nick) + if got != tt.want { + t.Errorf("isBotMentioned(%q, %q) = %v, want %v", tt.content, tt.nick, got, tt.want) + } + }) + } +} + +func TestStripBotMention(t *testing.T) { + tests := []struct { + name string + content string + nick string + want string + }{ + {"colon prefix", "bot: hello there", "bot", "hello there"}, + {"comma prefix", "bot, help me", "bot", "help me"}, + {"case insensitive", "BOT: hello", "bot", "hello"}, + {"no prefix match", "hello bot", "bot", "hello bot"}, + {"only prefix", "bot:", "bot", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := stripBotMention(tt.content, tt.nick) + if got != tt.want { + t.Errorf("stripBotMention(%q, %q) = %q, want %q", tt.content, tt.nick, got, tt.want) + } + }) + } +} diff --git a/pkg/channels/manager.go b/pkg/channels/manager.go index 34f05cb7b..60490c10a 100644 --- a/pkg/channels/manager.go +++ b/pkg/channels/manager.go @@ -62,6 +62,7 @@ var channelRateConfig = map[string]float64{ "discord": 1, "slack": 1, "line": 10, + "irc": 2, } type channelWorker struct { @@ -271,6 +272,10 @@ func (m *Manager) initChannels() error { m.initChannel("pico_client", "Pico Client") } + if m.config.Channels.IRC.Enabled && m.config.Channels.IRC.Server != "" { + m.initChannel("irc", "IRC") + } + logger.InfoCF("channels", "Channel initialization completed", map[string]any{ "enabled_channels": len(m.channels), }) diff --git a/pkg/channels/telegram/telegram.go b/pkg/channels/telegram/telegram.go index a2035853c..0a36247a6 100644 --- a/pkg/channels/telegram/telegram.go +++ b/pkg/channels/telegram/telegram.go @@ -88,7 +88,7 @@ func NewTelegramChannel(cfg *config.Config, bus *bus.MessageBus) (*TelegramChann telegramCfg, bus, telegramCfg.AllowFrom, - channels.WithMaxMessageLength(4096), + channels.WithMaxMessageLength(4000), channels.WithGroupTrigger(telegramCfg.GroupTrigger), channels.WithReasoningChannelID(telegramCfg.ReasoningChannelID), ) @@ -173,22 +173,57 @@ func (c *TelegramChannel) Send(ctx context.Context, msg bus.OutboundMessage) err return fmt.Errorf("invalid chat ID %s: %w", msg.ChatID, channels.ErrSendFailed) } - htmlContent := markdownToTelegramHTML(msg.Content) + if msg.Content == "" { + return nil + } - // Typing/placeholder handled by Manager.preSend — just send the message + // The Manager already splits messages to ≤4000 chars (WithMaxMessageLength), + // so msg.Content is guaranteed to be within that limit. We still need to + // check if HTML expansion pushes it beyond Telegram's 4096-char API limit. + queue := []string{msg.Content} + for len(queue) > 0 { + chunk := queue[0] + queue = queue[1:] + + htmlContent := markdownToTelegramHTML(chunk) + + if len([]rune(htmlContent)) > 4096 { + ratio := float64(len([]rune(chunk))) / float64(len([]rune(htmlContent))) + smallerLen := int(float64(4096) * ratio * 0.95) // 5% safety margin + if smallerLen < 100 { + smallerLen = 100 + } + // Push sub-chunks back to the front of the queue for + // re-validation instead of sending them blindly. + subChunks := channels.SplitMessage(chunk, smallerLen) + queue = append(subChunks, queue...) + continue + } + + if err := c.sendHTMLChunk(ctx, chatID, htmlContent, chunk); err != nil { + return err + } + } + + return nil +} + +// sendHTMLChunk sends a single HTML message, falling back to the original +// markdown as plain text on parse failure so users never see raw HTML tags. +func (c *TelegramChannel) sendHTMLChunk(ctx context.Context, chatID int64, htmlContent, mdFallback string) error { tgMsg := tu.Message(tu.ID(chatID), htmlContent) tgMsg.ParseMode = telego.ModeHTML - if _, err = c.bot.SendMessage(ctx, tgMsg); err != nil { + if _, err := c.bot.SendMessage(ctx, tgMsg); err != nil { logger.ErrorCF("telegram", "HTML parse failed, falling back to plain text", map[string]any{ "error": err.Error(), }) + tgMsg.Text = mdFallback tgMsg.ParseMode = "" if _, err = c.bot.SendMessage(ctx, tgMsg); err != nil { return fmt.Errorf("telegram send: %w", channels.ErrTemporary) } } - return nil } diff --git a/pkg/channels/telegram/telegram_test.go b/pkg/channels/telegram/telegram_test.go new file mode 100644 index 000000000..3a2f1aa66 --- /dev/null +++ b/pkg/channels/telegram/telegram_test.go @@ -0,0 +1,273 @@ +package telegram + +import ( + "context" + "encoding/json" + "errors" + "strings" + "testing" + + "github.com/mymmrac/telego" + ta "github.com/mymmrac/telego/telegoapi" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/channels" +) + +const testToken = "1234567890:aaaabbbbaaaabbbbaaaabbbbaaaabbbbccc" + +// stubCaller implements ta.Caller for testing. +type stubCaller struct { + calls []stubCall + callFn func(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) +} + +type stubCall struct { + URL string + Data *ta.RequestData +} + +func (s *stubCaller) Call(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) { + s.calls = append(s.calls, stubCall{URL: url, Data: data}) + return s.callFn(ctx, url, data) +} + +// stubConstructor implements ta.RequestConstructor for testing. +type stubConstructor struct{} + +func (s *stubConstructor) JSONRequest(parameters any) (*ta.RequestData, error) { + return &ta.RequestData{}, nil +} + +func (s *stubConstructor) MultipartRequest( + parameters map[string]string, + files map[string]ta.NamedReader, +) (*ta.RequestData, error) { + return &ta.RequestData{}, nil +} + +// successResponse returns a ta.Response that telego will treat as a successful SendMessage. +func successResponse(t *testing.T) *ta.Response { + t.Helper() + msg := &telego.Message{MessageID: 1} + b, err := json.Marshal(msg) + require.NoError(t, err) + return &ta.Response{Ok: true, Result: b} +} + +// newTestChannel creates a TelegramChannel with a mocked bot for unit testing. +func newTestChannel(t *testing.T, caller *stubCaller) *TelegramChannel { + t.Helper() + + bot, err := telego.NewBot(testToken, + telego.WithAPICaller(caller), + telego.WithRequestConstructor(&stubConstructor{}), + telego.WithDiscardLogger(), + ) + require.NoError(t, err) + + base := channels.NewBaseChannel("telegram", nil, nil, nil, + channels.WithMaxMessageLength(4000), + ) + base.SetRunning(true) + + return &TelegramChannel{ + BaseChannel: base, + bot: bot, + chatIDs: make(map[string]int64), + } +} + +func TestSend_EmptyContent(t *testing.T) { + caller := &stubCaller{ + callFn: func(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) { + t.Fatal("SendMessage should not be called for empty content") + return nil, nil + }, + } + ch := newTestChannel(t, caller) + + err := ch.Send(context.Background(), bus.OutboundMessage{ + ChatID: "12345", + Content: "", + }) + + assert.NoError(t, err) + assert.Empty(t, caller.calls, "no API calls should be made for empty content") +} + +func TestSend_ShortMessage_SingleCall(t *testing.T) { + caller := &stubCaller{ + callFn: func(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) { + return successResponse(t), nil + }, + } + ch := newTestChannel(t, caller) + + err := ch.Send(context.Background(), bus.OutboundMessage{ + ChatID: "12345", + Content: "Hello, world!", + }) + + assert.NoError(t, err) + assert.Len(t, caller.calls, 1, "short message should result in exactly one SendMessage call") +} + +func TestSend_LongMessage_SingleCall(t *testing.T) { + // With WithMaxMessageLength(4000), the Manager pre-splits messages before + // they reach Send(). A message at exactly 4000 chars should go through + // as a single SendMessage call (no re-split needed since HTML expansion + // won't exceed 4096 for plain text). + caller := &stubCaller{ + callFn: func(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) { + return successResponse(t), nil + }, + } + ch := newTestChannel(t, caller) + + longContent := strings.Repeat("a", 4000) + + err := ch.Send(context.Background(), bus.OutboundMessage{ + ChatID: "12345", + Content: longContent, + }) + + assert.NoError(t, err) + assert.Len(t, caller.calls, 1, "pre-split message within limit should result in one SendMessage call") +} + +func TestSend_HTMLFallback_PerChunk(t *testing.T) { + callCount := 0 + caller := &stubCaller{ + callFn: func(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) { + callCount++ + // Fail on odd calls (HTML attempt), succeed on even calls (plain text fallback) + if callCount%2 == 1 { + return nil, errors.New("Bad Request: can't parse entities") + } + return successResponse(t), nil + }, + } + ch := newTestChannel(t, caller) + + err := ch.Send(context.Background(), bus.OutboundMessage{ + ChatID: "12345", + Content: "Hello **world**", + }) + + assert.NoError(t, err) + // One short message → 1 HTML attempt (fail) + 1 plain text fallback (success) = 2 calls + assert.Equal(t, 2, len(caller.calls), "should have HTML attempt + plain text fallback") +} + +func TestSend_HTMLFallback_BothFail(t *testing.T) { + caller := &stubCaller{ + callFn: func(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) { + return nil, errors.New("send failed") + }, + } + ch := newTestChannel(t, caller) + + err := ch.Send(context.Background(), bus.OutboundMessage{ + ChatID: "12345", + Content: "Hello", + }) + + assert.Error(t, err) + assert.True(t, errors.Is(err, channels.ErrTemporary), "error should wrap ErrTemporary") + assert.Equal(t, 2, len(caller.calls), "should have HTML attempt + plain text attempt") +} + +func TestSend_LongMessage_HTMLFallback_StopsOnError(t *testing.T) { + // With a long message that gets split into 2 chunks, if both HTML and + // plain text fail on the first chunk, Send should return early. + caller := &stubCaller{ + callFn: func(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) { + return nil, errors.New("send failed") + }, + } + ch := newTestChannel(t, caller) + + longContent := strings.Repeat("x", 4001) + + err := ch.Send(context.Background(), bus.OutboundMessage{ + ChatID: "12345", + Content: longContent, + }) + + assert.Error(t, err) + // Should fail on the first chunk (2 calls: HTML + fallback), never reaching the second chunk. + assert.Equal(t, 2, len(caller.calls), "should stop after first chunk fails both HTML and plain text") +} + +func TestSend_MarkdownShortButHTMLLong_MultipleCalls(t *testing.T) { + caller := &stubCaller{ + callFn: func(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) { + return successResponse(t), nil + }, + } + ch := newTestChannel(t, caller) + + // Create markdown whose length is <= 4000 but whose HTML expansion is much longer. + // "**a** " (6 chars) becomes "a " (9 chars) in HTML, so repeating it many times + // yields HTML that exceeds Telegram's limit while markdown stays within it. + markdownContent := strings.Repeat("**a** ", 600) // 3600 chars markdown, HTML ~5400+ chars + assert.LessOrEqual(t, len([]rune(markdownContent)), 4000, "markdown content must not exceed chunk size") + + htmlExpanded := markdownToTelegramHTML(markdownContent) + assert.Greater( + t, len([]rune(htmlExpanded)), 4096, + "HTML expansion must exceed Telegram limit for this test to be meaningful", + ) + + err := ch.Send(context.Background(), bus.OutboundMessage{ + ChatID: "12345", + Content: markdownContent, + }) + + assert.NoError(t, err) + assert.Greater( + t, len(caller.calls), 1, + "markdown-short but HTML-long message should be split into multiple SendMessage calls", + ) +} + +func TestSend_NotRunning(t *testing.T) { + caller := &stubCaller{ + callFn: func(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) { + t.Fatal("should not be called") + return nil, nil + }, + } + ch := newTestChannel(t, caller) + ch.SetRunning(false) + + err := ch.Send(context.Background(), bus.OutboundMessage{ + ChatID: "12345", + Content: "Hello", + }) + + assert.ErrorIs(t, err, channels.ErrNotRunning) + assert.Empty(t, caller.calls) +} + +func TestSend_InvalidChatID(t *testing.T) { + caller := &stubCaller{ + callFn: func(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) { + t.Fatal("should not be called") + return nil, nil + }, + } + ch := newTestChannel(t, caller) + + err := ch.Send(context.Background(), bus.OutboundMessage{ + ChatID: "not-a-number", + Content: "Hello", + }) + + assert.Error(t, err) + assert.True(t, errors.Is(err, channels.ErrSendFailed), "error should wrap ErrSendFailed") + assert.Empty(t, caller.calls) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 7e364ad98..5886949f7 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -232,6 +232,7 @@ type ChannelsConfig struct { WeComAIBot WeComAIBotConfig `json:"wecom_aibot"` Pico PicoConfig `json:"pico"` PicoClient PicoClientConfig `json:"pico_client"` + IRC IRCConfig `json:"irc"` } // GroupTriggerConfig controls when the bot responds in group chats. @@ -425,6 +426,25 @@ type PicoClientConfig struct { AllowFrom FlexibleStringSlice `json:"allow_from" env:"PICOCLAW_CHANNELS_PICO_CLIENT_ALLOW_FROM"` } +type IRCConfig struct { + Enabled bool `json:"enabled" env:"PICOCLAW_CHANNELS_IRC_ENABLED"` + Server string `json:"server" env:"PICOCLAW_CHANNELS_IRC_SERVER"` + TLS bool `json:"tls" env:"PICOCLAW_CHANNELS_IRC_TLS"` + Nick string `json:"nick" env:"PICOCLAW_CHANNELS_IRC_NICK"` + User string `json:"user,omitempty" env:"PICOCLAW_CHANNELS_IRC_USER"` + RealName string `json:"real_name,omitempty" env:"PICOCLAW_CHANNELS_IRC_REAL_NAME"` + Password string `json:"password" env:"PICOCLAW_CHANNELS_IRC_PASSWORD"` + NickServPassword string `json:"nickserv_password" env:"PICOCLAW_CHANNELS_IRC_NICKSERV_PASSWORD"` + SASLUser string `json:"sasl_user" env:"PICOCLAW_CHANNELS_IRC_SASL_USER"` + SASLPassword string `json:"sasl_password" env:"PICOCLAW_CHANNELS_IRC_SASL_PASSWORD"` + Channels FlexibleStringSlice `json:"channels" env:"PICOCLAW_CHANNELS_IRC_CHANNELS"` + RequestCaps FlexibleStringSlice `json:"request_caps,omitempty" env:"PICOCLAW_CHANNELS_IRC_REQUEST_CAPS"` + AllowFrom FlexibleStringSlice `json:"allow_from" env:"PICOCLAW_CHANNELS_IRC_ALLOW_FROM"` + GroupTrigger GroupTriggerConfig `json:"group_trigger,omitempty"` + Typing TypingConfig `json:"typing,omitempty"` + ReasoningChannelID string `json:"reasoning_channel_id" env:"PICOCLAW_CHANNELS_IRC_REASONING_CHANNEL_ID"` +} + type HeartbeatConfig struct { Enabled bool `json:"enabled" env:"PICOCLAW_HEARTBEAT_ENABLED"` Interval int `json:"interval" env:"PICOCLAW_HEARTBEAT_INTERVAL"` // minutes, min 5 diff --git a/pkg/providers/openai_compat/provider.go b/pkg/providers/openai_compat/provider.go index 1904ee153..caee6f9ce 100644 --- a/pkg/providers/openai_compat/provider.go +++ b/pkg/providers/openai_compat/provider.go @@ -1,6 +1,7 @@ package openai_compat import ( + "bufio" "bytes" "context" "encoding/json" @@ -183,19 +184,94 @@ func (p *Provider) Chat( } defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response: %w", err) - } + contentType := resp.Header.Get("Content-Type") + // Non-200: read a prefix to tell HTML error page apart from JSON error body. if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("API request failed:\n Status: %d\n Body: %s", resp.StatusCode, string(body)) + body, readErr := io.ReadAll(io.LimitReader(resp.Body, 256)) + if readErr != nil { + return nil, fmt.Errorf("failed to read response: %w", readErr) + } + if looksLikeHTML(body, contentType) { + return nil, wrapHTMLResponseError(resp.StatusCode, body, contentType, p.apiBase) + } + return nil, fmt.Errorf( + "API request failed:\n Status: %d\n Body: %s", + resp.StatusCode, + responsePreview(body, 128), + ) } - return parseResponse(body) + // Peek without consuming so the full stream reaches the JSON decoder. + reader := bufio.NewReader(resp.Body) + prefix, err := reader.Peek(256) // io.EOF/ErrBufferFull are normal; only real errors abort + if err != nil && err != io.EOF && err != bufio.ErrBufferFull { + return nil, fmt.Errorf("failed to inspect response: %w", err) + } + if looksLikeHTML(prefix, contentType) { + return nil, wrapHTMLResponseError(resp.StatusCode, prefix, contentType, p.apiBase) + } + + out, err := parseResponse(reader) + if err != nil { + return nil, fmt.Errorf("failed to parse JSON response: %w", err) + } + + return out, nil } -func parseResponse(body []byte) (*LLMResponse, error) { +func wrapHTMLResponseError(statusCode int, body []byte, contentType, apiBase string) error { + respPreview := responsePreview(body, 128) + return fmt.Errorf( + "API request failed: %s returned HTML instead of JSON (content-type: %s); check api_base or proxy configuration.\n Status: %d\n Body: %s", + apiBase, + contentType, + statusCode, + respPreview, + ) +} + +func looksLikeHTML(body []byte, contentType string) bool { + contentType = strings.ToLower(strings.TrimSpace(contentType)) + if strings.Contains(contentType, "text/html") || strings.Contains(contentType, "application/xhtml+xml") { + return true + } + prefix := bytes.ToLower(leadingTrimmedPrefix(body, 128)) + return bytes.HasPrefix(prefix, []byte(" len(body) { + end = len(body) + } + return body[i:end] + } + } + return nil +} + +func responsePreview(body []byte, maxLen int) string { + trimmed := bytes.TrimSpace(body) + if len(trimmed) == 0 { + return "" + } + if len(trimmed) <= maxLen { + return string(trimmed) + } + return string(trimmed[:maxLen]) + "..." +} + +func parseResponse(body io.Reader) (*LLMResponse, error) { var apiResponse struct { Choices []struct { Message struct { @@ -222,8 +298,8 @@ func parseResponse(body []byte) (*LLMResponse, error) { Usage *UsageInfo `json:"usage"` } - if err := json.Unmarshal(body, &apiResponse); err != nil { - return nil, fmt.Errorf("failed to unmarshal response: %w", err) + if err := json.NewDecoder(body).Decode(&apiResponse); err != nil { + return nil, fmt.Errorf("failed to decode response: %w", err) } if len(apiResponse.Choices) == 0 { diff --git a/pkg/providers/openai_compat/provider_test.go b/pkg/providers/openai_compat/provider_test.go index 174bcf00d..5c4dcd1b0 100644 --- a/pkg/providers/openai_compat/provider_test.go +++ b/pkg/providers/openai_compat/provider_test.go @@ -1,7 +1,10 @@ package openai_compat import ( + "bytes" "encoding/json" + "fmt" + "io" "net/http" "net/http/httptest" "net/url" @@ -212,6 +215,132 @@ func TestProviderChat_HTTPError(t *testing.T) { } } +func TestProviderChat_JSONHTTPErrorDoesNotReportHTML(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"bad request"}`)) + })) + defer server.Close() + + p := NewProvider("key", server.URL, "") + _, err := p.Chat(t.Context(), []Message{{Role: "user", Content: "hi"}}, nil, "gpt-4o", nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "Status: 400") { + t.Fatalf("expected status code in error, got %v", err) + } + if strings.Contains(err.Error(), "returned HTML instead of JSON") { + t.Fatalf("expected non-HTML http error, got %v", err) + } +} + +func TestProviderChat_HTMLResponsesReturnHelpfulError(t *testing.T) { + tests := []struct { + name string + contentType string + statusCode int + body string + }{ + { + name: "html success response", + contentType: "text/html; charset=utf-8", + statusCode: http.StatusOK, + body: "gateway login", + }, + { + name: "html error response", + contentType: "text/html; charset=utf-8", + statusCode: http.StatusBadGateway, + body: "bad gateway", + }, + { + name: "mislabeled html success response", + contentType: "application/json", + statusCode: http.StatusOK, + body: " \r\n\tgateway login", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", tt.contentType) + w.WriteHeader(tt.statusCode) + _, _ = w.Write([]byte(tt.body)) + })) + defer server.Close() + + p := NewProvider("key", server.URL, "") + _, err := p.Chat(t.Context(), []Message{{Role: "user", Content: "hi"}}, nil, "gpt-4o", nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), fmt.Sprintf("Status: %d", tt.statusCode)) { + t.Fatalf("expected status code in error, got %v", err) + } + if !strings.Contains(err.Error(), "returned HTML instead of JSON") { + t.Fatalf("expected helpful HTML error, got %v", err) + } + if !strings.Contains(err.Error(), "check api_base or proxy configuration") { + t.Fatalf("expected configuration hint, got %v", err) + } + }) + } +} + +func TestProviderChat_SuccessResponseUsesStreamingDecoder(t *testing.T) { + content := strings.Repeat("a", 1024) + body := `{"choices":[{"message":{"content":"` + content + `"},"finish_reason":"stop"}]}` + + p := NewProvider("key", "https://example.com/v1", "") + p.httpClient = &http.Client{ + Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: &errAfterDataReadCloser{ + data: []byte(body), + chunkSize: 64, + }, + }, nil + }), + } + + out, err := p.Chat(t.Context(), []Message{{Role: "user", Content: "hi"}}, nil, "gpt-4o", nil) + if err != nil { + t.Fatalf("Chat() error = %v", err) + } + if out.Content != content { + t.Fatalf("Content = %q, want %q", out.Content, content) + } +} + +func TestProviderChat_LargeHTMLResponsePreviewIsTruncated(t *testing.T) { + body := append([]byte(""), bytes.Repeat([]byte("A"), 2048)...) + body = append(body, []byte("")...) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(http.StatusBadGateway) + _, _ = w.Write(body) + })) + defer server.Close() + + p := NewProvider("key", server.URL, "") + _, err := p.Chat(t.Context(), []Message{{Role: "user", Content: "hi"}}, nil, "gpt-4o", nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "Body: ") { + t.Fatalf("expected html preview in error, got %v", err) + } + if !strings.Contains(err.Error(), "...") { + t.Fatalf("expected truncated preview, got %v", err) + } +} + func TestProviderChat_StripsMoonshotPrefixAndNormalizesKimiTemperature(t *testing.T) { var requestBody map[string]any @@ -399,6 +528,40 @@ func TestProvider_RequestTimeoutOverride(t *testing.T) { } } +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { + return f(r) +} + +type errAfterDataReadCloser struct { + data []byte + chunkSize int + offset int +} + +func (r *errAfterDataReadCloser) Read(p []byte) (int, error) { + if r.offset >= len(r.data) { + return 0, io.ErrUnexpectedEOF + } + + n := r.chunkSize + if n <= 0 || n > len(p) { + n = len(p) + } + remaining := len(r.data) - r.offset + if n > remaining { + n = remaining + } + copy(p, r.data[r.offset:r.offset+n]) + r.offset += n + return n, nil +} + +func (r *errAfterDataReadCloser) Close() error { + return nil +} + func TestProvider_FunctionalOptionMaxTokensField(t *testing.T) { p := NewProvider("key", "https://example.com/v1", "", WithMaxTokensField("max_completion_tokens")) if p.maxTokensField != "max_completion_tokens" { diff --git a/pkg/tools/spawn_test.go b/pkg/tools/spawn_test.go index 0646c82a9..43223b8db 100644 --- a/pkg/tools/spawn_test.go +++ b/pkg/tools/spawn_test.go @@ -8,7 +8,7 @@ import ( func TestSpawnTool_Execute_EmptyTask(t *testing.T) { provider := &MockLLMProvider{} - manager := NewSubagentManager(provider, "test-model", "/tmp/test", nil) + manager := NewSubagentManager(provider, "test-model", "/tmp/test") tool := NewSpawnTool(manager) ctx := context.Background() @@ -42,7 +42,7 @@ func TestSpawnTool_Execute_EmptyTask(t *testing.T) { func TestSpawnTool_Execute_ValidTask(t *testing.T) { provider := &MockLLMProvider{} - manager := NewSubagentManager(provider, "test-model", "/tmp/test", nil) + manager := NewSubagentManager(provider, "test-model", "/tmp/test") tool := NewSpawnTool(manager) ctx := context.Background() diff --git a/pkg/tools/subagent.go b/pkg/tools/subagent.go index 429340047..e51cbaafa 100644 --- a/pkg/tools/subagent.go +++ b/pkg/tools/subagent.go @@ -6,7 +6,6 @@ import ( "sync" "time" - "github.com/sipeed/picoclaw/pkg/bus" "github.com/sipeed/picoclaw/pkg/providers" ) @@ -27,7 +26,6 @@ type SubagentManager struct { mu sync.RWMutex provider providers.LLMProvider defaultModel string - bus *bus.MessageBus workspace string tools *ToolRegistry maxIterations int @@ -41,13 +39,11 @@ type SubagentManager struct { func NewSubagentManager( provider providers.LLMProvider, defaultModel, workspace string, - bus *bus.MessageBus, ) *SubagentManager { return &SubagentManager{ tasks: make(map[string]*SubagentTask), provider: provider, defaultModel: defaultModel, - bus: bus, workspace: workspace, tools: NewToolRegistry(), maxIterations: 10, @@ -214,20 +210,6 @@ After completing the task, provide a clear summary of what was done.` Async: false, } } - - // Send announce message back to main agent - if sm.bus != nil { - announceContent := fmt.Sprintf("Task '%s' completed.\n\nResult:\n%s", task.Label, task.Result) - pubCtx, pubCancel := context.WithTimeout(context.Background(), 5*time.Second) - defer pubCancel() - sm.bus.PublishInbound(pubCtx, bus.InboundMessage{ - Channel: "system", - SenderID: fmt.Sprintf("subagent:%s", task.ID), - // Format: "original_channel:original_chat_id" for routing back - ChatID: fmt.Sprintf("%s:%s", task.OriginChannel, task.OriginChatID), - Content: announceContent, - }) - } } func (sm *SubagentManager) GetTask(taskID string) (*SubagentTask, bool) { diff --git a/pkg/tools/subagent_tool_test.go b/pkg/tools/subagent_tool_test.go index a1450410a..4b6f130a5 100644 --- a/pkg/tools/subagent_tool_test.go +++ b/pkg/tools/subagent_tool_test.go @@ -5,7 +5,6 @@ import ( "strings" "testing" - "github.com/sipeed/picoclaw/pkg/bus" "github.com/sipeed/picoclaw/pkg/providers" ) @@ -47,7 +46,7 @@ func (m *MockLLMProvider) GetContextWindow() int { func TestSubagentManager_SetLLMOptions_AppliesToRunToolLoop(t *testing.T) { provider := &MockLLMProvider{} - manager := NewSubagentManager(provider, "test-model", "/tmp/test", nil) + manager := NewSubagentManager(provider, "test-model", "/tmp/test") manager.SetLLMOptions(2048, 0.6) tool := NewSubagentTool(manager) @@ -73,7 +72,7 @@ func TestSubagentManager_SetLLMOptions_AppliesToRunToolLoop(t *testing.T) { // TestSubagentTool_Name verifies tool name func TestSubagentTool_Name(t *testing.T) { provider := &MockLLMProvider{} - manager := NewSubagentManager(provider, "test-model", "/tmp/test", nil) + manager := NewSubagentManager(provider, "test-model", "/tmp/test") tool := NewSubagentTool(manager) if tool.Name() != "subagent" { @@ -84,7 +83,7 @@ func TestSubagentTool_Name(t *testing.T) { // TestSubagentTool_Description verifies tool description func TestSubagentTool_Description(t *testing.T) { provider := &MockLLMProvider{} - manager := NewSubagentManager(provider, "test-model", "/tmp/test", nil) + manager := NewSubagentManager(provider, "test-model", "/tmp/test") tool := NewSubagentTool(manager) desc := tool.Description() @@ -99,7 +98,7 @@ func TestSubagentTool_Description(t *testing.T) { // TestSubagentTool_Parameters verifies tool parameters schema func TestSubagentTool_Parameters(t *testing.T) { provider := &MockLLMProvider{} - manager := NewSubagentManager(provider, "test-model", "/tmp/test", nil) + manager := NewSubagentManager(provider, "test-model", "/tmp/test") tool := NewSubagentTool(manager) params := tool.Parameters() @@ -149,8 +148,7 @@ func TestSubagentTool_Parameters(t *testing.T) { // TestSubagentTool_Execute_Success tests successful execution func TestSubagentTool_Execute_Success(t *testing.T) { provider := &MockLLMProvider{} - msgBus := bus.NewMessageBus() - manager := NewSubagentManager(provider, "test-model", "/tmp/test", msgBus) + manager := NewSubagentManager(provider, "test-model", "/tmp/test") tool := NewSubagentTool(manager) ctx := WithToolContext(context.Background(), "telegram", "chat-123") @@ -204,8 +202,7 @@ func TestSubagentTool_Execute_Success(t *testing.T) { // TestSubagentTool_Execute_NoLabel tests execution without label func TestSubagentTool_Execute_NoLabel(t *testing.T) { provider := &MockLLMProvider{} - msgBus := bus.NewMessageBus() - manager := NewSubagentManager(provider, "test-model", "/tmp/test", msgBus) + manager := NewSubagentManager(provider, "test-model", "/tmp/test") tool := NewSubagentTool(manager) ctx := context.Background() @@ -228,7 +225,7 @@ func TestSubagentTool_Execute_NoLabel(t *testing.T) { // TestSubagentTool_Execute_MissingTask tests error handling for missing task func TestSubagentTool_Execute_MissingTask(t *testing.T) { provider := &MockLLMProvider{} - manager := NewSubagentManager(provider, "test-model", "/tmp/test", nil) + manager := NewSubagentManager(provider, "test-model", "/tmp/test") tool := NewSubagentTool(manager) ctx := context.Background() @@ -278,8 +275,7 @@ func TestSubagentTool_Execute_NilManager(t *testing.T) { // TestSubagentTool_Execute_ContextPassing verifies context is properly used func TestSubagentTool_Execute_ContextPassing(t *testing.T) { provider := &MockLLMProvider{} - msgBus := bus.NewMessageBus() - manager := NewSubagentManager(provider, "test-model", "/tmp/test", msgBus) + manager := NewSubagentManager(provider, "test-model", "/tmp/test") tool := NewSubagentTool(manager) channel := "test-channel" @@ -304,8 +300,7 @@ func TestSubagentTool_Execute_ContextPassing(t *testing.T) { func TestSubagentTool_ForUserTruncation(t *testing.T) { // Create a mock provider that returns very long content provider := &MockLLMProvider{} - msgBus := bus.NewMessageBus() - manager := NewSubagentManager(provider, "test-model", "/tmp/test", msgBus) + manager := NewSubagentManager(provider, "test-model", "/tmp/test") tool := NewSubagentTool(manager) ctx := context.Background() diff --git a/scripts/test-irc.sh b/scripts/test-irc.sh new file mode 100755 index 000000000..40db01756 --- /dev/null +++ b/scripts/test-irc.sh @@ -0,0 +1,56 @@ +#!/bin/sh +# Starts a local Ergo IRC server for testing the IRC channel. +# +# Requirements: docker +# Usage: ./scripts/test-irc.sh + +set -e + +CONTAINER_NAME="picoclaw-test-ergo" +IRC_PORT=6667 + +# Clean up any previous instance +docker rm -f "$CONTAINER_NAME" >/dev/null 2>&1 || true + +echo "Starting Ergo IRC server on port $IRC_PORT..." +docker run -d \ + --name "$CONTAINER_NAME" \ + -p "$IRC_PORT:6667" \ + ghcr.io/ergochat/ergo:stable + +for i in $(seq 1 10); do + if nc -z localhost "$IRC_PORT" 2>/dev/null; then + break + fi + if [ "$i" -eq 10 ]; then + echo "ERROR: Server did not start within 10s" + exit 1 + fi + sleep 1 +done + +echo "" +echo "IRC server ready on localhost:$IRC_PORT" +echo "" +echo "Add this to your ~/.picoclaw/config.json under \"channels\":" +echo "" +echo ' "irc": {' +echo ' "enabled": true,' +echo ' "server": "localhost:6667",' +echo ' "tls": false,' +echo ' "nick": "picobot",' +echo ' "channels": ["#test"],' +echo ' "allow_from": [],' +echo ' "group_trigger": { "mention_only": true }' +echo ' }' +echo "" +echo "Then run picoclaw:" +echo " cd packages/picoclaw && go run ./cmd/picoclaw gateway" +echo "" +echo "Connect with an IRC client:" +echo " irssi: /connect localhost $IRC_PORT" +echo " weechat: /server add test localhost/$IRC_PORT && /connect test" +echo " Join #test, then: picobot: hello" +echo "" +echo "To stop the IRC server:" +echo " docker rm -f $CONTAINER_NAME"