diff --git a/CLAUDE.md b/CLAUDE.md index b9fb0b5e..45df479b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -188,25 +188,39 @@ Published to GHCR (`ghcr.io/nextlevelbuilder/goclaw`) and Docker Hub (`digitop/g Apply before finalizing any multi-phase plan. Trust-but-verify between scout → planner → final plan. +### Verification discipline (what to verify) + 1. **Verify factual claims against code** — re-grep/re-count every number, path, endpoint. Don't copy from scout summaries. -2. **Re-scout on scope change** — if phase promotes from deferred → active, re-scout. Don't reuse brainstorm summary. -3. **Grep delete scope deep** — `grep -rn '' .` whole repo. Stubs often have refs in catalogs/routing/switch cases. Enumerate ALL sites in todo. -4. **Verify external API endpoints via `docs-seeker`** — before writing endpoint into plan. Sibling APIs often use different roots. -5. **Cross-phase gates explicit** — "Phase N-1 merged + tests green" in phase Context. Execution order alone ≠ enforcement. -6. **Scout desktop and web separately** — `ui/desktop/frontend/` ≠ `ui/web/`. Different structure, i18n namespaces, test framework presence. -7. **i18n keys ordering** — add key + 3 catalogs as explicit todo step BEFORE handler code. Missing key = runtime crash. -8. **Signature-change callers enumeration** — grep + list all callers explicitly. "Update all callers" insufficient. -9. **Zero-coverage characterization test = blocker step** — write byte/request-body fixture test BEFORE migration. Not "recommended". -10. **Context key style convention** — check existing `context.go` pattern before introducing new key types. Mixed = code smell. -11. **Alias/shim coverage** — enumerate ALL exported symbols via `go doc `. Add compile-time signature guards. -12. **Verify pass MANDATORY after rewrite** — spawn fresh Explore/grep to audit planner output. Don't trust self-validation. -13. **No fabricated Go identifiers** — every type/function/package referenced in plan must be grep-verified to exist (`grep -rn '^func \|^type ' `). Plausible-sounding APIs (`Keyring`, `StartSpan`, `Validator`, `security.SSRF`) are RED FLAGS — conventions differ per codebase. Rule of thumb: if you can't cite `file_path:line_number` for a symbol, you're fabricating. Apply especially when planner says "reuse existing X" — re-verify X exists before writing into plan. -14. **Plausible-but-wrong API families** — watch for: OTel-style `StartSpan/EndSpan` (codebase may be emit-based); wrapper types like `Keyring/Vault/Manager` (codebase may use free functions); `internal/security` or `internal/auth` packages (often scattered instead of centralized). When unsure, `go doc ` lists actual exported surface. +2. **Trace semantics, not just cite lines** — when plan references existing/upstream code, identify WHEN each field mutates and under WHAT conditions. Line-range citation without control-flow trace = how ports silently invert behavior. Check: every call, or specific branches only? +3. **No fabricated identifiers / API families** — every symbol in plan must cite `file:line`. RED FLAGS: plausible-sounding wrappers (`Keyring`, `Validator`, `Manager`), centralized packages (`internal/security`, `internal/auth`) that may be scattered, OTel-style (`StartSpan/EndSpan`) when codebase is emit-based. When unsure, `go doc ` lists actual exported surface. Apply especially when plan says "reuse existing X". +4. **Struct scope audit before adding state** — verify lifetime (per-request/session/agent/process) before adding a field to an existing struct. "Plausibly per-X" is a red flag — grep construction + ownership. Shared-instance state leaks across isolation boundaries. +5. **Gate-premise test math** — before asserting "feature X triggers independently of Y", list all early-returns from function entry to X. Math-verify any fixture claiming "X without Y". +6. **Port = config-shape match** — "faithful port" divergences in config field name/type are silent breaking changes for users copying upstream config. Match upstream shape, or explicitly flag each divergence with rationale in the phase file. +7. **Verify external API endpoints via `docs-seeker`** — before writing endpoint into plan. Sibling APIs often use different roots. + +### Scope & coverage (where to look) + +8. **Grep delete scope deep** — `grep -rn '' .` whole repo. Stubs often have refs in catalogs/routing/switch cases. Enumerate ALL sites in todo. +9. **Signature-change callers enumeration** — grep + list all callers explicitly. "Update all callers" insufficient. +10. **Alias/shim coverage** — enumerate ALL exported symbols via `go doc `. Add compile-time signature guards. +11. **Scout desktop and web separately** — `ui/desktop/frontend/` ≠ `ui/web/`. Different structure, i18n namespaces, test framework presence. + +### Phasing & ordering (when) + +12. **Re-scout on scope change** — if phase promotes from deferred → active, re-scout. Don't reuse brainstorm summary. +13. **Cross-phase gates explicit** — "Phase N-1 merged + tests green" in phase Context. Execution order alone ≠ enforcement. +14. **Zero-coverage characterization test = blocker step** — write byte/request-body fixture test BEFORE migration. Not "recommended". +15. **i18n keys ordering** — add key + 3 catalogs as explicit todo step BEFORE handler code. Missing key = runtime crash. + +### Conventions & finalization + +16. **Context key style convention** — check existing `context.go` pattern before introducing new key types. Mixed = code smell. +17. **Verify pass MANDATORY after rewrite** — spawn fresh Explore/grep to audit planner output. Don't trust self-validation. **Pattern to avoid:** user asks → planner writes → report "done". **Safer pattern:** user asks → scout → planner writes → audit-verify → report. -**Concrete red-team practice:** After planner completes, run `code-reviewer`/`brainstormer` agent in audit mode with explicit instruction "spot-check 15+ factual claims against live codebase". Caught fabricated `internal/security/`, `crypto.Keyring`, `tracing.StartSpan` in Agent Hooks plan (see `plans/260414-2229-agent-hooks-system/reports/audit-260414-2310-plan-review.md`). +**Red-team practice:** After planner completes, run `code-reviewer`/`brainstormer` in audit mode: "spot-check 15+ claims vs live codebase". Past catches: fabricated `crypto.Keyring`/`tracing.StartSpan` (agent-hooks plan); inverted TS-port semantics + wrong struct scope + misread early-return gate (context-pruning plan). See `plans/*/reports/audit-*.md` for concrete examples. ## Post-Implementation Checklist diff --git a/cmd/gateway_consumer_handlers.go b/cmd/gateway_consumer_handlers.go index 3be0236b..10a6b54f 100644 --- a/cmd/gateway_consumer_handlers.go +++ b/cmd/gateway_consumer_handlers.go @@ -129,16 +129,14 @@ func handleSubagentAnnounce( // Enqueue into producer-consumer queue using tenant-scoped key from routing. isProcessor := enqueueSubagentAnnounce(queueKey, entry) if isProcessor { - deps.BgWg.Add(1) - go func() { - defer deps.BgWg.Done() + deps.BgWg.Go(func() { defer safego.Recover(nil, "component", "subagent_announce_loop", "session", sessionKey) // Fetch live roster for merged announce context. roster := deps.SubagentMgr.RosterForParent(parentAgent) processSubagentAnnounceLoop(ctx, routing, roster, deps.SubagentMgr, deps.Sched, deps.MsgBus, deps.Cfg) - }() + }) } return true diff --git a/cmd/gateway_setup.go b/cmd/gateway_setup.go index af0f721c..730f7a8f 100644 --- a/cmd/gateway_setup.go +++ b/cmd/gateway_setup.go @@ -45,6 +45,7 @@ func setupToolRegistry( browserMgr *browser.Manager, webFetchTool *tools.WebFetchTool, ttsTool *tools.TtsTool, + audioMgr *audio.Manager, permPE *permissions.PolicyEngine, toolPE *tools.PolicyEngine, dataDir string, @@ -142,6 +143,7 @@ func setupToolRegistry( } setupAudioExtras(cfg, ttsMgr) // Phase 3: registers Music + SFX providers. audio.BridgeLegacySTT(ttsMgr, cfg) // Phase 4: bridge per-channel STTProxyURL → channel-scoped providers. + audioMgr = ttsMgr // expose to caller for channel STT wiring (Phase 5) // Audio generation tool — backed by audio.Manager (Music + SFX). toolsReg.Register(tools.NewCreateAudioTool(ttsMgr)) diff --git a/cmd/gateway_setup_exec_workspace_test.go b/cmd/gateway_setup_exec_workspace_test.go index 6fc79ba8..cd210c92 100644 --- a/cmd/gateway_setup_exec_workspace_test.go +++ b/cmd/gateway_setup_exec_workspace_test.go @@ -22,7 +22,7 @@ func testExecToolFromGatewaySetup(t *testing.T, workspace, dataDir string) *tool cfg.Tools.Browser.Enabled = false providerRegistry := providers.NewRegistry(nil) - toolsReg, _, _, _, _, _, _, _, _, _, _ := setupToolRegistry(cfg, workspace, providerRegistry) + toolsReg, _, _, _, _, _, _, _, _, _, _, _ := setupToolRegistry(cfg, workspace, providerRegistry) execToolAny, ok := toolsReg.Get("exec") if !ok { diff --git a/internal/agent/loop_bench_test.go b/internal/agent/loop_bench_test.go index 14e1bee6..72e13254 100644 --- a/internal/agent/loop_bench_test.go +++ b/internal/agent/loop_bench_test.go @@ -91,7 +91,7 @@ func BenchmarkSanitizeHistory_WithToolCalls(b *testing.B) { } // Repeat to make it longer - for i := 0; i < 20; i++ { + for range 20 { msgs = append(msgs, msgs[:7]...) } @@ -118,7 +118,7 @@ func BenchmarkSanitizeHistory_WithMissingToolResults(b *testing.B) { } // Repeat to make it longer - for i := 0; i < 25; i++ { + for range 25 { msgs = append(msgs, msgs[:5]...) } @@ -144,7 +144,7 @@ func BenchmarkSanitizeHistory_ConsecutiveSameRole(b *testing.B) { } // Repeat to make it longer - for i := 0; i < 30; i++ { + for range 30 { msgs = append(msgs, msgs[:8]...) } @@ -161,7 +161,7 @@ func makeHistoryMessages(pairs int) []providers.Message { msgs := make([]providers.Message, 0, pairs*2) content := strings.Repeat("message content ", 10) - for i := 0; i < pairs; i++ { + for i := range pairs { // Add user message msgs = append(msgs, providers.Message{ Role: "user", diff --git a/internal/agent/router_abort_test.go b/internal/agent/router_abort_test.go index 2114e6dd..ff86359d 100644 --- a/internal/agent/router_abort_test.go +++ b/internal/agent/router_abort_test.go @@ -137,7 +137,7 @@ func TestAbortRun_AlreadyAborting(t *testing.T) { results := make([]AbortResult, n) var wg sync.WaitGroup wg.Add(n) - for i := 0; i < n; i++ { + for i := range n { i := i go func() { defer wg.Done() @@ -213,7 +213,7 @@ func TestAbortRun_AfterUnregister(t *testing.T) { // TestAbortRun_Race_UnregisterConcurrent verifies no panic and no goroutine leak // when UnregisterRun and AbortRun interleave across 100 iterations. func TestAbortRun_Race_UnregisterConcurrent(t *testing.T) { - for i := 0; i < 100; i++ { + for range 100 { r := NewRouter() runID := "run-race" sessionKey := "session-race" diff --git a/internal/audio/voice_cache_test.go b/internal/audio/voice_cache_test.go index f62a49c5..89682ded 100644 --- a/internal/audio/voice_cache_test.go +++ b/internal/audio/voice_cache_test.go @@ -47,7 +47,7 @@ func TestVoiceCache_LRUEviction(t *testing.T) { voices := []audio.Voice{{ID: "x"}} // Fill to capacity - for i := 0; i < cap; i++ { + for i := range cap { c.Set(ids[i], voices) } // Access ids[0] to make it recently used @@ -83,15 +83,13 @@ func TestVoiceCache_Invalidate(t *testing.T) { func TestVoiceCache_ConcurrentSafe(t *testing.T) { c := audio.NewVoiceCache(time.Hour, 500) var wg sync.WaitGroup - for i := 0; i < 100; i++ { - wg.Add(1) - go func() { - defer wg.Done() + for range 100 { + wg.Go(func() { tid := uuid.New() c.Set(tid, []audio.Voice{{ID: "v"}}) c.Get(tid) c.Invalidate(tid) - }() + }) } wg.Wait() } diff --git a/internal/backup/db_dump.go b/internal/backup/db_dump.go index 91c26075..14580b9f 100644 --- a/internal/backup/db_dump.go +++ b/internal/backup/db_dump.go @@ -84,11 +84,11 @@ func PgDumpVersion(ctx context.Context) (string, error) { // "pg_dump (PostgreSQL) 18.3" -> 18 func ParsePgDumpMajor(version string) int { const marker = "(PostgreSQL) " - idx := strings.Index(version, marker) - if idx < 0 { + _, after, ok := strings.Cut(version, marker) + if !ok { return 0 } - rest := version[idx+len(marker):] + rest := after end := 0 for end < len(rest) && rest[end] >= '0' && rest[end] <= '9' { end++ diff --git a/internal/cache/memory.go b/internal/cache/memory.go index 6e673370..bb1beec8 100644 --- a/internal/cache/memory.go +++ b/internal/cache/memory.go @@ -169,10 +169,9 @@ func (c *InMemoryCache[V]) sweepOnce() { sort.Slice(allAlive, func(i, j int) bool { return allAlive[i].createdAt.Before(allAlive[j].createdAt) }) - toEvict := len(allAlive) - c.maxSize + (c.maxSize / 5) // bring below cap + 20% headroom - if toEvict > len(allAlive) { - toEvict = len(allAlive) - } + toEvict := min( + // bring below cap + 20% headroom + len(allAlive)-c.maxSize+(c.maxSize/5), len(allAlive)) for i := 0; i < toEvict; i++ { c.data.Delete(allAlive[i].key) } diff --git a/internal/cache/memory_test.go b/internal/cache/memory_test.go index 0788372e..7e8c3e62 100644 --- a/internal/cache/memory_test.go +++ b/internal/cache/memory_test.go @@ -95,7 +95,7 @@ func TestInMemoryCache_Clear(t *testing.T) { // the background sweep goroutine (not just lazy on Get). func TestInMemoryCache_PeriodicSweep(t *testing.T) { c := NewInMemoryCache[string]( - WithSweepInterval[string](20*time.Millisecond), + WithSweepInterval[string](20 * time.Millisecond), ) defer c.Close() ctx := context.Background() @@ -126,7 +126,7 @@ func TestInMemoryCache_MaxSizeEviction(t *testing.T) { ctx := context.Background() // Insert 10 entries with distinct creation times to ensure oldest-first ordering - for i := 0; i < 10; i++ { + for i := range 10 { c.Set(ctx, string(rune('a'+i)), i, 0) time.Sleep(2 * time.Millisecond) } @@ -142,7 +142,7 @@ func TestInMemoryCache_MaxSizeEviction(t *testing.T) { // TestInMemoryCache_Close verifies Close stops the sweep goroutine (no leak). func TestInMemoryCache_Close(t *testing.T) { c := NewInMemoryCache[string]( - WithSweepInterval[string](10*time.Millisecond), + WithSweepInterval[string](10 * time.Millisecond), ) ctx := context.Background() c.Set(ctx, "k", "v", 0) @@ -168,13 +168,13 @@ func TestInMemoryCache_ConcurrentSweepAndSet(t *testing.T) { done := make(chan bool) go func() { - for i := 0; i < 500; i++ { + for i := range 500 { c.Set(ctx, string(rune('a'+(i%26))), i, 5*time.Millisecond) } done <- true }() go func() { - for i := 0; i < 500; i++ { + for i := range 500 { _, _ = c.Get(ctx, string(rune('a'+(i%26)))) } done <- true diff --git a/internal/channels/facebook/graph_api.go b/internal/channels/facebook/graph_api.go index 10ab11ff..b9bb93fb 100644 --- a/internal/channels/facebook/graph_api.go +++ b/internal/channels/facebook/graph_api.go @@ -189,7 +189,7 @@ var graphBackoffBase = 1 * time.Second func (g *GraphClient) doRequest(ctx context.Context, method, path string, body any) ([]byte, error) { apiURL := fmt.Sprintf("%s/%s%s", graphAPIBase, graphAPIVersion, path) - for attempt := 0; attempt < maxRetries; attempt++ { + for attempt := range maxRetries { if attempt > 0 { backoff := time.Duration(1< 0 { diff --git a/internal/gateway/ratelimit_test.go b/internal/gateway/ratelimit_test.go index 2d02fa8b..1ad03dd0 100644 --- a/internal/gateway/ratelimit_test.go +++ b/internal/gateway/ratelimit_test.go @@ -10,7 +10,7 @@ func TestRateLimiter_DisabledWhenZeroRPM(t *testing.T) { if rl.Enabled() { t.Fatal("expected disabled when rpm=0") } - for i := 0; i < 100; i++ { + for range 100 { if !rl.Allow("key") { t.Fatal("expected allow when disabled") } @@ -43,7 +43,7 @@ func TestRateLimiter_BurstAllowed(t *testing.T) { rl := NewRateLimiter(1, burst) key := "test-burst" allowed := 0 - for i := 0; i < burst; i++ { + for range burst { if rl.Allow(key) { allowed++ } @@ -97,7 +97,7 @@ func TestRateLimiter_DefaultBurstWhenZero(t *testing.T) { key := "burst-default" // Should be able to consume at least 5 tokens from the default burst. allowed := 0 - for i := 0; i < 5; i++ { + for range 5 { if rl.Allow(key) { allowed++ } diff --git a/internal/heartbeat/ticker_test.go b/internal/heartbeat/ticker_test.go index 21d3695b..7f7ec9ad 100644 --- a/internal/heartbeat/ticker_test.go +++ b/internal/heartbeat/ticker_test.go @@ -76,10 +76,10 @@ func TestDeref(t *testing.T) { expect string }{ {"nil_pointer", nil, ""}, - {"empty_string", strPtr(""), ""}, - {"normal_string", strPtr("hello"), "hello"}, - {"whitespace", strPtr(" "), " "}, - {"unicode_string", strPtr("你好"), "你好"}, + {"empty_string", new(""), ""}, + {"normal_string", new("hello"), "hello"}, + {"whitespace", new(" "), " "}, + {"unicode_string", new("你好"), "你好"}, } for _, tt := range tests { @@ -94,17 +94,17 @@ func TestDeref(t *testing.T) { func TestProcessResponse(t *testing.T) { tests := []struct { - name string - response string - maxChars int + name string + response string + maxChars int expectDeliver bool - description string + description string }{ { - name: "empty_response", - response: "", + name: "empty_response", + response: "", expectDeliver: true, - description: "empty response should deliver (no HEARTBEAT_OK token)", + description: "empty response should deliver (no HEARTBEAT_OK token)", }, { name: "heartbeat_ok_only", @@ -119,16 +119,16 @@ func TestProcessResponse(t *testing.T) { description: "HEARTBEAT_OK anywhere in response suppresses delivery", }, { - name: "normal_content", - response: "System check: CPU 45%, Memory 60%, Disk 70%", + name: "normal_content", + response: "System check: CPU 45%, Memory 60%, Disk 70%", expectDeliver: true, - description: "normal content without HEARTBEAT_OK is delivered", + description: "normal content without HEARTBEAT_OK is delivered", }, { - name: "ok_token_case_sensitive", - response: "heartbeat_ok Status is good", + name: "ok_token_case_sensitive", + response: "heartbeat_ok Status is good", expectDeliver: true, - description: "HEARTBEAT_OK is case-sensitive, lowercase should not suppress", + description: "HEARTBEAT_OK is case-sensitive, lowercase should not suppress", }, { name: "heartbeat_ok_at_start", @@ -143,10 +143,10 @@ func TestProcessResponse(t *testing.T) { description: "HEARTBEAT_OK at end suppresses", }, { - name: "multiline_without_token", - response: "Line 1\nLine 2\nLine 3", + name: "multiline_without_token", + response: "Line 1\nLine 2\nLine 3", expectDeliver: true, - description: "multiline without HEARTBEAT_OK is delivered", + description: "multiline without HEARTBEAT_OK is delivered", }, { name: "multiline_with_token", @@ -155,10 +155,10 @@ func TestProcessResponse(t *testing.T) { description: "HEARTBEAT_OK in middle of multiline suppresses", }, { - name: "long_response", - response: "This is a very long response with lots of details about the system status, but no special tokens", + name: "long_response", + response: "This is a very long response with lots of details about the system status, but no special tokens", expectDeliver: true, - description: "long response without token is delivered", + description: "long response without token is delivered", }, } @@ -173,8 +173,9 @@ func TestProcessResponse(t *testing.T) { } } +//go:fix inline func strPtr(s string) *string { - return &s + return new(s) } func TestIsWithinActiveHours_NoConfig(t *testing.T) { @@ -192,8 +193,8 @@ func TestIsWithinActiveHours_NoConfig(t *testing.T) { func TestIsWithinActiveHours_EmptyConfig(t *testing.T) { // When active hours are empty strings, should always return true hb := &store.AgentHeartbeat{ - ActiveHoursStart: strPtr(""), - ActiveHoursEnd: strPtr(""), + ActiveHoursStart: new(""), + ActiveHoursEnd: new(""), } got := isWithinActiveHours(*hb) if !got { @@ -219,11 +220,11 @@ func TestIsWithinActiveHours_SimpleWindow(t *testing.T) { } tests := []struct { - name string - hour int - minute int - startStr string - endStr string + name string + hour int + minute int + startStr string + endStr string expectInside bool }{ // Within window @@ -270,9 +271,9 @@ func TestIsWithinActiveHours_MidnightWrap(t *testing.T) { } tests := []struct { - name string - hour int - minute int + name string + hour int + minute int expectInside bool }{ {"midnight_10pm", 22, 0, true}, // 22:00 is in window @@ -302,5 +303,5 @@ func formatInt(i int) string { if i < 10 { return "0" + string(rune('0'+i)) } - return string(rune('0' + i/10)) + string(rune('0'+i%10)) + return string(rune('0'+i/10)) + string(rune('0'+i%10)) } diff --git a/internal/http/builtin_tools_tenant_settings_test.go b/internal/http/builtin_tools_tenant_settings_test.go index 273ee75d..f6e8a2fa 100644 --- a/internal/http/builtin_tools_tenant_settings_test.go +++ b/internal/http/builtin_tools_tenant_settings_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "io" + "maps" "net/http" "net/http/httptest" "strings" @@ -130,9 +131,7 @@ func (s *stubTenantCfgStore) ListAll(_ context.Context, tid uuid.UUID) (map[stri s.mu.Lock() defer s.mu.Unlock() out := make(map[string]bool, len(s.enabled)) - for k, v := range s.enabled { - out[k] = v - } + maps.Copy(out, s.enabled) return out, nil } diff --git a/internal/http/skills_upload.go b/internal/http/skills_upload.go index 909c8f42..b7af06bb 100644 --- a/internal/http/skills_upload.go +++ b/internal/http/skills_upload.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "log/slog" + "maps" "net/http" "os" "path/filepath" @@ -232,9 +233,7 @@ func (h *SkillsHandler) handleUpload(w http.ResponseWriter, r *http.Request) { ) skill.Status = depState.status skill.MissingDeps = depState.missing - for key, value := range depState.response { - response[key] = value - } + maps.Copy(response, depState.response) } } diff --git a/internal/http/skills_upload_test.go b/internal/http/skills_upload_test.go index ec00067a..58724220 100644 --- a/internal/http/skills_upload_test.go +++ b/internal/http/skills_upload_test.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "encoding/json" + "maps" "mime/multipart" "net/http" "net/http/httptest" @@ -365,12 +366,12 @@ func skillMarkdown(name, slug string) string { } type skillManageStoreStub struct { - baseDir string - version int64 - nextBySlug map[string]int - skills map[uuid.UUID]store.SkillInfo - systemDirs map[string]string - hashBySlug map[string]string // slug -> SKILL.md content hash (most recent) + baseDir string + version int64 + nextBySlug map[string]int + skills map[uuid.UUID]store.SkillInfo + systemDirs map[string]string + hashBySlug map[string]string // slug -> SKILL.md content hash (most recent) } func newSkillManageStoreStub(baseDir string) *skillManageStoreStub { @@ -515,9 +516,7 @@ func (s *skillManageStoreStub) ListAllSystemSkills(context.Context) []store.Skil } func (s *skillManageStoreStub) ListSystemSkillDirs(context.Context) map[string]string { out := make(map[string]string, len(s.systemDirs)) - for slug, dir := range s.systemDirs { - out[slug] = dir - } + maps.Copy(out, s.systemDirs) return out } func (s *skillManageStoreStub) StoreMissingDeps(_ context.Context, id uuid.UUID, missing []string) error { diff --git a/internal/knowledgegraph/extractor_helpers_test.go b/internal/knowledgegraph/extractor_helpers_test.go index ce2af5a9..b06992fc 100644 --- a/internal/knowledgegraph/extractor_helpers_test.go +++ b/internal/knowledgegraph/extractor_helpers_test.go @@ -125,7 +125,7 @@ func TestSplitChunks_Empty(t *testing.T) { func TestSplitChunks_MultipleChunks(t *testing.T) { // 5 paragraphs of 40 chars each; max=50 → multiple chunks. var paras []string - for i := 0; i < 5; i++ { + for i := range 5 { paras = append(paras, strings.Repeat(fmt.Sprintf("%d", i), 40)) } text := strings.Join(paras, "\n\n") @@ -341,7 +341,7 @@ func TestExtract_LongText_SplitsIntoChunks(t *testing.T) { // Build text longer than maxChunkChars (12000) with paragraph breaks. para := strings.Repeat("word ", 200) // ~1000 chars per para var paras []string - for i := 0; i < 15; i++ { + for range 15 { paras = append(paras, para) } longText := strings.Join(paras, "\n\n") // ~15000+ chars diff --git a/internal/providers/acp/helpers_test.go b/internal/providers/acp/helpers_test.go index 41d73da8..1bee1d7b 100644 --- a/internal/providers/acp/helpers_test.go +++ b/internal/providers/acp/helpers_test.go @@ -209,12 +209,10 @@ func TestLimitedWriter_ZeroMax(t *testing.T) { func TestLimitedWriter_ConcurrentWrites(t *testing.T) { lw := &limitedWriter{max: 1000} var wg sync.WaitGroup - for i := 0; i < 20; i++ { - wg.Add(1) - go func() { - defer wg.Done() + for range 20 { + wg.Go(func() { lw.Write([]byte("data")) - }() + }) } wg.Wait() // No panic and output <= max diff --git a/internal/providers/acp/jsonrpc_test.go b/internal/providers/acp/jsonrpc_test.go index 98fb3580..239ca01c 100644 --- a/internal/providers/acp/jsonrpc_test.go +++ b/internal/providers/acp/jsonrpc_test.go @@ -470,13 +470,6 @@ func TestConn_ReadLoop_MalformedInputNoPanic(t *testing.T) { } } -func min(a, b int) int { - if a < b { - return a - } - return b -} - func TestConn_IDIncrement(t *testing.T) { // Verify each Call gets a unique ID by capturing two requests clientR, serverW := io.Pipe() @@ -489,7 +482,7 @@ func TestConn_IDIncrement(t *testing.T) { // Goroutine that reads requests and responds go func() { - for i := 0; i < 2; i++ { + for range 2 { buf := make([]byte, 4096) n, _ := serverR.Read(buf) var req jsonrpcMessage diff --git a/internal/providers/acp/process_test.go b/internal/providers/acp/process_test.go index e58b6f49..317f2bdb 100644 --- a/internal/providers/acp/process_test.go +++ b/internal/providers/acp/process_test.go @@ -91,7 +91,7 @@ func TestProcessPool_Close_WithFakeProcesses(t *testing.T) { pp := NewProcessPool("x", nil, "", time.Minute) // Insert fake ACPProcess entries that are already exited - for i := 0; i < 3; i++ { + for i := range 3 { exitedCh := make(chan struct{}) close(exitedCh) // pre-exited proc := &ACPProcess{ diff --git a/internal/providers/acp/terminal_test.go b/internal/providers/acp/terminal_test.go index 61378863..61cb9d80 100644 --- a/internal/providers/acp/terminal_test.go +++ b/internal/providers/acp/terminal_test.go @@ -76,7 +76,7 @@ func TestCappedBuffer_OverflowExceedsExistingBuf(t *testing.T) { func TestCappedBuffer_MultipleSmallWrites(t *testing.T) { cb := &cappedBuffer{max: 10} - for i := 0; i < 5; i++ { + for range 5 { cb.Write([]byte("ab")) } got := cb.String() @@ -101,12 +101,10 @@ func TestCappedBuffer_ZeroMax(t *testing.T) { func TestCappedBuffer_ConcurrentWrites(t *testing.T) { cb := &cappedBuffer{max: 100} var wg sync.WaitGroup - for i := 0; i < 50; i++ { - wg.Add(1) - go func() { - defer wg.Done() + for range 50 { + wg.Go(func() { cb.Write([]byte("data")) - }() + }) } wg.Wait() s := cb.String() diff --git a/internal/providers/adapter_registry_test.go b/internal/providers/adapter_registry_test.go index ffe53f6d..f42cc21a 100644 --- a/internal/providers/adapter_registry_test.go +++ b/internal/providers/adapter_registry_test.go @@ -76,7 +76,7 @@ func TestAdapterRegistry_ConcurrentRegister(t *testing.T) { }) var wg sync.WaitGroup - for i := 0; i < 32; i++ { + for i := range 32 { wg.Add(2) go func(id int) { defer wg.Done() @@ -144,5 +144,5 @@ func (s *stubAdapter) Capabilities() ProviderCapabilities { return ProviderCapab func (s *stubAdapter) ToRequest(ChatRequest) ([]byte, http.Header, error) { return nil, nil, nil } -func (s *stubAdapter) FromResponse([]byte) (*ChatResponse, error) { return nil, nil } +func (s *stubAdapter) FromResponse([]byte) (*ChatResponse, error) { return nil, nil } func (s *stubAdapter) FromStreamChunk([]byte) (*StreamChunk, error) { return nil, nil } diff --git a/internal/providers/anthropic_request.go b/internal/providers/anthropic_request.go index 6e2add03..58dfb3a0 100644 --- a/internal/providers/anthropic_request.go +++ b/internal/providers/anthropic_request.go @@ -15,14 +15,14 @@ const CacheBoundaryMarker = "" // Returns 1 block with cache_control if no boundary (backwards compat). func splitSystemPromptForCache(content string) []map[string]any { ephemeral := map[string]any{"type": "ephemeral"} - idx := strings.Index(content, CacheBoundaryMarker) - if idx == -1 { + before, after, ok := strings.Cut(content, CacheBoundaryMarker) + if !ok { return []map[string]any{ {"type": "text", "text": content, "cache_control": ephemeral}, } } - stable := strings.TrimSpace(content[:idx]) - dynamic := strings.TrimSpace(content[idx+len(CacheBoundaryMarker):]) + stable := strings.TrimSpace(before) + dynamic := strings.TrimSpace(after) blocks := []map[string]any{ {"type": "text", "text": stable, "cache_control": ephemeral}, } diff --git a/internal/providers/claude_cli_mcp.go b/internal/providers/claude_cli_mcp.go index ae31e41c..df03d080 100644 --- a/internal/providers/claude_cli_mcp.go +++ b/internal/providers/claude_cli_mcp.go @@ -266,11 +266,12 @@ func sanitizePathSegment(s string) string { // Payload: agentID|userID|channel|chatID|peerKind|workspace|tenantID func SignBridgeContext(key, agentID, userID, channel, chatID, peerKind, workspace, tenantID string, extra ...string) string { mac := hmac.New(sha256.New, []byte(key)) - payload := agentID + "|" + userID + "|" + channel + "|" + chatID + "|" + peerKind + "|" + workspace + "|" + tenantID + var payload strings.Builder + payload.WriteString(agentID + "|" + userID + "|" + channel + "|" + chatID + "|" + peerKind + "|" + workspace + "|" + tenantID) for _, e := range extra { - payload += "|" + e + payload.WriteString("|" + e) } - mac.Write([]byte(payload)) + mac.Write([]byte(payload.String())) return hex.EncodeToString(mac.Sum(nil)) } diff --git a/internal/store/contact_collector_test.go b/internal/store/contact_collector_test.go index 561dedbf..5e21f2ea 100644 --- a/internal/store/contact_collector_test.go +++ b/internal/store/contact_collector_test.go @@ -92,7 +92,7 @@ func TestContactCollector_SameTenantDedup(t *testing.T) { tenant := uuid.New() ctx := WithTenantID(context.Background(), tenant) - for i := 0; i < 5; i++ { + for range 5 { c.EnsureContact(ctx, "telegram", "tg-main", "user-123", "uid-1", "Alice", "alice", "user", "user", "", "") } diff --git a/internal/store/pg/team_task_siblings.go b/internal/store/pg/team_task_siblings.go index 06cfb6fd..fa13bcdb 100644 --- a/internal/store/pg/team_task_siblings.go +++ b/internal/store/pg/team_task_siblings.go @@ -48,10 +48,7 @@ func (s *PGTeamStore) BatchGetTaskSiblingsByBasenames( const chunkSize = 500 for start := 0; start < len(clean); start += chunkSize { - end := start + chunkSize - if end > len(clean) { - end = len(clean) - } + end := min(start+chunkSize, len(clean)) chunk := clean[start:end] const q = ` diff --git a/internal/tasks/task_ticker_test.go b/internal/tasks/task_ticker_test.go index 432c6ca5..3c0b36dd 100644 --- a/internal/tasks/task_ticker_test.go +++ b/internal/tasks/task_ticker_test.go @@ -470,13 +470,11 @@ func TestTaskTicker_ConcurrentStartStop_Race(t *testing.T) { tt.Start() var wg sync.WaitGroup - for i := 0; i < 5; i++ { - wg.Add(1) - go func() { - defer wg.Done() + for range 5 { + wg.Go(func() { // Access cooldown map concurrently via pruneCooldowns (uses lock). tt.pruneCooldowns() - }() + }) } wg.Wait() tt.Stop() diff --git a/internal/tokencount/count_bench_test.go b/internal/tokencount/count_bench_test.go index 9cfbbec4..2bf61b79 100644 --- a/internal/tokencount/count_bench_test.go +++ b/internal/tokencount/count_bench_test.go @@ -67,7 +67,7 @@ func BenchmarkCountMessages(b *testing.B) { func BenchmarkCountMessages_LargeHistory(b *testing.B) { tc := NewTiktokenCounter() msgs := make([]providers.Message, 50) - for i := 0; i < 50; i++ { + for i := range 50 { role := "user" if i%2 == 1 { role = "assistant" diff --git a/internal/tools/context_keys.go b/internal/tools/context_keys.go index 8ed3022b..be554e6f 100644 --- a/internal/tools/context_keys.go +++ b/internal/tools/context_keys.go @@ -3,6 +3,7 @@ package tools import ( "context" "log/slog" + "maps" "sync" "github.com/google/uuid" @@ -255,12 +256,8 @@ func BuiltinToolSettingsFromCtx(ctx context.Context) BuiltinToolSettings { // Both tiers present: layer tenant override on top of global defaults. merged := make(BuiltinToolSettings, len(global)+len(tenant)) - for k, v := range global { - merged[k] = v - } - for k, v := range tenant { - merged[k] = v - } + maps.Copy(merged, global) + maps.Copy(merged, tenant) return merged } diff --git a/internal/tools/credentialed_exec.go b/internal/tools/credentialed_exec.go index c1327044..695e74c4 100644 --- a/internal/tools/credentialed_exec.go +++ b/internal/tools/credentialed_exec.go @@ -11,6 +11,7 @@ import ( "os" "os/exec" "regexp" + "slices" "strings" "time" @@ -189,10 +190,8 @@ func matchesBinaryVerbose(args []string, denyPatternsJSON json.RawMessage) strin slog.Warn("secure_cli.invalid_deny_pattern", "pattern", p, "error", err) continue } - for _, arg := range args { - if re.MatchString(arg) { - return p - } + if slices.ContainsFunc(args, re.MatchString) { + return p } } return "" diff --git a/internal/tools/registry_bench_test.go b/internal/tools/registry_bench_test.go index cbae69a3..48b26891 100644 --- a/internal/tools/registry_bench_test.go +++ b/internal/tools/registry_bench_test.go @@ -32,7 +32,7 @@ func (m *MockTool) Execute(ctx context.Context, args map[string]any) *Result { // BenchmarkRegistry_Get_50Tools benchmarks tool lookup in registry with 50 tools. func BenchmarkRegistry_Get_50Tools(b *testing.B) { reg := NewRegistry() - for i := 0; i < 50; i++ { + for i := range 50 { tool := &MockTool{ name: fmt.Sprintf("tool_%02d", i), description: "Mock tool for benchmarking", @@ -58,7 +58,7 @@ func BenchmarkRegistry_Get_50Tools(b *testing.B) { // BenchmarkRegistry_Get_100Tools benchmarks tool lookup in registry with 100 tools. func BenchmarkRegistry_Get_100Tools(b *testing.B) { reg := NewRegistry() - for i := 0; i < 100; i++ { + for i := range 100 { tool := &MockTool{ name: fmt.Sprintf("tool_%03d", i), description: "Mock tool for benchmarking", @@ -84,7 +84,7 @@ func BenchmarkRegistry_Get_100Tools(b *testing.B) { // BenchmarkRegistry_List_50Tools benchmarks listing tools from registry with 50 tools. func BenchmarkRegistry_List_50Tools(b *testing.B) { reg := NewRegistry() - for i := 0; i < 50; i++ { + for i := range 50 { tool := &MockTool{ name: fmt.Sprintf("tool_%02d", i), description: "Mock tool for benchmarking", @@ -102,7 +102,7 @@ func BenchmarkRegistry_List_50Tools(b *testing.B) { // BenchmarkRegistry_List_100Tools benchmarks listing tools from registry with 100 tools. func BenchmarkRegistry_List_100Tools(b *testing.B) { reg := NewRegistry() - for i := 0; i < 100; i++ { + for i := range 100 { tool := &MockTool{ name: fmt.Sprintf("tool_%03d", i), description: "Mock tool for benchmarking", @@ -120,7 +120,7 @@ func BenchmarkRegistry_List_100Tools(b *testing.B) { // BenchmarkRegistry_ProviderDefs_50Tools benchmarks generating provider definitions for 50 tools. func BenchmarkRegistry_ProviderDefs_50Tools(b *testing.B) { reg := NewRegistry() - for i := 0; i < 50; i++ { + for i := range 50 { tool := &MockTool{ name: fmt.Sprintf("tool_%02d", i), description: "Mock tool for benchmarking", @@ -144,7 +144,7 @@ func BenchmarkRegistry_ProviderDefs_50Tools(b *testing.B) { // BenchmarkRegistry_Alias_50Tools benchmarks tool lookup with aliases in registry. func BenchmarkRegistry_Alias_50Tools(b *testing.B) { reg := NewRegistry() - for i := 0; i < 50; i++ { + for i := range 50 { tool := &MockTool{ name: fmt.Sprintf("tool_%02d", i), description: "Mock tool for benchmarking", @@ -165,7 +165,7 @@ func BenchmarkRegistry_Alias_50Tools(b *testing.B) { // BenchmarkRegistry_Disable_Enable benchmarks disabling and enabling tools. func BenchmarkRegistry_Disable_Enable(b *testing.B) { reg := NewRegistry() - for i := 0; i < 50; i++ { + for i := range 50 { tool := &MockTool{ name: fmt.Sprintf("tool_%02d", i), description: "Mock tool for benchmarking", @@ -185,7 +185,7 @@ func BenchmarkRegistry_Disable_Enable(b *testing.B) { // BenchmarkRegistry_Count benchmarks counting tools in registry. func BenchmarkRegistry_Count(b *testing.B) { reg := NewRegistry() - for i := 0; i < 50; i++ { + for i := range 50 { tool := &MockTool{ name: fmt.Sprintf("tool_%02d", i), description: "Mock tool for benchmarking", @@ -203,7 +203,7 @@ func BenchmarkRegistry_Count(b *testing.B) { // BenchmarkRegistry_Resolve_WithDisabled benchmarks resolving tools with some disabled. func BenchmarkRegistry_Resolve_WithDisabled(b *testing.B) { reg := NewRegistry() - for i := 0; i < 50; i++ { + for i := range 50 { tool := &MockTool{ name: fmt.Sprintf("tool_%02d", i), description: "Mock tool for benchmarking", @@ -212,7 +212,7 @@ func BenchmarkRegistry_Resolve_WithDisabled(b *testing.B) { } // Disable half the tools - for i := 0; i < 25; i++ { + for i := range 25 { reg.Disable(fmt.Sprintf("tool_%02d", i)) } diff --git a/internal/tools/shell_abort_test.go b/internal/tools/shell_abort_test.go index 5195d5ec..b05c3b0b 100644 --- a/internal/tools/shell_abort_test.go +++ b/internal/tools/shell_abort_test.go @@ -80,7 +80,7 @@ func findOrphanSleeps(t *testing.T) []string { } var found []string - for _, line := range strings.Split(string(out), "\n") { + for line := range strings.SplitSeq(string(out), "\n") { // Match lines containing "sleep 60" but not the grep/ps command itself. if strings.Contains(line, "sleep 60") && !strings.Contains(line, "ps aux") { fields := strings.Fields(line) diff --git a/internal/tools/web_search_config.go b/internal/tools/web_search_config.go index 2fbc0cfb..a4972061 100644 --- a/internal/tools/web_search_config.go +++ b/internal/tools/web_search_config.go @@ -1,6 +1,7 @@ package tools import ( + "slices" "strings" "time" @@ -98,12 +99,7 @@ func NormalizeWebSearchProviderOrder(order []string) []string { } func isKnownSearchProvider(id string) bool { - for _, p := range defaultSearchProviderOrder { - if p == id { - return true - } - } - return false + return slices.Contains(defaultSearchProviderOrder, id) } // --- Shared provider helpers --- diff --git a/internal/tracing/collector_retry_test.go b/internal/tracing/collector_retry_test.go index d597db74..bef21885 100644 --- a/internal/tracing/collector_retry_test.go +++ b/internal/tracing/collector_retry_test.go @@ -219,7 +219,7 @@ func TestShutdown_WorkerExitsCleanly(t *testing.T) { c := newTestCollector(&alwaysFailStore{}) // Put a few items in the retry channel before starting worker. - for i := 0; i < 5; i++ { + for range 5 { c.retryCh <- pendingUpdate{TraceID: uuid.New(), Updates: map[string]any{"status": "pending"}} } diff --git a/internal/tracing/cost.go b/internal/tracing/cost.go index 85cd3ca4..c30aeaa3 100644 --- a/internal/tracing/cost.go +++ b/internal/tracing/cost.go @@ -29,12 +29,10 @@ func CalculateCost(pricing *config.ModelPricing, usage *providers.Usage) float64 // ReasoningPerMillion rate is set. Otherwise price the full CompletionTokens // at OutputPerMillion — matches the provider billing semantics described above. if pricing.ReasoningPerMillion > 0 && usage.ThinkingTokens > 0 { - visible := usage.CompletionTokens - usage.ThinkingTokens - if visible < 0 { + visible := max(usage.CompletionTokens-usage.ThinkingTokens, // Defensive: thinkingChars/4 estimate for Anthropic may exceed OutputTokens // under unusual streaming conditions. Clamp to zero instead of going negative. - visible = 0 - } + 0) cost += float64(visible) * pricing.OutputPerMillion / 1_000_000 cost += float64(usage.ThinkingTokens) * pricing.ReasoningPerMillion / 1_000_000 } else { diff --git a/internal/vault/rescan_test.go b/internal/vault/rescan_test.go index d86b3b2d..0d991e2e 100644 --- a/internal/vault/rescan_test.go +++ b/internal/vault/rescan_test.go @@ -16,61 +16,61 @@ func TestInferOwnerFromPath(t *testing.T) { } tests := []struct { - path string - wantAgentID *string - wantTeamID *string - wantScope string + path string + wantAgentID *string + wantTeamID *string + wantScope string wantStrippedPath string }{ // Legacy agents/{key}/... → personal scope, full path preserved { - path: "agents/my-bot/notes/todo.md", - wantAgentID: strPtr("uuid-1"), - wantScope: "personal", + path: "agents/my-bot/notes/todo.md", + wantAgentID: new("uuid-1"), + wantScope: "personal", wantStrippedPath: "agents/my-bot/notes/todo.md", }, { - path: "agents/my-bot/file.md", - wantAgentID: strPtr("uuid-1"), - wantScope: "personal", + path: "agents/my-bot/file.md", + wantAgentID: new("uuid-1"), + wantScope: "personal", wantStrippedPath: "agents/my-bot/file.md", }, // Root-level {agent_key}/... → personal scope (workspace layout) { - path: "my-bot/telegram/123/report.md", - wantAgentID: strPtr("uuid-1"), - wantScope: "personal", + path: "my-bot/telegram/123/report.md", + wantAgentID: new("uuid-1"), + wantScope: "personal", wantStrippedPath: "my-bot/telegram/123/report.md", }, { - path: "other-bot/docs/guide.md", - wantAgentID: strPtr("uuid-2"), - wantScope: "personal", + path: "other-bot/docs/guide.md", + wantAgentID: new("uuid-2"), + wantScope: "personal", wantStrippedPath: "other-bot/docs/guide.md", }, // teams/{uuid}/... → team scope, full path preserved { - path: "teams/" + validUUID + "/doc.md", - wantTeamID: strPtr(validUUID), - wantScope: "team", + path: "teams/" + validUUID + "/doc.md", + wantTeamID: new(validUUID), + wantScope: "team", wantStrippedPath: "teams/" + validUUID + "/doc.md", }, { - path: "teams/" + validUUID + "/deep/nested.md", - wantTeamID: strPtr(validUUID), - wantScope: "team", + path: "teams/" + validUUID + "/deep/nested.md", + wantTeamID: new(validUUID), + wantScope: "team", wantStrippedPath: "teams/" + validUUID + "/deep/nested.md", }, // Root-level file (no slash) → shared { - path: "README.md", - wantScope: "shared", + path: "README.md", + wantScope: "shared", wantStrippedPath: "README.md", }, // Nested file not matching any agent key → shared { - path: "docs/guide.md", - wantScope: "shared", + path: "docs/guide.md", + wantScope: "shared", wantStrippedPath: "docs/guide.md", }, // Unknown agent under agents/ prefix → skip @@ -90,8 +90,8 @@ func TestInferOwnerFromPath(t *testing.T) { }, // Unknown root folder (not an agent key) → shared { - path: "telegram/group/file.md", - wantScope: "shared", + path: "telegram/group/file.md", + wantScope: "shared", wantStrippedPath: "telegram/group/file.md", }, } @@ -203,4 +203,5 @@ func TestInferTitle(t *testing.T) { } } -func strPtr(s string) *string { return &s } +//go:fix inline +func strPtr(s string) *string { return new(s) } diff --git a/internal/vault/safe_walk_test.go b/internal/vault/safe_walk_test.go index f9169b74..a4d9ae98 100644 --- a/internal/vault/safe_walk_test.go +++ b/internal/vault/safe_walk_test.go @@ -99,7 +99,7 @@ func TestSafeWalkWorkspace_ExcludedPaths(t *testing.T) { func TestSafeWalkWorkspace_MaxFileLimit(t *testing.T) { dir := t.TempDir() - for i := 0; i < 20; i++ { + for i := range 20 { writeFile(t, dir, filepath.Join("files", string(rune('a'+i))+".txt"), "data") } @@ -122,7 +122,7 @@ func TestSafeWalkWorkspace_MaxTotalBytes(t *testing.T) { // Create files that exceed total byte limit. Use a whitelisted extension // (.txt) so the files actually register — Phase 01 whitelist would skip .bin. bigContent := make([]byte, 1024) // 1KB each - for i := 0; i < 10; i++ { + for i := range 10 { writeFile(t, dir, filepath.Join("data", string(rune('a'+i))+".txt"), string(bigContent)) } @@ -143,7 +143,7 @@ func TestSafeWalkWorkspace_MaxTotalBytes(t *testing.T) { func TestSafeWalkWorkspace_ContextCancel(t *testing.T) { dir := t.TempDir() - for i := 0; i < 50; i++ { + for i := range 50 { writeFile(t, dir, filepath.Join("files", string(rune('a'+i/26))+"_"+string(rune('a'+i%26))+".txt"), "data") } @@ -208,7 +208,7 @@ func TestIsExcludedPath(t *testing.T) { {"images/screenshot.png", false}, {"teams/abc-123/doc.md", false}, {"soul-notes.md", false}, - {"deep/SOUL.md", false}, // not root-level context file + {"deep/SOUL.md", false}, // not root-level context file {".uploads/photo.jpg", false}, {"report.pdf", false}, }