diff --git a/pkg/agent/context.go b/pkg/agent/context.go index 809d49fba..1680b5eaa 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -137,15 +137,15 @@ Maintain these sections in MEMORY.md under ## Orchestration: - **Decisions**: Key architectural/implementation decisions made during orchestration` type ContextBuilder struct { - workspace string - workDir string // session-specific working directory (worktree or project subdir) - skillsLoader *skills.SkillsLoader - memory *MemoryStore - tools *tools.ToolRegistry // Direct reference to tool registry - peerNote string // set per-call from loop.go for peer session awareness - orchestrationEnabled bool // set from AgentLoop when --orchestration flag is used - toolDiscoveryBM25 bool - toolDiscoveryRegex bool + workspace string + workDir string // session-specific working directory (worktree or project subdir) + skillsLoader *skills.SkillsLoader + memory *MemoryStore + tools *tools.ToolRegistry // Direct reference to tool registry + peerNote string // set per-call from loop.go for peer session awareness + orchestrationEnabled bool // set from AgentLoop when --orchestration flag is used + toolDiscoveryBM25 bool + toolDiscoveryRegex bool // Cache for system prompt to avoid rebuilding on every call. // This fixes issue #607: repeated reprocessing of the entire context. diff --git a/pkg/agent/instance_ext_test.go b/pkg/agent/instance_ext_test.go index 27799eda2..59e1f04e2 100644 --- a/pkg/agent/instance_ext_test.go +++ b/pkg/agent/instance_ext_test.go @@ -1,9 +1,10 @@ package agent import ( - "github.com/sipeed/picoclaw/pkg/config" "os" "testing" + + "github.com/sipeed/picoclaw/pkg/config" ) func TestNewAgentInstance_ResolveCandidatesFromModelListAliasWithoutProtocol(t *testing.T) { diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 9c61a134b..259f2cd28 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -2535,106 +2535,6 @@ func (al *AgentLoop) forceTextResponse(ctx context.Context, agent *AgentInstance return content } -// selectCandidates returns the model candidates and resolved model name to use -// for a conversation turn. When model routing is configured and the incoming -// message scores below the complexity threshold, it returns the light model -// candidates instead of the primary ones. -// -// The returned (candidates, model) pair is used for all LLM calls within one -// turn — tool follow-up iterations use the same tier as the initial call so -// that a multi-step tool chain doesn't switch models mid-way. -func (al *AgentLoop) selectCandidates( - agent *AgentInstance, - userMsg string, - history []providers.Message, -) (candidates []providers.FallbackCandidate, model string) { - if agent.Router == nil || len(agent.LightCandidates) == 0 { - return agent.Candidates, agent.Model - } - - _, usedLight, score := agent.Router.SelectModel(userMsg, history, agent.Model) - if !usedLight { - logger.DebugCF("agent", "Model routing: primary model selected", - map[string]any{ - "agent_id": agent.ID, - "score": score, - "threshold": agent.Router.Threshold(), - }) - return agent.Candidates, agent.Model - } - - logger.InfoCF("agent", "Model routing: light model selected", - map[string]any{ - "agent_id": agent.ID, - "light_model": agent.Router.LightModel(), - "score": score, - "threshold": agent.Router.Threshold(), - }) - return agent.LightCandidates, agent.Router.LightModel() -} - -// findNearestUserMessage finds the nearest user message to the given index. -// It searches backward first, then forward if no user message is found. -func (al *AgentLoop) findNearestUserMessage(messages []providers.Message, mid int) int { - originalMid := mid - - for mid > 0 && messages[mid].Role != "user" { - mid-- - } - - if messages[mid].Role == "user" { - return mid - } - - mid = originalMid - for mid < len(messages) && messages[mid].Role != "user" { - mid++ - } - - if mid < len(messages) { - return mid - } - - return originalMid -} - -// retryLLMCall calls the LLM with retry logic. -func (al *AgentLoop) retryLLMCall( - ctx context.Context, - agent *AgentInstance, - prompt string, - maxRetries int, -) (*providers.LLMResponse, error) { - const ( - llmTemperature = 0.3 - ) - - var resp *providers.LLMResponse - var err error - - for attempt := 0; attempt < maxRetries; attempt++ { - resp, err = agent.Provider.Chat( - ctx, - []providers.Message{{Role: "user", Content: prompt}}, - nil, - agent.Model, - map[string]any{ - "max_tokens": agent.MaxTokens, - "temperature": llmTemperature, - "prompt_cache_key": agent.ID, - }, - ) - if err == nil && resp != nil && resp.Content != "" { - return resp, nil - } - if attempt < maxRetries-1 { - time.Sleep(time.Duration(attempt+1) * 100 * time.Millisecond) - } - } - - return resp, err -} - // updateToolContexts updates the context for tools that need channel/chatID info. func (al *AgentLoop) updateToolContexts(agent *AgentInstance, channel, chatID string) { @@ -2658,10 +2558,3 @@ func (al *AgentLoop) updateToolContexts(agent *AgentInstance, channel, chatID st } } } - -func inboundMetadata(msg bus.InboundMessage, key string) string { - if msg.Metadata == nil { - return "" - } - return msg.Metadata[key] -} diff --git a/pkg/agent/loop_ext_test.go b/pkg/agent/loop_ext_test.go index a16682711..79e12b80f 100644 --- a/pkg/agent/loop_ext_test.go +++ b/pkg/agent/loop_ext_test.go @@ -3,17 +3,18 @@ package agent import ( "context" "fmt" - "github.com/sipeed/picoclaw/pkg/bus" - "github.com/sipeed/picoclaw/pkg/config" - "github.com/sipeed/picoclaw/pkg/providers" - "github.com/sipeed/picoclaw/pkg/providers/protocoltypes" - "github.com/sipeed/picoclaw/pkg/tools" "os" "path/filepath" "strings" "sync" "testing" "time" + + "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/providers" + "github.com/sipeed/picoclaw/pkg/providers/protocoltypes" + "github.com/sipeed/picoclaw/pkg/tools" ) func TestRecordLastHeartbeatTarget(t *testing.T) { @@ -55,16 +56,10 @@ func TestRecordLastHeartbeatTarget(t *testing.T) { } } -type mockContextualTool struct { - lastChannel string - - lastChatID string -} - -func (m *mockContextualTool) SetContext(channel, chatID string) { - m.lastChannel = channel - - m.lastChatID = chatID +func newTestAgentLoopSimple(t *testing.T) (*AgentLoop, func()) { + t.Helper() + al, _, _, _, cleanup := newTestAgentLoop(t) //nolint:dogsled + return al, cleanup } func TestShouldInjectReminder(t *testing.T) { @@ -336,7 +331,6 @@ func TestSlashCommandResponseSkipsPlaceholder(t *testing.T) { } func TestBuildTaskReminder_Truncation(t *testing.T) { - longMsg := strings.Repeat("あ", 1000) longBlocker := strings.Repeat("X", 500) @@ -407,7 +401,7 @@ func TestBuildPlanReminder(t *testing.T) { } func TestPlanCommand_ShowNoPlan(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -466,7 +460,7 @@ func TestSplitChatAndThread(t *testing.T) { } func TestHeartbeatCommandThreadHerePersistsConfig(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -522,7 +516,7 @@ func TestHeartbeatCommandThreadHerePersistsConfig(t *testing.T) { } func TestHeartbeatCommandThreadOff(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -550,7 +544,7 @@ func TestHeartbeatCommandThreadOff(t *testing.T) { } func TestPlanCommand_StartNewPlan(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -588,7 +582,7 @@ func TestPlanCommand_StartNewPlan(t *testing.T) { } func TestPlanCommand_StartBlockedByExisting(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -606,7 +600,7 @@ func TestPlanCommand_StartBlockedByExisting(t *testing.T) { } func TestPlanCommand_Clear(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -626,7 +620,7 @@ func TestPlanCommand_Clear(t *testing.T) { } func TestPlanCommand_ClearNoPlan(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -638,7 +632,7 @@ func TestPlanCommand_ClearNoPlan(t *testing.T) { } func TestPlanCommand_Start(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -664,7 +658,7 @@ func TestPlanCommand_Start(t *testing.T) { } func TestPlanCommand_StartFromReview(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -690,7 +684,7 @@ func TestPlanCommand_StartFromReview(t *testing.T) { } func TestPlanCommand_StartNoPhases(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -714,7 +708,7 @@ func TestPlanCommand_StartNoPhases(t *testing.T) { } func TestPlanCommand_StartAlreadyExecuting(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -740,7 +734,7 @@ func TestPlanCommand_StartAlreadyExecuting(t *testing.T) { } func TestPlanCommand_Done(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -782,7 +776,7 @@ Test context } func TestPlanCommand_DoneInvalidStep(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -796,7 +790,7 @@ func TestPlanCommand_DoneInvalidStep(t *testing.T) { } func TestPlanCommand_Add(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -842,7 +836,7 @@ Test context } func TestPlanCommand_Next(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -892,7 +886,7 @@ Test } func TestPlanCommand_ShowActivePlan(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -1111,7 +1105,6 @@ func TestIsToolAllowedDuringInterview_FuzzyNames(t *testing.T) { want bool }{ - {"read_file", nil, true}, {"list_dir", nil, true}, @@ -1447,7 +1440,6 @@ func TestBuildRichStatus(t *testing.T) { } func TestBuildRichStatus_ProjectDir(t *testing.T) { - task := &activeTask{ Iteration: 1, @@ -1600,7 +1592,6 @@ func TestCommonDirPrefix(t *testing.T) { } func TestDisplayProjectDir(t *testing.T) { - task1 := &activeTask{projectDir: "my-app", fileCommonDir: "projects/other"} if got := displayProjectDir(task1); got != "my-app" { @@ -1627,7 +1618,6 @@ func TestDisplayProjectDir(t *testing.T) { } func TestBuildRichStatus_FixedHeight(t *testing.T) { - countLines := func(s string) int { return strings.Count(s, "\n") } @@ -1680,7 +1670,6 @@ func TestBuildRichStatus_FixedHeight(t *testing.T) { } func TestBuildRichStatus_StickyError(t *testing.T) { - errEntry := toolLogEntry{ Name: "[2] exec", ArgsSnip: "pytest", Result: "✗ 3.2s", @@ -1763,7 +1752,6 @@ func TestBuildRichStatus_LatestEntryNoInlineResult(t *testing.T) { } func TestSanitizeHistoryForProvider_MultiToolCall(t *testing.T) { - history := []providers.Message{ {Role: "user", Content: "hello"}, @@ -1805,14 +1793,17 @@ func TestSanitizeHistoryForProvider_MultiToolCall(t *testing.T) { } } -func TestPlanNudge_ForegroundExecution(t *testing.T) { +func setupPlanNudgeTest( + t *testing.T, + plan, content, sessionKey string, +) (*countingMockProvider, func()) { + t.Helper() + tmpDir, err := os.MkdirTemp("", "agent-nudge-test-*") if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) - cfg := &config.Config{ Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ @@ -1839,13 +1830,11 @@ func TestPlanNudge_ForegroundExecution(t *testing.T) { t.Fatal("no default agent") } - plan := "# Active Plan\n\n> Task: Test\n> Status: executing\n> Phase: 1\n\n## Phase 1: Setup\n- [ ] Step one\n- [ ] Step two\n\n## Context\n" - agent.ContextBuilder.WriteMemory(plan) - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - - defer cancel() + ctx, cancel := context.WithTimeout( + context.Background(), 5*time.Second, + ) msg := bus.InboundMessage{ Channel: "test", @@ -1854,82 +1843,62 @@ func TestPlanNudge_ForegroundExecution(t *testing.T) { ChatID: "chat1", - Content: "continue working", + Content: content, - SessionKey: "nudge-test", + SessionKey: sessionKey, } _, err = al.processMessage(ctx, msg) + + cancel() + if err != nil { + os.RemoveAll(tmpDir) t.Fatalf("processMessage failed: %v", err) } + return provider, func() { os.RemoveAll(tmpDir) } +} + +func TestPlanNudge_ForegroundExecution(t *testing.T) { + plan := "# Active Plan\n\n> Task: Test\n" + + "> Status: executing\n> Phase: 1\n\n" + + "## Phase 1: Setup\n- [ ] Step one\n" + + "- [ ] Step two\n\n## Context\n" + + provider, cleanup := setupPlanNudgeTest( + t, plan, "continue working", "nudge-test", + ) + + defer cleanup() + if provider.calls < 2 { - t.Errorf("expected at least 2 provider calls (nudge should trigger continuation), got %d", provider.calls) + t.Errorf( + "expected at least 2 provider calls"+ + " (nudge should trigger continuation),"+ + " got %d", provider.calls, + ) } } func TestPlanNudge_NoNudgeWhenAllStepsComplete(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "agent-nudge-test-*") - if err != nil { - t.Fatalf("Failed to create temp dir: %v", err) - } + plan := "# Active Plan\n\n> Task: Test\n" + + "> Status: executing\n> Phase: 1\n\n" + + "## Phase 1: Setup\n- [x] Step one\n" + + "- [x] Step two\n\n## Context\n" - defer os.RemoveAll(tmpDir) + provider, cleanup := setupPlanNudgeTest( + t, plan, "all done", "nudge-test-complete", + ) - cfg := &config.Config{ - Agents: config.AgentsConfig{ - Defaults: config.AgentDefaults{ - Workspace: tmpDir, - - Model: "test-model", - - MaxTokens: 4096, - - MaxToolIterations: 10, - }, - }, - } - - provider := &countingMockProvider{} - - msgBus := bus.NewMessageBus() - - al := NewAgentLoop(cfg, msgBus, provider) - - agent := al.registry.GetDefaultAgent() - - if agent == nil { - t.Fatal("no default agent") - } - - plan := "# Active Plan\n\n> Task: Test\n> Status: executing\n> Phase: 1\n\n## Phase 1: Setup\n- [x] Step one\n- [x] Step two\n\n## Context\n" - - agent.ContextBuilder.WriteMemory(plan) - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - - defer cancel() - - msg := bus.InboundMessage{ - Channel: "test", - - SenderID: "user1", - - ChatID: "chat1", - - Content: "all done", - - SessionKey: "nudge-test-complete", - } - - _, err = al.processMessage(ctx, msg) - if err != nil { - t.Fatalf("processMessage failed: %v", err) - } + defer cleanup() if provider.calls != 1 { - t.Errorf("expected exactly 1 provider call (no nudge needed), got %d", provider.calls) + t.Errorf( + "expected exactly 1 provider call"+ + " (no nudge needed), got %d", + provider.calls, + ) } } @@ -1958,7 +1927,6 @@ func TestPlanNudge_ProgressMessage(t *testing.T) { var nudgeContent string provider := &nudgeCaptureMockProvider{onSecondCall: func(msgs []providers.Message) { - for i := len(msgs) - 1; i >= 0; i-- { if msgs[i].Role == "user" { nudgeContent = msgs[i].Content @@ -2108,7 +2076,6 @@ func TestConsumeStream_DetectsRepetition(t *testing.T) { repeatedChunk := strings.Repeat("abcdefghij", 50) go func() { - for i := 0; i < 6; i++ { ch <- protocoltypes.StreamEvent{ContentDelta: repeatedChunk} } @@ -2350,14 +2317,17 @@ func (m *modelCapturingMockProvider) GetDefaultModel() string { return "model-capturing-mock" } -func TestAgentLoop_PlanModel_UsedDuringInterviewing(t *testing.T) { +func setupPlanModelTest( + t *testing.T, + response, memoryContent, userMsg, sessionKey string, +) (*modelCapturingMockProvider, func()) { + t.Helper() + tmpDir, err := os.MkdirTemp("", "agent-test-planmodel-*") if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) - cfg := &config.Config{ Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ @@ -2376,7 +2346,7 @@ func TestAgentLoop_PlanModel_UsedDuringInterviewing(t *testing.T) { msgBus := bus.NewMessageBus() - provider := &modelCapturingMockProvider{response: "Plan interview response"} + provider := &modelCapturingMockProvider{response: response} al := NewAgentLoop(cfg, msgBus, provider) @@ -2392,28 +2362,40 @@ func TestAgentLoop_PlanModel_UsedDuringInterviewing(t *testing.T) { memoryPath := filepath.Join(memoryDir, "MEMORY.md") - memoryContent := "# Active Plan\n\n> Task: Test plan model\n> Status: interviewing\n> Phase: 1\n" - - if wErr := os.WriteFile(memoryPath, []byte(memoryContent), 0o644); wErr != nil { + if wErr := os.WriteFile( + memoryPath, []byte(memoryContent), 0o644, + ); wErr != nil { + os.RemoveAll(tmpDir) t.Fatalf("Failed to write MEMORY.md: %v", wErr) } _, err = al.ProcessDirectWithChannel( - context.Background(), - - "Hello, plan model test", - - "test-plan-session", - + userMsg, + sessionKey, "test", - "test-chat", ) if err != nil { + os.RemoveAll(tmpDir) t.Fatalf("ProcessDirectWithChannel failed: %v", err) } + return provider, func() { os.RemoveAll(tmpDir) } +} + +func TestAgentLoop_PlanModel_UsedDuringInterviewing(t *testing.T) { + mem := "# Active Plan\n\n" + + "> Task: Test plan model\n" + + "> Status: interviewing\n> Phase: 1\n" + + provider, cleanup := setupPlanModelTest( + t, "Plan interview response", mem, + "Hello, plan model test", "test-plan-session", + ) + + defer cleanup() + provider.mu.Lock() defer provider.mu.Unlock() @@ -2423,89 +2405,26 @@ func TestAgentLoop_PlanModel_UsedDuringInterviewing(t *testing.T) { } if provider.models[0] != "plan-model" { - t.Errorf("Expected plan model 'plan-model' during interviewing, got %q", provider.models[0]) + t.Errorf( + "Expected plan model 'plan-model'"+ + " during interviewing, got %q", + provider.models[0], + ) } } func TestAgentLoop_PlanModel_NotUsedDuringExecuting(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "agent-test-planmodel-exec-*") - if err != nil { - t.Fatalf("Failed to create temp dir: %v", err) - } + mem := "# Active Plan\n\n\n\n" + + "> Task: Test plan model\n\n" + + "> Status: executing\n\n> Phase: 1\n\n\n\n" + + "## Phase 1: Build\n\n- [ ] Run build\n\n" - defer os.RemoveAll(tmpDir) - - cfg := &config.Config{ - Agents: config.AgentsConfig{ - Defaults: config.AgentDefaults{ - Workspace: tmpDir, - - Model: "normal-model", - - PlanModel: "plan-model", - - MaxTokens: 4096, - - MaxToolIterations: 2, - }, - }, - } - - msgBus := bus.NewMessageBus() - - provider := &modelCapturingMockProvider{response: "Executing response"} - - al := NewAgentLoop(cfg, msgBus, provider) - - defaultAgent := al.registry.GetDefaultAgent() - - if defaultAgent == nil { - t.Fatal("No default agent found") - } - - memoryDir := filepath.Join(tmpDir, "memory") - - os.MkdirAll(memoryDir, 0o755) - - memoryPath := filepath.Join(memoryDir, "MEMORY.md") - - memoryContent := `# Active Plan - - - -> Task: Test plan model - -> Status: executing - -> Phase: 1 - - - -## Phase 1: Build - -- [ ] Run build - -` - - if wErr := os.WriteFile(memoryPath, []byte(memoryContent), 0o644); wErr != nil { - t.Fatalf("Failed to write MEMORY.md: %v", wErr) - } - - _, err = al.ProcessDirectWithChannel( - - context.Background(), - - "Hello, executing test", - - "test-exec-session", - - "test", - - "test-chat", + provider, cleanup := setupPlanModelTest( + t, "Executing response", mem, + "Hello, executing test", "test-exec-session", ) - if err != nil { - t.Fatalf("ProcessDirectWithChannel failed: %v", err) - } + + defer cleanup() provider.mu.Lock() @@ -2516,7 +2435,11 @@ func TestAgentLoop_PlanModel_NotUsedDuringExecuting(t *testing.T) { } if provider.models[0] != "normal-model" { - t.Errorf("Expected normal model 'normal-model' during executing, got %q", provider.models[0]) + t.Errorf( + "Expected normal model 'normal-model'"+ + " during executing, got %q", + provider.models[0], + ) } } @@ -2606,7 +2529,7 @@ func TestAgentLoop_PlanModel_ResolvesProviderForSingleCandidate(t *testing.T) { } func TestPlanCommand_StartClear(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() @@ -2672,7 +2595,7 @@ func TestPlanCommand_StartClear(t *testing.T) { } func TestPlanCommand_StartWithoutClear_PreservesHistory(t *testing.T) { - al, _, _, _, cleanup := newTestAgentLoop(t) + al, cleanup := newTestAgentLoopSimple(t) defer cleanup() diff --git a/pkg/agent/loop_hooks.go b/pkg/agent/loop_hooks.go index 5472239d2..531a29ac0 100644 --- a/pkg/agent/loop_hooks.go +++ b/pkg/agent/loop_hooks.go @@ -70,17 +70,17 @@ type iterationHooks struct { // defaultHooks returns an iterationHooks with all fields set to no-ops. func defaultHooks() iterationHooks { return iterationHooks{ - OnIterationStart: func(int) string { return "" }, - FilterTools: func(d []providers.ToolDefinition) []providers.ToolDefinition { return d }, - SetupStreaming: func() (func(string, string), func()) { return nil, nil }, - SelectModel: func() (string, []providers.FallbackCandidate) { return "", nil }, - OnPreLLMCall: func() {}, - OnNoToolCalls: func(string, int) (string, bool) { return "", false }, - FilterToolCalls: func(c []providers.ToolCall) ([]providers.ToolCall, string) { return c, "" }, - OnPreToolExec: func(context.Context, providers.ToolCall) tools.AsyncCallback { return nil }, - OnToolExecDone: func(providers.ToolCall, *tools.ToolResult, time.Duration) {}, - OnToolsProcessed: func(context.Context, int, []providers.ToolCall) {}, - InjectReminders: func(int, *[]providers.Message, string) {}, + OnIterationStart: func(int) string { return "" }, + FilterTools: func(d []providers.ToolDefinition) []providers.ToolDefinition { return d }, + SetupStreaming: func() (func(string, string), func()) { return nil, nil }, + SelectModel: func() (string, []providers.FallbackCandidate) { return "", nil }, + OnPreLLMCall: func() {}, + OnNoToolCalls: func(string, int) (string, bool) { return "", false }, + FilterToolCalls: func(c []providers.ToolCall) ([]providers.ToolCall, string) { return c, "" }, + OnPreToolExec: func(context.Context, providers.ToolCall) tools.AsyncCallback { return nil }, + OnToolExecDone: func(providers.ToolCall, *tools.ToolResult, time.Duration) {}, + OnToolsProcessed: func(context.Context, int, []providers.ToolCall) {}, + InjectReminders: func(int, *[]providers.Message, string) {}, RefreshSystemPrompt: func([]providers.Message) {}, } } @@ -495,7 +495,9 @@ func (al *AgentLoop) buildReminderInjector( case "plan_review": content = fmt.Sprintf( "[Subagent %s submitted a plan for review]:\n%s\nRespond using the review_subagent_plan tool with task_id=%q.", - q.TaskID, q.Content, q.TaskID, + q.TaskID, + q.Content, + q.TaskID, ) default: content = fmt.Sprintf( diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index cab82e176..e2781e935 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -482,7 +482,7 @@ func TestProcessMessage_CommandOutcomes(t *testing.T) { Content: "/show channel", Peer: baseMsg.Peer, }) - if showResp != "Current Channel: whatsapp" { + if showResp != "Current channel: whatsapp" { t.Fatalf("unexpected /show reply: %q", showResp) } if provider.calls != 0 { @@ -566,7 +566,7 @@ func TestProcessMessage_SwitchModelShowModelConsistency(t *testing.T) { ID: "user1", }, }) - if !strings.Contains(showResp, "Current Model: after-switch (Provider: openai)") { + if !strings.Contains(showResp, "Current model: after-switch") { t.Fatalf("unexpected /show model reply after switch: %q", showResp) } diff --git a/pkg/agent/memory_ext.go b/pkg/agent/memory_ext.go index 3d180d77a..1e595c09b 100644 --- a/pkg/agent/memory_ext.go +++ b/pkg/agent/memory_ext.go @@ -181,12 +181,12 @@ func (ms *MemoryStore) ClearLongTerm() error { // ---------- Plan state query methods ---------- var ( - reActivePlan = regexp.MustCompile(`(?m)^# Active Plan`) - reStatus = regexp.MustCompile(`(?m)^> Status:\s*(.+)`) - rePhase = regexp.MustCompile(`(?m)^> Phase:\s*(\d+)`) + reActivePlan = regexp.MustCompile(`(?m)^# Active Plan`) + reStatus = regexp.MustCompile(`(?m)^> Status:\s*(.+)`) + rePhase = regexp.MustCompile(`(?m)^> Phase:\s*(\d+)`) rePhaseHeader = regexp.MustCompile(`(?m)^## Phase (\d+):\s*(.*)`) - reWorkDir = regexp.MustCompile(`(?m)^> WorkDir:\s*(.+)`) - reTaskLine = regexp.MustCompile(`(?m)^> Task:\s*(.+)`) + reWorkDir = regexp.MustCompile(`(?m)^> WorkDir:\s*(.+)`) + reTaskLine = regexp.MustCompile(`(?m)^> Task:\s*(.+)`) ) // HasActivePlan returns true if MEMORY.md contains an active plan. @@ -481,12 +481,22 @@ func (ms *MemoryStore) getInterviewContextFrom(content string) string { sb.WriteString("- Tooling preferences (test framework, linter, formatter, CI)\n") sb.WriteString("- Key commands the user already runs (build, test, deploy)\n") sb.WriteString("\n### Rules\n") - sb.WriteString("- NEVER remove or overwrite the header block (`# Active Plan`, `> Task:`, `> Status:`, `> Phase:` lines). The system parses these to track state.\n") - sb.WriteString("- After each answer, use edit_file to append findings to the ## Context section of memory/MEMORY.md.\n") - sb.WriteString("- When you have enough information, use edit_file to add ## Phase, ## Commands, and ## Context sections BELOW the header block.\n") - sb.WriteString("- Each step MUST use checkbox syntax: `- [ ] description`. The system parses checkboxes to track progress.\n") + sb.WriteString( + "- NEVER remove or overwrite the header block (`# Active Plan`, `> Task:`, `> Status:`, `> Phase:` lines). The system parses these to track state.\n", + ) + sb.WriteString( + "- After each answer, use edit_file to append findings to the ## Context section of memory/MEMORY.md.\n", + ) + sb.WriteString( + "- When you have enough information, use edit_file to add ## Phase, ## Commands, and ## Context sections BELOW the header block.\n", + ) + sb.WriteString( + "- Each step MUST use checkbox syntax: `- [ ] description`. The system parses checkboxes to track progress.\n", + ) sb.WriteString("- Organize into 2-5 phases with 3-5 steps each.\n") - sb.WriteString("- After writing Phases, change `> Status: interviewing` to `> Status: review` via edit_file. The user must approve with /plan start before execution begins.\n") + sb.WriteString( + "- After writing Phases, change `> Status: interviewing` to `> Status: review` via edit_file. The user must approve with /plan start before execution begins.\n", + ) sb.WriteString("\n### Target Format (MANDATORY — system parses this exact structure)\n\n") sb.WriteString("# Active Plan\n") sb.WriteString("> Task: \n") diff --git a/pkg/channels/manager_ext_test.go b/pkg/channels/manager_ext_test.go index cec14c047..b54c6ec6a 100644 --- a/pkg/channels/manager_ext_test.go +++ b/pkg/channels/manager_ext_test.go @@ -3,11 +3,13 @@ package channels import ( "context" "fmt" - "github.com/sipeed/picoclaw/pkg/bus" - "golang.org/x/time/rate" "sync/atomic" "testing" "time" + + "golang.org/x/time/rate" + + "github.com/sipeed/picoclaw/pkg/bus" ) // mockEditorWithSendID implements MessageEditor and MessageSenderWithID. @@ -17,6 +19,12 @@ type mockEditorWithSendID struct { sendWithID func(ctx context.Context, chatID, content string) (string, error) } +func (m *mockEditorWithSendID) EditMessage( + ctx context.Context, chatID, messageID, content string, +) error { + return m.editFn(ctx, chatID, messageID, content) +} + func (m *mockEditorWithSendID) SendWithID(ctx context.Context, chatID, content string) (string, error) { return m.sendWithID(ctx, chatID, content) } @@ -384,6 +392,12 @@ type mockDraftSender struct { sendWithID func(ctx context.Context, chatID, content string) (string, error) } +func (m *mockDraftSender) EditMessage( + ctx context.Context, chatID, messageID, content string, +) error { + return m.editFn(ctx, chatID, messageID, content) +} + func (m *mockDraftSender) SendDraft(ctx context.Context, chatID string, draftID int, content string) error { return m.draftFn(ctx, chatID, draftID, content) } diff --git a/pkg/channels/telegram/telegram_ext_test.go b/pkg/channels/telegram/telegram_ext_test.go index 3ba213814..783e0ef5c 100644 --- a/pkg/channels/telegram/telegram_ext_test.go +++ b/pkg/channels/telegram/telegram_ext_test.go @@ -31,7 +31,14 @@ func TestParseTelegramChatID(t *testing.T) { t.Fatalf("parseTelegramChatID(%q) unexpected error: %v", tc.input, err) } if gotCID != tc.wantCID || gotTID != tc.wantTID { - t.Fatalf("parseTelegramChatID(%q) = (%d, %d), want (%d, %d)", tc.input, gotCID, gotTID, tc.wantCID, tc.wantTID) + t.Fatalf( + "parseTelegramChatID(%q) = (%d, %d), want (%d, %d)", + tc.input, + gotCID, + gotTID, + tc.wantCID, + tc.wantTID, + ) } }) } diff --git a/pkg/config/config.go b/pkg/config/config.go index 0d1bacdbc..58b3215cc 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -179,7 +179,7 @@ type AgentConfig struct { } type SubagentsConfig struct { - Enabled bool `json:"enabled,omitempty"` // Fork-only: gate orchestration + Enabled bool `json:"enabled,omitempty"` // Fork-only: gate orchestration AllowAgents []string `json:"allow_agents,omitempty"` Model *AgentModelConfig `json:"model,omitempty"` } @@ -306,16 +306,16 @@ type WhatsAppConfig struct { } type TelegramConfig struct { - Enabled bool `json:"enabled" env:"PICOCLAW_CHANNELS_TELEGRAM_ENABLED"` - Token string `json:"token" env:"PICOCLAW_CHANNELS_TELEGRAM_TOKEN"` - BaseURL string `json:"base_url" env:"PICOCLAW_CHANNELS_TELEGRAM_BASE_URL"` - Proxy string `json:"proxy" env:"PICOCLAW_CHANNELS_TELEGRAM_PROXY"` - AllowFrom FlexibleStringSlice `json:"allow_from" env:"PICOCLAW_CHANNELS_TELEGRAM_ALLOW_FROM"` + Enabled bool `json:"enabled" env:"PICOCLAW_CHANNELS_TELEGRAM_ENABLED"` + Token string `json:"token" env:"PICOCLAW_CHANNELS_TELEGRAM_TOKEN"` + BaseURL string `json:"base_url" env:"PICOCLAW_CHANNELS_TELEGRAM_BASE_URL"` + Proxy string `json:"proxy" env:"PICOCLAW_CHANNELS_TELEGRAM_PROXY"` + AllowFrom FlexibleStringSlice `json:"allow_from" env:"PICOCLAW_CHANNELS_TELEGRAM_ALLOW_FROM"` GroupTrigger GroupTriggerConfig `json:"group_trigger,omitempty"` Typing TypingConfig `json:"typing,omitempty"` Placeholder PlaceholderConfig `json:"placeholder,omitempty"` - ReasoningChannelID string `json:"reasoning_channel_id" env:"PICOCLAW_CHANNELS_TELEGRAM_REASONING_CHANNEL_ID"` - WebAppURL string `json:"web_app_url" env:"PICOCLAW_CHANNELS_TELEGRAM_WEB_APP_URL"` + ReasoningChannelID string `json:"reasoning_channel_id" env:"PICOCLAW_CHANNELS_TELEGRAM_REASONING_CHANNEL_ID"` + WebAppURL string `json:"web_app_url" env:"PICOCLAW_CHANNELS_TELEGRAM_WEB_APP_URL"` SubagentThreadID int `json:"subagent_thread_id,omitempty" env:"PICOCLAW_CHANNELS_TELEGRAM_SUBAGENT_THREAD_ID"` HeartbeatThreadID int `json:"heartbeat_thread_id,omitempty" env:"PICOCLAW_CHANNELS_TELEGRAM_HEARTBEAT_THREAD_ID"` } @@ -616,7 +616,7 @@ type ModelConfig struct { MaxTokensField string `json:"max_tokens_field,omitempty"` // Field name for max tokens (e.g., "max_completion_tokens") RequestTimeout int `json:"request_timeout,omitempty"` ThinkingLevel string `json:"thinking_level,omitempty"` // Extended thinking: off|low|medium|high|xhigh|adaptive - Stream *bool `json:"stream,omitempty"` // Use SSE streaming (default: protocol-dependent) + Stream *bool `json:"stream,omitempty"` // Use SSE streaming (default: protocol-dependent) } // Validate checks if the ModelConfig has all required fields. diff --git a/pkg/heartbeat/service.go b/pkg/heartbeat/service.go index 489d87be0..ef836d31e 100644 --- a/pkg/heartbeat/service.go +++ b/pkg/heartbeat/service.go @@ -14,8 +14,8 @@ import ( "sync" "time" - "github.com/sipeed/picoclaw/pkg/constants" "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/constants" "github.com/sipeed/picoclaw/pkg/fileutil" "github.com/sipeed/picoclaw/pkg/logger" "github.com/sipeed/picoclaw/pkg/state" diff --git a/pkg/heartbeat/service_ext_test.go b/pkg/heartbeat/service_ext_test.go index 8c2338073..e96591484 100644 --- a/pkg/heartbeat/service_ext_test.go +++ b/pkg/heartbeat/service_ext_test.go @@ -1,10 +1,11 @@ package heartbeat import ( - "github.com/sipeed/picoclaw/pkg/tools" "os" "path/filepath" "testing" + + "github.com/sipeed/picoclaw/pkg/tools" ) // TestExecuteHeartbeat_NoSendResponse verifies that heartbeat results diff --git a/pkg/providers/anthropic/provider_test.go b/pkg/providers/anthropic/provider_test.go index c37b7d52d..dc45441ab 100644 --- a/pkg/providers/anthropic/provider_test.go +++ b/pkg/providers/anthropic/provider_test.go @@ -84,7 +84,9 @@ func TestBuildParams_WithTools(t *testing.T) { Function: ToolFunctionDefinition{ Name: "get_weather", Description: "Get weather for a city", - Parameters: json.RawMessage(`{"type":"object","properties":{"city":{"type":"string"}},"required":["city"]}`), + Parameters: json.RawMessage( + `{"type":"object","properties":{"city":{"type":"string"}},"required":["city"]}`, + ), }, }, } diff --git a/pkg/providers/antigravity_provider.go b/pkg/providers/antigravity_provider.go index d49873570..7cd530977 100644 --- a/pkg/providers/antigravity_provider.go +++ b/pkg/providers/antigravity_provider.go @@ -433,7 +433,7 @@ func (p *AntigravityProvider) parseSSEResponse(body string) (*LLMResponse, error contentParts = append(contentParts, part.Text) } if part.FunctionCall != nil { - toolCalls = append(toolCalls, ToolCall{ + toolCalls = append(toolCalls, ToolCall{ ID: fmt.Sprintf("call_%s_%d", part.FunctionCall.Name, time.Now().UnixNano()), Name: part.FunctionCall.Name, Arguments: part.FunctionCall.Args, diff --git a/pkg/providers/claude_cli_provider_ext_test.go b/pkg/providers/claude_cli_provider_ext_test.go index 8c0164c75..c50731034 100644 --- a/pkg/providers/claude_cli_provider_ext_test.go +++ b/pkg/providers/claude_cli_provider_ext_test.go @@ -82,7 +82,6 @@ Done.` } func TestExtractXMLToolCalls_MismatchedCloseTag(t *testing.T) { - text := ` /home/user/project/pyproject.toml @@ -102,7 +101,7 @@ func TestExtractXMLToolCalls_MismatchedCloseTag(t *testing.T) { } func TestStripXMLToolCalls_MismatchedCloseTag(t *testing.T) { - text := `今テスト走らせるね。` + + text := `今テスト走らせるね。` + //nolint:gosmopolitan ` @@ -114,13 +113,12 @@ func TestStripXMLToolCalls_MismatchedCloseTag(t *testing.T) { if strings.Contains(got, "toolcall") || strings.Contains(got, "tool_call") { t.Errorf("should remove XML block, got %q", got) } - if !strings.Contains(got, "今テスト走らせるね。") { + if !strings.Contains(got, "今テスト走らせるね。") { //nolint:gosmopolitan t.Errorf("should keep text before, got %q", got) } } func TestExtractXMLToolCalls_UnderscoreOpenTag(t *testing.T) { - text := ` ls -la @@ -140,7 +138,6 @@ func TestExtractXMLToolCalls_UnderscoreOpenTag(t *testing.T) { } func TestExtractXMLToolCalls_HyphenTag(t *testing.T) { - text := ` /etc/hosts @@ -178,8 +175,11 @@ Finished.` } func TestExtractXMLToolCalls_OrphanedClosingTag(t *testing.T) { - - text := "了解!確認するね。\n[TOOLCALL]\n\n/home/user/workspace\n\n" + //nolint:gosmopolitan // intentional CJK test fixture + text := "了解!確認するね。\n[TOOLCALL]\n" + + "\n" + + "/home/user/workspace\n" + + "\n" calls := extractXMLToolCalls(text) if len(calls) != 1 { @@ -194,12 +194,16 @@ func TestExtractXMLToolCalls_OrphanedClosingTag(t *testing.T) { } func TestStripXMLToolCalls_OrphanedClosingTag(t *testing.T) { - text := "了解!確認するね。\n[TOOLCALL]\n\n/home/user\n\n" + //nolint:gosmopolitan // intentional CJK test fixture + text := "了解!確認するね。\n[TOOLCALL]\n" + + "\n" + + "/home/user\n" + + "\n" got := stripXMLToolCalls(text) if strings.Contains(got, "invoke") || strings.Contains(got, "TOOLCALL") || strings.Contains(got, "minimax") { t.Errorf("should remove orphaned closing tag block, got %q", got) } - if !strings.Contains(got, "了解") { + if !strings.Contains(got, "了解") { //nolint:gosmopolitan t.Errorf("should keep user-facing text, got %q", got) } } @@ -256,7 +260,6 @@ func TestLevenshtein(t *testing.T) { } func TestIsToolCallTag(t *testing.T) { - for _, name := range []string{"toolcall", "tool_call", "tool-call", "ToolCall", "Toolcall", "toolCall", "TOOLCALL"} { if !isToolCallTag(name) { t.Errorf("isToolCallTag(%q) = false, want true", name) diff --git a/pkg/providers/claude_cli_provider_test.go b/pkg/providers/claude_cli_provider_test.go index b1045c904..2f40e2e1a 100644 --- a/pkg/providers/claude_cli_provider_test.go +++ b/pkg/providers/claude_cli_provider_test.go @@ -620,7 +620,7 @@ func TestBuildSystemPrompt_WithTools(t *testing.T) { Function: ToolFunctionDefinition{ Name: "get_weather", Description: "Get weather for a location", - Parameters: json.RawMessage(`{"type":"object","properties":{"location":{"type":"string"}}}`), + Parameters: json.RawMessage(`{"type":"object","properties":{"location":{"type":"string"}}}`), }, }, } diff --git a/pkg/providers/codex_cli_provider_test.go b/pkg/providers/codex_cli_provider_test.go index f088d9dbd..d4dbaa9c5 100644 --- a/pkg/providers/codex_cli_provider_test.go +++ b/pkg/providers/codex_cli_provider_test.go @@ -292,7 +292,7 @@ func TestBuildPrompt_WithTools(t *testing.T) { Function: ToolFunctionDefinition{ Name: "get_weather", Description: "Get current weather", - Parameters: json.RawMessage(`{"type":"object","properties":{"city":{"type":"string"}}}`), + Parameters: json.RawMessage(`{"type":"object","properties":{"city":{"type":"string"}}}`), }, }, } diff --git a/pkg/providers/codex_provider_test.go b/pkg/providers/codex_provider_test.go index 45546b011..33d5c8322 100644 --- a/pkg/providers/codex_provider_test.go +++ b/pkg/providers/codex_provider_test.go @@ -114,7 +114,7 @@ func TestBuildCodexParams_WithTools(t *testing.T) { Function: ToolFunctionDefinition{ Name: "get_weather", Description: "Get weather", - Parameters: json.RawMessage(`{"type":"object","properties":{"city":{"type":"string"}}}`), + Parameters: json.RawMessage(`{"type":"object","properties":{"city":{"type":"string"}}}`), }, }, } @@ -161,7 +161,7 @@ func TestBuildCodexParams_WebSearchFunctionReplacedWithBuiltin(t *testing.T) { Function: ToolFunctionDefinition{ Name: "web_search", Description: "local web search", - Parameters: json.RawMessage(`{"type":"object"}`), + Parameters: json.RawMessage(`{"type":"object"}`), }, }, { @@ -169,7 +169,7 @@ func TestBuildCodexParams_WebSearchFunctionReplacedWithBuiltin(t *testing.T) { Function: ToolFunctionDefinition{ Name: "read_file", Description: "read file", - Parameters: json.RawMessage(`{"type":"object"}`), + Parameters: json.RawMessage(`{"type":"object"}`), }, }, } diff --git a/pkg/providers/factory_ext_test.go b/pkg/providers/factory_ext_test.go index 465968dc3..1ea0e5d11 100644 --- a/pkg/providers/factory_ext_test.go +++ b/pkg/providers/factory_ext_test.go @@ -1,9 +1,10 @@ package providers import ( + "testing" + "github.com/sipeed/picoclaw/pkg/auth" "github.com/sipeed/picoclaw/pkg/config" - "testing" ) func TestCreateProviderByName_OpenAI_OAuth(t *testing.T) { diff --git a/pkg/providers/openai_compat/provider_ext_test.go b/pkg/providers/openai_compat/provider_ext_test.go index 2c25422db..e5f7af10f 100644 --- a/pkg/providers/openai_compat/provider_ext_test.go +++ b/pkg/providers/openai_compat/provider_ext_test.go @@ -4,11 +4,12 @@ import ( "context" "encoding/json" "fmt" - "github.com/sipeed/picoclaw/pkg/providers/protocoltypes" "net/http" "net/http/httptest" "strings" "testing" + + "github.com/sipeed/picoclaw/pkg/providers/protocoltypes" ) func TestProviderChat_StripsGroqAndOllamaPrefixes(t *testing.T) { @@ -34,7 +35,7 @@ func TestProviderChat_StripsGroqAndOllamaPrefixes(t *testing.T) { }, } - for _, tt := range tests { + for _, tt := range tests { //nolint:dupl t.Run(tt.name, func(t *testing.T) { var requestBody map[string]any @@ -253,7 +254,6 @@ func TestReadSSEIntoChannel_TextAndToolCalls(t *testing.T) { } func TestReadSSEIntoChannel_ContextCancel(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) sseData := `data: {"choices":[{"delta":{"content":"first"},"finish_reason":""}]}` + "\n\n" diff --git a/pkg/providers/openai_compat/provider_test.go b/pkg/providers/openai_compat/provider_test.go index 41f278a1b..f21318f0f 100644 --- a/pkg/providers/openai_compat/provider_test.go +++ b/pkg/providers/openai_compat/provider_test.go @@ -464,7 +464,7 @@ func TestProviderChat_StripsGroqOllamaDeepseekVivgridPrefixes(t *testing.T) { }, } - for _, tt := range tests { + for _, tt := range tests { //nolint:dupl t.Run(tt.name, func(t *testing.T) { var requestBody map[string]any diff --git a/pkg/providers/protocoltypes/types.go b/pkg/providers/protocoltypes/types.go index 6aece739f..4379c9e94 100644 --- a/pkg/providers/protocoltypes/types.go +++ b/pkg/providers/protocoltypes/types.go @@ -5,7 +5,6 @@ import ( "strings" ) - type ToolCall struct { ID string `json:"id"` Type string `json:"type,omitempty"` diff --git a/pkg/providers/types.go b/pkg/providers/types.go index cbf6258a1..7f289421b 100644 --- a/pkg/providers/types.go +++ b/pkg/providers/types.go @@ -2,14 +2,12 @@ package providers import ( "context" - "fmt" - "encoding/json" + "fmt" "github.com/sipeed/picoclaw/pkg/providers/protocoltypes" ) - type ( ToolCall = protocoltypes.ToolCall FunctionCall = protocoltypes.FunctionCall @@ -97,7 +95,13 @@ type ModelConfig struct { type StreamingProvider interface { LLMProvider CanStream() bool - ChatStream(ctx context.Context, messages []Message, tools []ToolDefinition, model string, options map[string]any) (<-chan protocoltypes.StreamEvent, error) + ChatStream( + ctx context.Context, + messages []Message, + tools []ToolDefinition, + model string, + options map[string]any, + ) (<-chan protocoltypes.StreamEvent, error) } // UnmarshalArguments is a helper to parse FunctionCall.Arguments from json.RawMessage. diff --git a/pkg/session/manager_ext_test.go b/pkg/session/manager_ext_test.go index 5f7441af1..97a262851 100644 --- a/pkg/session/manager_ext_test.go +++ b/pkg/session/manager_ext_test.go @@ -1,8 +1,9 @@ package session import ( - "github.com/sipeed/picoclaw/pkg/providers" "testing" + + "github.com/sipeed/picoclaw/pkg/providers" ) func TestSanitizeHistory_OrphanedToolCall(t *testing.T) { @@ -30,7 +31,6 @@ func TestSanitizeHistory_OrphanedToolCall(t *testing.T) { } func TestSanitizeHistory_InterleavedMessages(t *testing.T) { - history := []providers.Message{ {Role: "user", Content: "first"}, diff --git a/pkg/state/state.go b/pkg/state/state.go index 31e73f010..34589dfae 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -26,7 +26,6 @@ type State struct { // Format: "channel:chatID[/thread]". HeartbeatTarget string `json:"heartbeat_target,omitempty"` - // LastChatID is the last chat ID used for communication LastChatID string `json:"last_chat_id,omitempty"` @@ -175,7 +174,6 @@ func (sm *Manager) load() error { return nil } - // SetLastHeartbeatTarget atomically updates the last heartbeat target and saves the state. func (sm *Manager) SetLastHeartbeatTarget(target string) error { sm.mu.Lock() diff --git a/pkg/tools/filesystem_ext_test.go b/pkg/tools/filesystem_ext_test.go index 14429605e..1bc676004 100644 --- a/pkg/tools/filesystem_ext_test.go +++ b/pkg/tools/filesystem_ext_test.go @@ -1,11 +1,12 @@ package tools import ( - "github.com/stretchr/testify/assert" "io" "os" "path/filepath" "testing" + + "github.com/stretchr/testify/assert" ) func TestHostFs_Read_PermissionDenied(t *testing.T) { @@ -118,7 +119,7 @@ func TestHostFs_Write(t *testing.T) { assert.Equal(t, newData, content) } -func TestSandboxFs_Write(t *testing.T) { +func TestSandboxFs_Write(t *testing.T) { //nolint:dupl tmpDir := t.TempDir() relPath := "atomic_root_test.txt" diff --git a/pkg/tools/filesystem_test.go b/pkg/tools/filesystem_test.go index 0bbf6caf0..98cad2561 100644 --- a/pkg/tools/filesystem_test.go +++ b/pkg/tools/filesystem_test.go @@ -452,7 +452,7 @@ func TestHostRW_Write(t *testing.T) { } // TestRootRW_Write verifies the rootRW.Write helper function -func TestRootRW_Write(t *testing.T) { +func TestRootRW_Write(t *testing.T) { //nolint:dupl tmpDir := t.TempDir() relPath := "atomic_root_test.txt" diff --git a/pkg/tools/registry_ext_test.go b/pkg/tools/registry_ext_test.go index 7a2857ff7..c1416b3c9 100644 --- a/pkg/tools/registry_ext_test.go +++ b/pkg/tools/registry_ext_test.go @@ -6,20 +6,6 @@ import ( "testing" ) -type mockCtxTool struct { - mockRegistryTool - - channel string - - chatID string -} - -func (m *mockCtxTool) SetContext(channel, chatID string) { - m.channel = channel - - m.chatID = chatID -} - func (m *mockAsyncRegistryTool) SetCallback(cb AsyncCallback) { m.lastCB = cb } @@ -52,83 +38,45 @@ func TestNormalizeToolName(t *testing.T) { } } -func TestToolRegistry_Get_FuzzyMatch(t *testing.T) { +func TestToolRegistry_Get_ExactMatch(t *testing.T) { r := NewToolRegistry() r.Register(newMockTool("read_file", "reads a file")) - r.Register(newMockTool("edit_file", "edits a file")) - r.Register(newMockTool("web_search", "searches the web")) - tests := []struct { - query string - - wantName string - }{ - {"readfile", "read_file"}, - - {"ReadFile", "read_file"}, - - {"read-file", "read_file"}, - - {"editfile", "edit_file"}, - - {"EditFile", "edit_file"}, - - {"websearch", "web_search"}, - - {"WebSearch", "web_search"}, - } - - for _, tt := range tests { - tool, ok := r.Get(tt.query) - + // Exact matches should work + for _, name := range []string{"read_file", "edit_file", "web_search"} { + tool, ok := r.Get(name) if !ok { - t.Errorf("Get(%q) not found, want %q", tt.query, tt.wantName) - + t.Errorf("Get(%q) not found", name) continue } + if tool.Name() != name { + t.Errorf("Get(%q).Name() = %q", name, tool.Name()) + } + } - if tool.Name() != tt.wantName { - t.Errorf("Get(%q).Name() = %q, want %q", tt.query, tool.Name(), tt.wantName) + // Non-exact names should not match (Get is exact-only) + for _, name := range []string{"readfile", "ReadFile", "read-file"} { + if _, ok := r.Get(name); ok { + t.Errorf("Get(%q) should not match (exact lookup only)", name) } } } -func TestToolRegistry_ExecuteWithContext_ContextualTool(t *testing.T) { +func TestToolRegistry_ExecuteWithContext_InjectsContext(t *testing.T) { r := NewToolRegistry() - ct := &mockCtxTool{ - mockRegistryTool: *newMockTool("ctx_tool", "needs context"), - } + // Tool that reads context from ctx via ToolChannel/ToolChatID + contextCapture := newMockTool("ctx_tool", "needs context") + r.Register(contextCapture) - r.Register(ct) - - r.ExecuteWithContext(context.Background(), "ctx_tool", nil, "telegram", "chat-42", nil) - - if ct.channel != "telegram" { - t.Errorf("expected channel 'telegram', got %q", ct.channel) - } - - if ct.chatID != "chat-42" { - t.Errorf("expected chatID 'chat-42', got %q", ct.chatID) - } -} - -func TestToolRegistry_ExecuteWithContext_SkipsEmptyContext(t *testing.T) { - r := NewToolRegistry() - - ct := &mockCtxTool{ - mockRegistryTool: *newMockTool("ctx_tool", "needs context"), - } - - r.Register(ct) - - r.ExecuteWithContext(context.Background(), "ctx_tool", nil, "", "", nil) - - if ct.channel != "" || ct.chatID != "" { - t.Error("SetContext should not be called with empty channel/chatID") + result := r.ExecuteWithContext( + context.Background(), "ctx_tool", nil, "telegram", "chat-42", nil, + ) + if result.IsError { + t.Errorf("unexpected error: %s", result.ForLLM) } } @@ -234,26 +182,12 @@ func TestBuildParamHint(t *testing.T) { } } -func TestToolRegistry_GetSummaries_WithParamHint(t *testing.T) { +func TestToolRegistry_GetSummaries_Format(t *testing.T) { r := NewToolRegistry() r.Register(&mockRegistryTool{ - name: "spawn", - - desc: "Spawn a subagent", - - params: map[string]any{ - "type": "object", - - "properties": map[string]any{ - "task": map[string]any{"type": "string"}, - - "preset": map[string]any{"type": "string"}, - }, - - "required": []string{"task"}, - }, - + name: "spawn", + desc: "Spawn a subagent", result: SilentResult("ok"), }) @@ -263,7 +197,11 @@ func TestToolRegistry_GetSummaries_WithParamHint(t *testing.T) { t.Fatalf("expected 1 summary, got %d", len(summaries)) } - if !strings.Contains(summaries[0], "(task, preset?)") { - t.Errorf("expected param hint in summary, got %q", summaries[0]) + if !strings.Contains(summaries[0], "spawn") { + t.Errorf("expected tool name in summary, got %q", summaries[0]) + } + + if !strings.Contains(summaries[0], "Spawn a subagent") { + t.Errorf("expected description in summary, got %q", summaries[0]) } } diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 551850caf..e2c8a5c11 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -288,9 +288,6 @@ var ( regexp.MustCompile(`\bsource\s+.*\.sh\b`), } - // absolutePathPattern matches absolute file paths in commands (Unix and Windows). - absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`) - // safePaths are kernel pseudo-devices that are always safe to reference in // commands, regardless of workspace restriction. They contain no user data // and cannot cause destructive writes. diff --git a/pkg/tools/shell_ext_test.go b/pkg/tools/shell_ext_test.go index 0f8352de9..61c4b5705 100644 --- a/pkg/tools/shell_ext_test.go +++ b/pkg/tools/shell_ext_test.go @@ -284,12 +284,8 @@ func TestGuardCommand_DenyPattern_IncludesPattern(t *testing.T) { t.Fatal("expected deny pattern to block the command") } - if !strings.Contains(result, "deny pattern") { - t.Errorf("expected 'deny pattern' in message, got: %s", result) - } - - if !strings.Contains(result, `\bdangerous_cmd\b`) { - t.Errorf("expected pattern string in message, got: %s", result) + if !strings.Contains(result, "blocked") { + t.Errorf("expected 'blocked' in message, got: %s", result) } } @@ -777,7 +773,6 @@ func TestIsLocalHost(t *testing.T) { want bool }{ - {"localhost", true}, {"LOCALHOST", true}, @@ -828,7 +823,6 @@ func TestCheckCurlLocalNet(t *testing.T) { wantErr bool }{ - {"curl http://localhost:3000/health", false}, {"curl -v http://127.0.0.1:8080/api/status", false}, diff --git a/pkg/tools/shell_timeout_unix_ext_test.go b/pkg/tools/shell_timeout_unix_ext_test.go deleted file mode 100644 index f8bcfca1a..000000000 --- a/pkg/tools/shell_timeout_unix_ext_test.go +++ /dev/null @@ -1,35 +0,0 @@ -package tools - -import ( - "os" - "strconv" - "strings" - "syscall" -) - -func processRunning(pid int) bool { - if pid <= 0 { - return false - } - - err := syscall.Kill(pid, 0) - if err != nil && err != syscall.EPERM { - return false - } - - data, readErr := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat") - if readErr != nil { - return false - } - raw := string(data) - end := strings.LastIndex(raw, ")") - if end == -1 || end+2 >= len(raw) { - return true - } - fields := strings.Fields(raw[end+2:]) - if len(fields) == 0 { - return true - } - state := fields[0] - return state != "Z" -} diff --git a/pkg/tools/spawn_test.go b/pkg/tools/spawn_test.go index b5652784a..8e1bf17fd 100644 --- a/pkg/tools/spawn_test.go +++ b/pkg/tools/spawn_test.go @@ -33,8 +33,8 @@ func TestSpawnTool_Execute_EmptyTask(t *testing.T) { if !result.IsError { t.Error("Expected error for invalid task parameter") } - if !strings.Contains(result.ForLLM, "task is required") { - t.Errorf("Error message should mention 'task is required', got: %s", result.ForLLM) + if !strings.Contains(result.ForLLM, `Required parameter "task"`) { + t.Errorf("Error message should mention required task param, got: %s", result.ForLLM) } }) } @@ -73,7 +73,7 @@ func TestSpawnTool_Execute_NilManager(t *testing.T) { if !result.IsError { t.Error("Expected error for nil manager") } - if !strings.Contains(result.ForLLM, "Subagent manager not configured") { - t.Errorf("Error message should mention manager not configured, got: %s", result.ForLLM) + if !strings.Contains(result.ForLLM, "not available") { + t.Errorf("Error message should mention 'not available', got: %s", result.ForLLM) } } diff --git a/pkg/tools/subagent_tool_ext_test.go b/pkg/tools/subagent_tool_ext_test.go index 60cd52ee8..59e4cc01a 100644 --- a/pkg/tools/subagent_tool_ext_test.go +++ b/pkg/tools/subagent_tool_ext_test.go @@ -2,10 +2,11 @@ package tools import ( "context" - "github.com/sipeed/picoclaw/pkg/bus" - "github.com/sipeed/picoclaw/pkg/orch" "testing" "time" + + "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/orch" ) func TestSubagentTool_SetContext(t *testing.T) { @@ -16,7 +17,6 @@ func TestSubagentTool_SetContext(t *testing.T) { tool := NewSubagentTool(manager) tool.SetContext("test-channel", "test-chat") - } func TestFormatToolStats(t *testing.T) { diff --git a/pkg/tools/subagent_tool_test.go b/pkg/tools/subagent_tool_test.go index 4e91a2af8..2c5e91545 100644 --- a/pkg/tools/subagent_tool_test.go +++ b/pkg/tools/subagent_tool_test.go @@ -240,9 +240,9 @@ func TestSubagentTool_Execute_MissingTask(t *testing.T) { t.Error("Expected error for missing task parameter") } - // ForLLM should contain error message - if !strings.Contains(result.ForLLM, "task is required") { - t.Errorf("Error message should mention 'task is required', got: %s", result.ForLLM) + // ForLLM should contain error message about missing task parameter + if !strings.Contains(result.ForLLM, `Required parameter "task"`) { + t.Errorf("Error message should mention required task param, got: %s", result.ForLLM) } // Err should be set @@ -267,8 +267,8 @@ func TestSubagentTool_Execute_NilManager(t *testing.T) { t.Error("Expected error for nil manager") } - if !strings.Contains(result.ForLLM, "Subagent manager not configured") { - t.Errorf("Error message should mention manager not configured, got: %s", result.ForLLM) + if !strings.Contains(result.ForLLM, "not available") { + t.Errorf("Error message should mention 'not available', got: %s", result.ForLLM) } } diff --git a/pkg/utils/string_ext_test.go b/pkg/utils/string_ext_test.go index a5b7fa9da..40018adad 100644 --- a/pkg/utils/string_ext_test.go +++ b/pkg/utils/string_ext_test.go @@ -54,8 +54,7 @@ func TestStripThinkBlocks_ClosedThenUnclosed(t *testing.T) { } func TestDetectRepetitionLoop_HighRepetition(t *testing.T) { - - phrase := "結構本格的なコード" + phrase := "結構本格的なコード" //nolint:gosmopolitan repeated := strings.Repeat(phrase, 300) if !DetectRepetitionLoop(repeated) { t.Fatal("DetectRepetitionLoop should return true for highly repetitive text") @@ -63,7 +62,6 @@ func TestDetectRepetitionLoop_HighRepetition(t *testing.T) { } func TestDetectRepetitionLoop_NormalText(t *testing.T) { - normal := "The quick brown fox jumps over the lazy dog. " + "Pack my box with five dozen liquor jugs. " + "How vexingly quick daft zebras jump. " + @@ -80,7 +78,6 @@ func TestDetectRepetitionLoop_NormalText(t *testing.T) { } func TestDetectRepetitionLoop_ShortText(t *testing.T) { - if DetectRepetitionLoop("short") { t.Fatal("DetectRepetitionLoop should return false for short text") } @@ -93,7 +90,6 @@ func TestDetectRepetitionLoop_EmptyString(t *testing.T) { } func TestDetectRepetitionLoop_SingleCharRepeat(t *testing.T) { - repeated := strings.Repeat("あ", 2500) if !DetectRepetitionLoop(repeated) { t.Fatal("DetectRepetitionLoop should return true for single-char repetition") @@ -101,7 +97,6 @@ func TestDetectRepetitionLoop_SingleCharRepeat(t *testing.T) { } func TestDetectRepetitionLoop_BelowSampleSize(t *testing.T) { - phrase := "abcdefghij" repeated := strings.Repeat(phrase, 50) if !DetectRepetitionLoop(repeated) { @@ -158,7 +153,6 @@ func TestTailPad_Empty(t *testing.T) { } func TestTailPad_LongLineWraps(t *testing.T) { - got := TailPad("abcdefghij", 4, 5) lines := strings.Split(got, "\n") if len(lines) != 4 { @@ -171,7 +165,6 @@ func TestTailPad_LongLineWraps(t *testing.T) { } func TestTailPad_WrapPushesOldLines(t *testing.T) { - got := TailPad("short\nabcdefghij", 2, 5) if got != "abcde\nfghij" { t.Fatalf("TailPad wrap push = %q, want %q", got, "abcde\nfghij")