Merge pull request #861 from p3ddd/refactor/modernize

refactor(modernize): apply safe modernize fixes
This commit is contained in:
Meng Zhuo
2026-03-01 15:38:59 +08:00
committed by GitHub
27 changed files with 88 additions and 119 deletions
+3 -4
View File
@@ -7,6 +7,7 @@ import (
"os"
"path/filepath"
"runtime"
"slices"
"strings"
"sync"
"time"
@@ -249,10 +250,8 @@ func (cb *ContextBuilder) sourceFilesChangedLocked() bool {
}
// Check tracked source files (bootstrap + memory).
for _, p := range cb.sourcePaths() {
if cb.fileChangedSince(p) {
return true
}
if slices.ContainsFunc(cb.sourcePaths(), cb.fileChangedSince) {
return true
}
// --- Skills directory (handled separately from sourcePaths) ---
+2 -2
View File
@@ -404,11 +404,11 @@ func TestConcurrentBuildSystemPromptWithCache(t *testing.T) {
var wg sync.WaitGroup
errs := make(chan string, goroutines*iterations)
for g := 0; g < goroutines; g++ {
for g := range goroutines {
wg.Add(1)
go func(id int) {
defer wg.Done()
for i := 0; i < iterations; i++ {
for i := range iterations {
result := cb.BuildSystemPromptWithCache()
if result == "" {
errs <- "empty prompt returned"
+3 -14
View File
@@ -5,6 +5,7 @@ import (
"fmt"
"os"
"path/filepath"
"slices"
"testing"
"time"
@@ -187,13 +188,7 @@ func TestToolRegistry_ToolRegistration(t *testing.T) {
toolsList := toolsInfo["names"].([]string)
// Check that our custom tool name is in the list
found := false
for _, name := range toolsList {
if name == "mock_custom" {
found = true
break
}
}
found := slices.Contains(toolsList, "mock_custom")
if !found {
t.Error("Expected custom tool to be registered")
}
@@ -262,13 +257,7 @@ func TestToolRegistry_GetDefinitions(t *testing.T) {
toolsList := toolsInfo["names"].([]string)
// Check that our custom tool name is in the list
found := false
for _, name := range toolsList {
if name == "mock_custom" {
found = true
break
}
}
found := slices.Contains(toolsList, "mock_custom")
if !found {
t.Error("Expected custom tool to be registered")
}
+1 -1
View File
@@ -111,7 +111,7 @@ func (ms *MemoryStore) GetRecentDailyNotes(days int) string {
var sb strings.Builder
first := true
for i := 0; i < days; i++ {
for i := range days {
date := time.Now().AddDate(0, 0, -i)
dateStr := date.Format("20060102") // YYYYMMDD
monthDir := dateStr[:6] // YYYYMM
+3 -3
View File
@@ -67,7 +67,7 @@ func TestPublishInbound_ContextCancel(t *testing.T) {
// Fill the buffer
ctx := context.Background()
for i := 0; i < defaultBusBufferSize; i++ {
for i := range defaultBusBufferSize {
if err := mb.PublishInbound(ctx, InboundMessage{Content: "fill"}); err != nil {
t.Fatalf("fill failed at %d: %v", i, err)
}
@@ -154,7 +154,7 @@ func TestConcurrentPublishClose(t *testing.T) {
wg.Add(numGoroutines + 1)
// Spawn many goroutines trying to publish
for i := 0; i < numGoroutines; i++ {
for range numGoroutines {
go func() {
defer wg.Done()
// Use a short timeout context so we don't block forever after close
@@ -194,7 +194,7 @@ func TestPublishInbound_FullBuffer(t *testing.T) {
ctx := context.Background()
// Fill the buffer
for i := 0; i < defaultBusBufferSize; i++ {
for i := range defaultBusBufferSize {
if err := mb.PublishInbound(ctx, InboundMessage{Content: "fill"}); err != nil {
t.Fatalf("fill failed at %d: %v", i, err)
}
+6 -8
View File
@@ -274,13 +274,12 @@ func TestWorkerRateLimiter(t *testing.T) {
limiter: rate.NewLimiter(2, 1),
}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
ctx := t.Context()
go m.runWorker(ctx, "test", w)
// Enqueue 4 messages
for i := 0; i < 4; i++ {
for i := range 4 {
w.queue <- bus.OutboundMessage{Channel: "test", ChatID: "1", Content: fmt.Sprintf("msg%d", i)}
}
@@ -352,8 +351,7 @@ func TestRunWorker_MessageSplitting(t *testing.T) {
limiter: rate.NewLimiter(rate.Inf, 1),
}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
ctx := t.Context()
go m.runWorker(ctx, "test", w)
@@ -576,7 +574,7 @@ func TestRecordPlaceholder_ConcurrentSafe(t *testing.T) {
m := newTestManager()
var wg sync.WaitGroup
for i := 0; i < 100; i++ {
for i := range 100 {
wg.Add(1)
go func(i int) {
defer wg.Done()
@@ -591,7 +589,7 @@ func TestRecordTypingStop_ConcurrentSafe(t *testing.T) {
m := newTestManager()
var wg sync.WaitGroup
for i := 0; i < 100; i++ {
for i := range 100 {
wg.Add(1)
go func(i int) {
defer wg.Done()
@@ -834,7 +832,7 @@ func TestLazyWorkerCreation(t *testing.T) {
func TestBuildMediaScope_FastIDUniqueness(t *testing.T) {
seen := make(map[string]bool)
for i := 0; i < 1000; i++ {
for range 1000 {
scope := BuildMediaScope("test", "chat1", "")
if seen[scope] {
t.Fatalf("duplicate scope generated: %s", scope)
+1 -4
View File
@@ -337,10 +337,7 @@ func (c *OneBotChannel) sendAPIRequest(action string, params any, timeout time.D
}
func (c *OneBotChannel) reconnectLoop() {
interval := time.Duration(c.config.ReconnectInterval) * time.Second
if interval < 5*time.Second {
interval = 5 * time.Second
}
interval := max(time.Duration(c.config.ReconnectInterval)*time.Second, 5*time.Second)
for {
select {
+2 -2
View File
@@ -292,8 +292,8 @@ func (c *PicoChannel) authenticate(r *http.Request) bool {
// Check Authorization header
auth := r.Header.Get("Authorization")
if strings.HasPrefix(auth, "Bearer ") {
if strings.TrimPrefix(auth, "Bearer ") == token {
if after, ok := strings.CutPrefix(auth, "Bearer "); ok {
if after == token {
return true
}
}
+8 -24
View File
@@ -23,10 +23,7 @@ func SplitMessage(content string, maxLen int) []string {
var messages []string
// Dynamic buffer: 10% of maxLen, but at least 50 chars if possible
codeBlockBuffer := maxLen / 10
if codeBlockBuffer < 50 {
codeBlockBuffer = 50
}
codeBlockBuffer := max(maxLen/10, 50)
if codeBlockBuffer > maxLen/2 {
codeBlockBuffer = maxLen / 2
}
@@ -40,10 +37,7 @@ func SplitMessage(content string, maxLen int) []string {
}
// Effective split point: maxLen minus buffer, to leave room for code blocks
effectiveLimit := maxLen - codeBlockBuffer
if effectiveLimit < maxLen/2 {
effectiveLimit = maxLen / 2
}
effectiveLimit := max(maxLen-codeBlockBuffer, maxLen/2)
end := start + effectiveLimit
@@ -85,10 +79,9 @@ func SplitMessage(content string, maxLen int) []string {
// If we have a reasonable amount of content after the header, split inside
if msgEnd > headerEndIdx+20 {
// Find a better split point closer to maxLen
innerLimit := start + maxLen - 5 // Leave room for "\n```"
if innerLimit > totalLen {
innerLimit = totalLen
}
innerLimit := min(
// Leave room for "\n```"
start+maxLen-5, totalLen)
betterEnd := findLastNewlineInRange(runes, start, innerLimit, 200)
if betterEnd > headerEndIdx {
msgEnd = betterEnd
@@ -117,10 +110,7 @@ func SplitMessage(content string, maxLen int) []string {
if unclosedIdx-start > 20 {
msgEnd = unclosedIdx
} else {
splitAt := start + maxLen - 5
if splitAt > totalLen {
splitAt = totalLen
}
splitAt := min(start+maxLen-5, totalLen)
chunk := strings.TrimRight(string(runes[start:splitAt]), " \t\n\r") + "\n```"
messages = append(messages, chunk)
remaining := strings.TrimSpace(header + "\n" + string(runes[splitAt:totalLen]))
@@ -196,10 +186,7 @@ func findNewlineFrom(runes []rune, from int) int {
// findLastNewlineInRange finds the last newline within the last searchWindow runes
// of the range runes[start:end]. Returns the absolute index or start-1 (indicating not found).
func findLastNewlineInRange(runes []rune, start, end, searchWindow int) int {
searchStart := end - searchWindow
if searchStart < start {
searchStart = start
}
searchStart := max(end-searchWindow, start)
for i := end - 1; i >= searchStart; i-- {
if runes[i] == '\n' {
return i
@@ -211,10 +198,7 @@ func findLastNewlineInRange(runes []rune, start, end, searchWindow int) int {
// findLastSpaceInRange finds the last space/tab within the last searchWindow runes
// of the range runes[start:end]. Returns the absolute index or start-1 (indicating not found).
func findLastSpaceInRange(runes []rune, start, end, searchWindow int) int {
searchStart := end - searchWindow
if searchStart < start {
searchStart = start
}
searchStart := max(end-searchWindow, start)
for i := end - 1; i >= searchStart; i-- {
if runes[i] == ' ' || runes[i] == '\t' {
return i
+1 -1
View File
@@ -43,7 +43,7 @@ func encryptTestMessageApp(message, aesKey string) (string, error) {
// Prepare message: random(16) + msg_len(4) + msg + corp_id
random := make([]byte, 0, 16)
for i := 0; i < 16; i++ {
for i := range 16 {
random = append(random, byte(i+1))
}
+1 -1
View File
@@ -42,7 +42,7 @@ func encryptTestMessage(message, aesKey string) (string, error) {
// Prepare message: random(16) + msg_len(4) + msg + receiveid
random := make([]byte, 0, 16)
for i := 0; i < 16; i++ {
for i := range 16 {
random = append(random, byte(i))
}
+1 -1
View File
@@ -125,7 +125,7 @@ func pkcs7Unpad(data []byte) ([]byte, error) {
return nil, fmt.Errorf("padding size larger than data")
}
// Verify all padding bytes
for i := 0; i < padding; i++ {
for i := range padding {
if data[len(data)-1-i] != byte(padding) {
return nil, fmt.Errorf("invalid padding byte at position %d", i)
}
+5 -7
View File
@@ -64,7 +64,7 @@ func TestGetModelConfig_RoundRobin(t *testing.T) {
// Test round-robin distribution
results := make(map[string]int)
for i := 0; i < 30; i++ {
for range 30 {
result, err := cfg.GetModelConfig("lb-model")
if err != nil {
t.Fatalf("GetModelConfig() error = %v", err)
@@ -94,17 +94,15 @@ func TestGetModelConfig_Concurrent(t *testing.T) {
var wg sync.WaitGroup
errors := make(chan error, goroutines*iterations)
for i := 0; i < goroutines; i++ {
wg.Add(1)
go func() {
defer wg.Done()
for j := 0; j < iterations; j++ {
for range goroutines {
wg.Go(func() {
for range iterations {
_, err := cfg.GetModelConfig("concurrent-model")
if err != nil {
errors <- err
}
}
}()
})
}
wg.Wait()
+2 -3
View File
@@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"maps"
"net/http"
"sync"
"time"
@@ -122,9 +123,7 @@ func (s *Server) readyHandler(w http.ResponseWriter, r *http.Request) {
s.mu.RLock()
ready := s.ready
checks := make(map[string]Check)
for k, v := range s.checks {
checks[k] = v
}
maps.Copy(checks, s.checks)
s.mu.RUnlock()
if !ready {
+11 -11
View File
@@ -49,7 +49,7 @@ func TestReleaseAll(t *testing.T) {
paths := make([]string, 3)
refs := make([]string, 3)
for i := 0; i < 3; i++ {
for i := range 3 {
paths[i] = createTempFile(t, dir, strings.Repeat("a", i+1)+".jpg")
var err error
refs[i], err = store.Store(paths[i], MediaMeta{Source: "test"}, "scope1")
@@ -228,12 +228,12 @@ func TestConcurrentSafety(t *testing.T) {
var wg sync.WaitGroup
wg.Add(goroutines)
for g := 0; g < goroutines; g++ {
for g := range goroutines {
go func(gIdx int) {
defer wg.Done()
scope := strings.Repeat("s", gIdx+1)
for i := 0; i < filesPerGoroutine; i++ {
for i := range filesPerGoroutine {
path := createTempFile(t, dir, strings.Repeat("f", gIdx*filesPerGoroutine+i+1)+".tmp")
ref, err := store.Store(path, MediaMeta{Source: "test"}, scope)
if err != nil {
@@ -448,11 +448,11 @@ func TestConcurrentCleanupSafety(t *testing.T) {
wg.Add(workers * 4)
// Store workers
for w := 0; w < workers; w++ {
for w := range workers {
go func(wIdx int) {
defer wg.Done()
scope := fmt.Sprintf("scope-%d", wIdx)
for i := 0; i < ops; i++ {
for i := range ops {
p := createTempFile(t, dir, fmt.Sprintf("w%d-f%d.tmp", wIdx, i))
store.Store(p, MediaMeta{Source: "test"}, scope)
}
@@ -460,30 +460,30 @@ func TestConcurrentCleanupSafety(t *testing.T) {
}
// Resolve workers
for w := 0; w < workers; w++ {
for range workers {
go func() {
defer wg.Done()
for i := 0; i < ops; i++ {
for range ops {
store.Resolve("media://nonexistent")
}
}()
}
// ReleaseAll workers
for w := 0; w < workers; w++ {
for w := range workers {
go func(wIdx int) {
defer wg.Done()
for i := 0; i < ops; i++ {
for range ops {
store.ReleaseAll(fmt.Sprintf("scope-%d", wIdx))
}
}(w)
}
// CleanExpired workers
for w := 0; w < workers; w++ {
for range workers {
go func() {
defer wg.Done()
for i := 0; i < ops; i++ {
for range ops {
store.CleanExpired()
}
}()
+5 -5
View File
@@ -212,14 +212,14 @@ func translateTools(tools []ToolDefinition) []anthropic.ToolUnionParam {
}
func parseResponse(resp *anthropic.Message) *LLMResponse {
var content string
var content strings.Builder
var toolCalls []ToolCall
for _, block := range resp.Content {
switch block.Type {
case "text":
tb := block.AsText()
content += tb.Text
content.WriteString(tb.Text)
case "tool_use":
tu := block.AsToolUse()
var args map[string]any
@@ -246,7 +246,7 @@ func parseResponse(resp *anthropic.Message) *LLMResponse {
}
return &LLMResponse{
Content: content,
Content: content.String(),
ToolCalls: toolCalls,
FinishReason: finishReason,
Usage: &UsageInfo{
@@ -264,8 +264,8 @@ func normalizeBaseURL(apiBase string) string {
}
base = strings.TrimRight(base, "/")
if strings.HasSuffix(base, "/v1") {
base = strings.TrimSuffix(base, "/v1")
if before, ok := strings.CutSuffix(base, "/v1"); ok {
base = before
}
if base == "" {
return defaultBaseURL
+2 -2
View File
@@ -163,8 +163,8 @@ func resolveCodexModel(model string) (string, string) {
return codexDefaultModel, "empty model"
}
if strings.HasPrefix(m, "openai/") {
m = strings.TrimPrefix(m, "openai/")
if after, ok := strings.CutPrefix(m, "openai/"); ok {
m = after
} else if strings.Contains(m, "/") {
return codexDefaultModel, "non-openai model namespace"
}
+2 -2
View File
@@ -138,7 +138,7 @@ func TestCooldown_FailureWindowReset(t *testing.T) {
ct, current := newTestTracker(now)
// 4 errors → 1h cooldown
for i := 0; i < 4; i++ {
for range 4 {
ct.MarkFailure("openai", FailoverRateLimit)
*current = current.Add(2 * time.Second) // small advance between errors
}
@@ -230,7 +230,7 @@ func TestCooldown_ConcurrentAccess(t *testing.T) {
ct := NewCooldownTracker()
var wg sync.WaitGroup
for i := 0; i < 100; i++ {
for range 100 {
wg.Add(3)
go func() {
defer wg.Done()
+4 -4
View File
@@ -312,8 +312,8 @@ func stripSystemParts(messages []Message) []openaiMessage {
}
func normalizeModel(model, apiBase string) string {
idx := strings.Index(model, "/")
if idx == -1 {
before, after, ok := strings.Cut(model, "/")
if !ok {
return model
}
@@ -321,10 +321,10 @@ func normalizeModel(model, apiBase string) string {
return model
}
prefix := strings.ToLower(model[:idx])
prefix := strings.ToLower(before)
switch prefix {
case "moonshot", "nvidia", "groq", "ollama", "deepseek", "google", "openrouter", "zhipu", "mistral":
return model[idx+1:]
return after
default:
return model
}
+8 -5
View File
@@ -1,6 +1,9 @@
package routing
import "testing"
import (
"strings"
"testing"
)
func TestNormalizeAgentID_Empty(t *testing.T) {
if got := NormalizeAgentID(""); got != DefaultAgentID {
@@ -57,11 +60,11 @@ func TestNormalizeAgentID_AllInvalid(t *testing.T) {
}
func TestNormalizeAgentID_TruncatesAt64(t *testing.T) {
long := ""
for i := 0; i < 100; i++ {
long += "a"
var long strings.Builder
for range 100 {
long.WriteString("a")
}
got := NormalizeAgentID(long)
got := NormalizeAgentID(long.String())
if len(got) > MaxAgentIDLength {
t.Errorf("length = %d, want <= %d", len(got), MaxAgentIDLength)
}
+1 -1
View File
@@ -240,7 +240,7 @@ func (sl *SkillsLoader) parseSimpleYAML(content string) map[string]string {
normalized := strings.ReplaceAll(content, "\r\n", "\n")
normalized = strings.ReplaceAll(normalized, "\r", "\n")
for _, line := range strings.Split(normalized, "\n") {
for line := range strings.SplitSeq(normalized, "\n") {
line = strings.TrimSpace(line)
if line == "" || strings.HasPrefix(line, "#") {
continue
+2 -2
View File
@@ -1,7 +1,7 @@
package skills
import (
"sort"
"slices"
"strings"
"sync"
"time"
@@ -183,7 +183,7 @@ func buildTrigrams(s string) []uint32 {
}
// Sort and Deduplication
sort.Slice(trigrams, func(i, j int) bool { return trigrams[i] < trigrams[j] })
slices.Sort(trigrams)
n := 1
for i := 1; i < len(trigrams); i++ {
if trigrams[i] != trigrams[i-1] {
+2 -2
View File
@@ -153,7 +153,7 @@ func TestSearchCacheConcurrency(t *testing.T) {
// Concurrent writes
go func() {
for i := 0; i < 100; i++ {
for i := range 100 {
cache.Put("query-write-"+string(rune('a'+i%26)), []SearchResult{{Slug: "x"}})
}
done <- struct{}{}
@@ -161,7 +161,7 @@ func TestSearchCacheConcurrency(t *testing.T) {
// Concurrent reads
go func() {
for i := 0; i < 100; i++ {
for range 100 {
cache.Get("query-write-a")
}
done <- struct{}{}
+2 -2
View File
@@ -135,7 +135,7 @@ func TestConcurrentAccess(t *testing.T) {
// Test concurrent writes
done := make(chan bool, 10)
for i := 0; i < 10; i++ {
for i := range 10 {
go func(idx int) {
channel := fmt.Sprintf("channel-%d", idx)
sm.SetLastChannel(channel)
@@ -144,7 +144,7 @@ func TestConcurrentAccess(t *testing.T) {
}
// Wait for all goroutines to complete
for i := 0; i < 10; i++ {
for range 10 {
<-done
}
+5 -3
View File
@@ -3,6 +3,7 @@ package tools
import (
"context"
"fmt"
"strings"
"sync"
"time"
@@ -222,7 +223,8 @@ func (t *CronTool) listJobs() *ToolResult {
return SilentResult("No scheduled jobs")
}
result := "Scheduled jobs:\n"
var result strings.Builder
result.WriteString("Scheduled jobs:\n")
for _, j := range jobs {
var scheduleInfo string
if j.Schedule.Kind == "every" && j.Schedule.EveryMS != nil {
@@ -234,10 +236,10 @@ func (t *CronTool) listJobs() *ToolResult {
} else {
scheduleInfo = "unknown"
}
result += fmt.Sprintf("- %s (id: %s, %s)\n", j.Name, j.ID, scheduleInfo)
result.WriteString(fmt.Sprintf("- %s (id: %s, %s)\n", j.Name, j.ID, scheduleInfo))
}
return SilentResult(result)
return SilentResult(result.String())
}
func (t *CronTool) removeJob(args map[string]any) *ToolResult {
+1 -1
View File
@@ -329,7 +329,7 @@ func TestToolRegistry_ConcurrentAccess(t *testing.T) {
r := NewToolRegistry()
var wg sync.WaitGroup
for i := 0; i < 50; i++ {
for i := range 50 {
wg.Add(1)
go func(n int) {
defer wg.Done()
+4 -4
View File
@@ -284,7 +284,7 @@ func (p *DuckDuckGoSearchProvider) extractResults(html string, count int, query
maxItems := min(len(matches), count)
for i := 0; i < maxItems; i++ {
for i := range maxItems {
urlStr := matches[i][1]
title := stripTags(matches[i][2])
title = strings.TrimSpace(title)
@@ -292,9 +292,9 @@ func (p *DuckDuckGoSearchProvider) extractResults(html string, count int, query
// URL decoding if needed
if strings.Contains(urlStr, "uddg=") {
if u, err := url.QueryUnescape(urlStr); err == nil {
idx := strings.Index(u, "uddg=")
if idx != -1 {
urlStr = u[idx+5:]
_, after, ok := strings.Cut(u, "uddg=")
if ok {
urlStr = after
}
}
}