From c74dbd4c19037cdbac1bc05d65decf81f87b1388 Mon Sep 17 00:00:00 2001 From: dj-oyu <68707227+dj-oyu@users.noreply.github.com> Date: Sun, 15 Mar 2026 19:02:31 +0900 Subject: [PATCH] refactor: align fork structure with upstream to minimize merge conflicts Three changes to reduce conflict surface when merging upstream: 1. Move Subagents and SkillsFilter from instanceExt back to AgentInstance struct (matches upstream field placement) 2. Remove al *AgentLoop parameter from registerSharedTools to match upstream signature. Orchestration registration now happens via separate registerAllOrchestrationTools method call in NewAgentLoop 3. Move buildToolsSection from context.go to context_ext.go (fork-only code that doesn't exist upstream) Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/agent/context.go | 23 ----------------------- pkg/agent/context_ext.go | 30 +++++++++++++++++++++++++++++- pkg/agent/instance.go | 2 ++ pkg/agent/instance_ext.go | 3 --- pkg/agent/loop.go | 15 +++++++-------- pkg/agent/loop_ext.go | 18 ++++++++++++++++++ 6 files changed, 56 insertions(+), 35 deletions(-) diff --git a/pkg/agent/context.go b/pkg/agent/context.go index e8924036f..13f52b44f 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -157,29 +157,6 @@ Your workspace is at: %s toolsSection, executingRule, toolDiscovery) } -func (cb *ContextBuilder) buildToolsSection() string { - if cb.tools == nil { - return "" - } - - summaries := cb.tools.GetSummaries() - if len(summaries) == 0 { - return "" - } - - var sb strings.Builder - sb.WriteString("## Available Tools\n\n") - sb.WriteString( - "**CRITICAL**: You MUST use tools to perform actions. Do NOT pretend to execute commands or schedule tasks.\n\n", - ) - sb.WriteString("You have access to the following tools:\n\n") - for _, s := range summaries { - sb.WriteString(s) - sb.WriteString("\n") - } - return sb.String() -} - func (cb *ContextBuilder) getDiscoveryRule() string { if !cb.toolDiscoveryBM25 && !cb.toolDiscoveryRegex { return "" diff --git a/pkg/agent/context_ext.go b/pkg/agent/context_ext.go index 6919380d2..5355612e1 100644 --- a/pkg/agent/context_ext.go +++ b/pkg/agent/context_ext.go @@ -1,6 +1,10 @@ package agent -import "github.com/sipeed/picoclaw/pkg/tools" +import ( + "strings" + + "github.com/sipeed/picoclaw/pkg/tools" +) // contextBuilderExt holds fork-specific fields for ContextBuilder. // Embedded in ContextBuilder so existing field access (cb.workDir, cb.tools, etc.) continues to work. @@ -33,6 +37,30 @@ func (cb *ContextBuilder) SetOrchestrationEnabled(enabled bool) { cb.orchestrationEnabled = enabled } +// buildToolsSection generates the "Available Tools" section for the system prompt. +func (cb *ContextBuilder) buildToolsSection() string { + if cb.tools == nil { + return "" + } + + summaries := cb.tools.GetSummaries() + if len(summaries) == 0 { + return "" + } + + var sb strings.Builder + sb.WriteString("## Available Tools\n\n") + sb.WriteString( + "**CRITICAL**: You MUST use tools to perform actions. Do NOT pretend to execute commands or schedule tasks.\n\n", + ) + sb.WriteString("You have access to the following tools:\n\n") + for _, s := range summaries { + sb.WriteString(s) + sb.WriteString("\n") + } + return sb.String() +} + // extIdentityOverrides returns the orchestration-specific overrides for // getIdentity: banner prefix, identity string, and plan executing rule. // When orchestration is disabled, all return values are empty strings. diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index c0ede9a9c..3bd0fa7e0 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -37,6 +37,8 @@ type AgentInstance struct { Sessions *session.LegacyAdapter ContextBuilder *ContextBuilder Tools *tools.ToolRegistry + Subagents *config.SubagentsConfig + SkillsFilter []string Candidates []providers.FallbackCandidate PlanModel string PlanFallbacks []string diff --git a/pkg/agent/instance_ext.go b/pkg/agent/instance_ext.go index 4a76a819e..830a3bb2c 100644 --- a/pkg/agent/instance_ext.go +++ b/pkg/agent/instance_ext.go @@ -18,9 +18,6 @@ type instanceExt struct { // Used by runAgentLoop to wait for spawned subagents before worktree cleanup. SubagentMgr *tools.SubagentManager - Subagents *config.SubagentsConfig - SkillsFilter []string - // Interview staleness tracking: consecutive turns where MEMORY.md was not updated. interviewStaleCount int interviewMemoryLen int diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 8a295ab8b..ae1671717 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -155,8 +155,11 @@ func NewAgentLoop( // Initialize fork-specific fields (stats, sessions, orchestration, gcLoop). al.initLoopExt(cfg, registry, len(enableStats) > 0 && enableStats[0]) - // Register shared tools to all agents (needs al for reporter injection). - registerSharedTools(cfg, msgBus, registry, provider, al) + // Register shared tools to all agents. + registerSharedTools(cfg, msgBus, registry, provider) + + // Register fork-specific orchestration tools (needs al for reporter injection). + al.registerAllOrchestrationTools(cfg, registry, provider, msgBus) return al } @@ -167,8 +170,6 @@ func registerSharedTools( msgBus *bus.MessageBus, registry *AgentRegistry, provider providers.LLMProvider, - - al *AgentLoop, ) { for _, agentID := range registry.ListAgentIDs() { agent, ok := registry.GetAgent(agentID) @@ -298,9 +299,6 @@ func registerSharedTools( } } - // Orchestration tools (spawn, subagent, answer, review_plan) - registerOrchestrationTools(cfg, agent, agentID, registry, provider, msgBus, al) - // Update context builder with the complete tools registry agent.ContextBuilder.SetToolsRegistry(agent.Tools) } @@ -644,7 +642,8 @@ func (al *AgentLoop) ReloadProviderAndConfig( } // Ensure shared tools are re-registered on the new registry - registerSharedTools(cfg, al.bus, registry, provider, al) + registerSharedTools(cfg, al.bus, registry, provider) + al.registerAllOrchestrationTools(cfg, registry, provider, al.bus) // Atomically swap the config and registry under write lock // This ensures readers see a consistent pair diff --git a/pkg/agent/loop_ext.go b/pkg/agent/loop_ext.go index 4ef4aaecf..46807cbb0 100644 --- a/pkg/agent/loop_ext.go +++ b/pkg/agent/loop_ext.go @@ -107,6 +107,24 @@ func (al *AgentLoop) SetHeartbeatThreadUpdater(fn func(int)) { al.onHeartbeatThreadUpdate = fn } +// registerAllOrchestrationTools iterates all agents and registers orchestration +// tools for those with subagents enabled. Called from NewAgentLoop after +// registerSharedTools, keeping the upstream function signature clean. +func (al *AgentLoop) registerAllOrchestrationTools( + cfg *config.Config, + registry *AgentRegistry, + provider providers.LLMProvider, + msgBus *bus.MessageBus, +) { + for _, agentID := range registry.ListAgentIDs() { + agent, ok := registry.GetAgent(agentID) + if !ok { + continue + } + registerOrchestrationTools(cfg, agent, agentID, registry, provider, msgBus, al) + } +} + // registerOrchestrationTools registers spawn, subagent, answer, and review_plan // tools for agents with orchestration enabled. func registerOrchestrationTools(