lzh/picoclaw
1
0
Fork 0
mirror of https://github.com/sipeed/picoclaw.git synced 2026-06-12 18:08:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: improve code readability and consistency across multiple files

Browse Source
This commit is contained in:
Administrator
2026-03-21 17:12:45 +08:00
parent 4f646ef2b8
commit 087e8519c5
9 changed files with 133 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/agent/subturn.go
+34 -9
View File
@@ -138,11 +138,11 @@ type SubTurnConfig struct {
// - Critical=false: SubTurn exits gracefully without error
//
// When parent finishes with hard abort (Finish(true)):
// - All SubTurns are cancelled regardless of Critical flag
// - All SubTurns are canceled regardless of Critical flag
Critical bool
// Timeout is the maximum duration for this SubTurn.
// If the SubTurn runs longer than this, it will be cancelled.
// If the SubTurn runs longer than this, it will be canceled.
// Default is 5 minutes (defaultSubTurnTimeout) if not specified.
Timeout time.Duration
@@ -177,6 +177,8 @@ type SubTurnConfig struct {
}
// ====================== Sub-turn Events (Aligned with EventBus) ======================
// SubTurnSpawnEvent is emitted when a child sub-turn is started.
type SubTurnSpawnEvent struct {
ParentID string
ChildID string
@@ -232,10 +234,15 @@ type AgentLoopSpawner struct {
}
// SpawnSubTurn implements tools.SubTurnSpawner interface.
func (s *AgentLoopSpawner) SpawnSubTurn(ctx context.Context, cfg tools.SubTurnConfig) (*tools.ToolResult, error) {
func (s *AgentLoopSpawner) SpawnSubTurn(
ctx context.Context,
cfg tools.SubTurnConfig,
) (*tools.ToolResult, error) {
parentTS := turnStateFromContext(ctx)
if parentTS == nil {
return nil, errors.New("parent turnState not found in context - cannot spawn sub-turn outside of a turn")
return nil, errors.New(
"parent turnState not found in context - cannot spawn sub-turn outside of a turn",
)
}
// Convert tools.SubTurnConfig to agent.SubTurnConfig
@@ -266,18 +273,27 @@ func NewSubTurnSpawner(al *AgentLoop) *AgentLoopSpawner {
func SpawnSubTurn(ctx context.Context, cfg SubTurnConfig) (*tools.ToolResult, error) {
al := AgentLoopFromContext(ctx)
if al == nil {
return nil, errors.New("AgentLoop not found in context - ensure context is properly initialized")
return nil, errors.New(
"AgentLoop not found in context - ensure context is properly initialized",
)
}
parentTS := turnStateFromContext(ctx)
if parentTS == nil {
return nil, errors.New("parent turnState not found in context - cannot spawn sub-turn outside of a turn")
return nil, errors.New(
"parent turnState not found in context - cannot spawn sub-turn outside of a turn",
)
}
return spawnSubTurn(ctx, al, parentTS, cfg)
}
func spawnSubTurn(ctx context.Context, al *AgentLoop, parentTS *turnState, cfg SubTurnConfig) (result *tools.ToolResult, err error) {
func spawnSubTurn(
ctx context.Context,
al *AgentLoop,
parentTS *turnState,
cfg SubTurnConfig,
) (result *tools.ToolResult, err error) {
// Get effective SubTurn configuration
rtCfg := al.getSubTurnConfig()
@@ -512,7 +528,12 @@ func deliverSubTurnResult(parentTS *turnState, childID string, result *tools.Too
// - Injects recovery prompt asking for shorter response
// - Retries up to 2 times
// - Handles cases where max_tokens is hit
func runTurn(ctx context.Context, al *AgentLoop, ts *turnState, cfg SubTurnConfig) (*tools.ToolResult, error) {
func runTurn(
ctx context.Context,
al *AgentLoop,
ts *turnState,
cfg SubTurnConfig,
) (*tools.ToolResult, error) {
// Derive candidates from the requested model using the parent loop's provider.
defaultProvider := al.GetConfig().Agents.Defaults.Provider
candidates := providers.ResolveCandidates(
@@ -639,7 +660,11 @@ func runTurn(ctx context.Context, al *AgentLoop, ts *turnState, cfg SubTurnConfi
"retries": contextRetryCount,
"max_retries": maxContextRetries,
})
return nil, fmt.Errorf("context limit exceeded after %d retries: %w", maxContextRetries, err)
return nil, fmt.Errorf(
"context limit exceeded after %d retries: %w",
maxContextRetries,
err,
)
}
logger.WarnCF("subturn", "Context length exceeded, compressing and retrying",
pkg/agent/subturn_test.go
+6 -24
View File
@@ -434,15 +434,9 @@ func TestHardAbortCascading(t *testing.T) {
childCtx, childCancel := context.WithCancel(rootTS.ctx)
defer childCancel()
childTS := &turnState{
ctx: childCtx,
cancelFunc: childCancel,
turnID: "child-1",
parentTurnID: sessionKey,
depth: 1,
session: &ephemeralSessionStore{},
pendingResults: make(chan *tools.ToolResult, 16),
concurrencySem: make(chan struct{}, 5),
ctx: childCtx,
}
_ = childCancel
// Attach cancelFunc to rootTS so Finish() can trigger it
rootTS.cancelFunc = parentCancel
@@ -1556,29 +1550,17 @@ func TestGrandchildAbort_CascadingCancellation(t *testing.T) {
parentCtx, parentCancel := context.WithCancel(grandparentTS.ctx)
defer parentCancel()
parentTS := &turnState{
ctx: parentCtx,
turnID: "parent",
parentTurnID: "grandparent",
depth: 1,
session: newEphemeralSession(nil),
pendingResults: make(chan *tools.ToolResult, 16),
concurrencySem: make(chan struct{}, testMaxConcurrentSubTurns),
ctx: parentCtx,
}
parentTS.cancelFunc = parentCancel
_ = parentCancel
// Create grandchild turn (depth 2) as child of parent
childCtx, childCancel := context.WithCancel(parentTS.ctx)
defer childCancel()
childTS := &turnState{
ctx: childCtx,
turnID: "grandchild",
parentTurnID: "parent",
depth: 2,
session: newEphemeralSession(nil),
pendingResults: make(chan *tools.ToolResult, 16),
concurrencySem: make(chan struct{}, testMaxConcurrentSubTurns),
ctx: childCtx,
}
childTS.cancelFunc = childCancel
_ = childCancel
// Verify all contexts are active
select {
pkg/agent/turn_state.go
+10 -1
View File
@@ -165,7 +165,16 @@ func (al *AgentLoop) FormatTree(turnInfo *TurnInfo, prefix string, isLast bool)
orphanMarker = " (Orphaned)"
}
fmt.Fprintf(&sb, "%s%s[%s] Depth:%d (%s)%s\n", prefix, marker, turnInfo.TurnID, turnInfo.Depth, status, orphanMarker)
fmt.Fprintf(
&sb,
"%s%s[%s] Depth:%d (%s)%s\n",
prefix,
marker,
turnInfo.TurnID,
turnInfo.Depth,
status,
orphanMarker,
)
// Prepare prefix for children
childPrefix := prefix
pkg/config/config.go
+32 -22
View File
@@ -221,11 +221,11 @@ type RoutingConfig struct {
// SubTurnConfig configures the SubTurn execution system.
type SubTurnConfig struct {
MaxDepth int `json:"max_depth" env:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_MAX_DEPTH"`
MaxConcurrent int `json:"max_concurrent" env:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_MAX_CONCURRENT"`
DefaultTimeoutMinutes int `json:"default_timeout_minutes" env:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_DEFAULT_TIMEOUT_MINUTES"`
DefaultTokenBudget int `json:"default_token_budget" env:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_DEFAULT_TOKEN_BUDGET"`
ConcurrencyTimeoutSec int `json:"concurrency_timeout_sec" env:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_CONCURRENCY_TIMEOUT_SEC"`
MaxDepth int `json:"max_depth" env:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_MAX_DEPTH"`
MaxConcurrent int `json:"max_concurrent" env:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_MAX_CONCURRENT"`
DefaultTimeoutMinutes int `json:"default_timeout_minutes" env:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_DEFAULT_TIMEOUT_MINUTES"`
DefaultTokenBudget int `json:"default_token_budget" env:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_DEFAULT_TOKEN_BUDGET"`
ConcurrencyTimeoutSec int `json:"concurrency_timeout_sec" env:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_CONCURRENCY_TIMEOUT_SEC"`
}
type ToolFeedbackConfig struct {
@@ -251,7 +251,7 @@ type AgentDefaults struct {
MaxMediaSize int `json:"max_media_size,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_MEDIA_SIZE"`
Routing *RoutingConfig `json:"routing,omitempty"`
SteeringMode string `json:"steering_mode,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_STEERING_MODE"` // "one-at-a-time" (default) or "all"
SubTurn SubTurnConfig `json:"subturn" envPrefix:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_"`
SubTurn SubTurnConfig `json:"subturn" envPrefix:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_"`
ToolFeedback ToolFeedbackConfig `json:"tool_feedback,omitempty"`
}
@@ -721,9 +721,9 @@ type SearXNGConfig struct {
}
type GLMSearchConfig struct {
Enabled bool `json:"enabled" env:"PICOCLAW_TOOLS_WEB_GLM_ENABLED"`
APIKey string `json:"api_key" env:"PICOCLAW_TOOLS_WEB_GLM_API_KEY"`
BaseURL string `json:"base_url" env:"PICOCLAW_TOOLS_WEB_GLM_BASE_URL"`
Enabled bool `json:"enabled" env:"PICOCLAW_TOOLS_WEB_GLM_ENABLED"`
APIKey string `json:"api_key" env:"PICOCLAW_TOOLS_WEB_GLM_API_KEY"`
BaseURL string `json:"base_url" env:"PICOCLAW_TOOLS_WEB_GLM_BASE_URL"`
// SearchEngine specifies the search backend: "search_std" (default),
// "search_pro", "search_pro_sogou", or "search_pro_quark".
SearchEngine string `json:"search_engine" env:"PICOCLAW_TOOLS_WEB_GLM_SEARCH_ENGINE"`
@@ -731,7 +731,7 @@ type GLMSearchConfig struct {
}
type WebToolsConfig struct {
ToolConfig ` envPrefix:"PICOCLAW_TOOLS_WEB_"`
ToolConfig ` envPrefix:"PICOCLAW_TOOLS_WEB_"`
Brave BraveConfig ` json:"brave"`
Tavily TavilyConfig ` json:"tavily"`
DuckDuckGo DuckDuckGoConfig ` json:"duckduckgo"`
@@ -743,13 +743,13 @@ type WebToolsConfig struct {
// the client-side web_search tool is hidden to avoid duplicate search surfaces,
// and the provider's built-in search is used instead. Falls back to client-side
// search when the provider does not support native search.
PreferNative bool `json:"prefer_native" env:"PICOCLAW_TOOLS_WEB_PREFER_NATIVE"`
PreferNative bool ` json:"prefer_native" env:"PICOCLAW_TOOLS_WEB_PREFER_NATIVE"`
// Proxy is an optional proxy URL for web tools (http/https/socks5/socks5h).
// For authenticated proxies, prefer HTTP_PROXY/HTTPS_PROXY env vars instead of embedding credentials in config.
Proxy string `json:"proxy,omitempty" env:"PICOCLAW_TOOLS_WEB_PROXY"`
FetchLimitBytes int64 `json:"fetch_limit_bytes,omitempty" env:"PICOCLAW_TOOLS_WEB_FETCH_LIMIT_BYTES"`
Format string `json:"format,omitempty" env:"PICOCLAW_TOOLS_WEB_FORMAT"`
PrivateHostWhitelist FlexibleStringSlice `json:"private_host_whitelist,omitempty" env:"PICOCLAW_TOOLS_WEB_PRIVATE_HOST_WHITELIST"`
Proxy string ` json:"proxy,omitempty" env:"PICOCLAW_TOOLS_WEB_PROXY"`
FetchLimitBytes int64 ` json:"fetch_limit_bytes,omitempty" env:"PICOCLAW_TOOLS_WEB_FETCH_LIMIT_BYTES"`
Format string ` json:"format,omitempty" env:"PICOCLAW_TOOLS_WEB_FORMAT"`
PrivateHostWhitelist FlexibleStringSlice ` json:"private_host_whitelist,omitempty" env:"PICOCLAW_TOOLS_WEB_PRIVATE_HOST_WHITELIST"`
}
type CronToolsConfig struct {
@@ -864,10 +864,10 @@ type MCPServerConfig struct {
// MCPConfig defines configuration for all MCP servers
type MCPConfig struct {
ToolConfig ` envPrefix:"PICOCLAW_TOOLS_MCP_"`
ToolConfig ` envPrefix:"PICOCLAW_TOOLS_MCP_"`
Discovery ToolDiscoveryConfig ` json:"discovery"`
// Servers is a map of server name to server configuration
Servers map[string]MCPServerConfig `json:"servers,omitempty"`
Servers map[string]MCPServerConfig ` json:"servers,omitempty"`
}
func LoadConfig(path string) (*Config, error) {
@@ -901,10 +901,13 @@ func LoadConfig(path string) (*Config, error) {
if passphrase := credential.PassphraseProvider(); passphrase != "" {
for _, m := range cfg.ModelList {
if m.APIKey != "" && !strings.HasPrefix(m.APIKey, "enc://") && !strings.HasPrefix(m.APIKey, "file://") {
fmt.Fprintf(os.Stderr,
if m.APIKey != "" && !strings.HasPrefix(m.APIKey, "enc://") &&
!strings.HasPrefix(m.APIKey, "file://") {
fmt.Fprintf(
os.Stderr,
"picoclaw: warning: model %q has a plaintext api_key; call SaveConfig to encrypt it\n",
m.ModelName)
m.ModelName,
)
}
}
}
@@ -957,7 +960,8 @@ func encryptPlaintextAPIKeys(models []ModelConfig, passphrase string) ([]ModelCo
changed := false
for i := range sealed {
m := &sealed[i]
if m.APIKey == "" || strings.HasPrefix(m.APIKey, "enc://") || strings.HasPrefix(m.APIKey, "file://") {
if m.APIKey == "" || strings.HasPrefix(m.APIKey, "enc://") ||
strings.HasPrefix(m.APIKey, "file://") {
continue
}
encrypted, err := credential.Encrypt(passphrase, "", m.APIKey)
@@ -990,7 +994,13 @@ func resolveAPIKeys(models []ModelConfig, configDir string) error {
for j, key := range models[i].APIKeys {
resolved, err := cr.Resolve(key)
if err != nil {
return fmt.Errorf("model_list[%d] (%s): api_keys[%d]: %w", i, models[i].ModelName, j, err)
return fmt.Errorf(
"model_list[%d] (%s): api_keys[%d]: %w",
i,
models[i].ModelName,
j,
err,
)
}
models[i].APIKeys[j] = resolved
}
pkg/tools/registry.go
-1
View File
@@ -403,4 +403,3 @@ func (r *ToolRegistry) GetAll() []Tool {
}
return tools
}
pkg/tools/spawn.go
+21 -7
View File
@@ -72,11 +72,19 @@ func (t *SpawnTool) Execute(ctx context.Context, args map[string]any) *ToolResul
// ExecuteAsync implements AsyncExecutor. The callback is passed through to the
// subagent manager as a call parameter — never stored on the SpawnTool instance.
func (t *SpawnTool) ExecuteAsync(ctx context.Context, args map[string]any, cb AsyncCallback) *ToolResult {
func (t *SpawnTool) ExecuteAsync(
ctx context.Context,
args map[string]any,
cb AsyncCallback,
) *ToolResult {
return t.execute(ctx, args, cb)
}
func (t *SpawnTool) execute(ctx context.Context, args map[string]any, cb AsyncCallback) *ToolResult {
func (t *SpawnTool) execute(
ctx context.Context,
args map[string]any,
cb AsyncCallback,
) *ToolResult {
task, ok := args["task"].(string)
if !ok || strings.TrimSpace(task) == "" {
return ErrorResult("task is required and must be a non-empty string")
@@ -93,14 +101,21 @@ func (t *SpawnTool) execute(ctx context.Context, args map[string]any, cb AsyncCa
}
// Build system prompt for spawned subagent
systemPrompt := fmt.Sprintf(`You are a spawned subagent running in the background. Complete the given task independently and report back when done.
systemPrompt := fmt.Sprintf(
`You are a spawned subagent running in the background. Complete the given task independently and report back when done.
Task: %s`, task)
Task: %s`,
task,
)
if label != "" {
systemPrompt = fmt.Sprintf(`You are a spawned subagent labeled "%s" running in the background. Complete the given task independently and report back when done.
systemPrompt = fmt.Sprintf(
`You are a spawned subagent labeled "%s" running in the background. Complete the given task independently and report back when done.
Task: %s`, label, task)
Task: %s`,
label,
task,
)
}
// Use spawner if available (direct SpawnSubTurn call)
@@ -115,7 +130,6 @@ Task: %s`, label, task)
Temperature: t.temperature,
Async: true, // Async execution
})
if err != nil {
result = ErrorResult(fmt.Sprintf("Spawn failed: %v", err)).WithError(err)
}
pkg/tools/subagent.go
+27 -7
View File
@@ -147,7 +147,11 @@ func (sm *SubagentManager) Spawn(
return fmt.Sprintf("Spawned subagent for task: %s", task), nil
}
func (sm *SubagentManager) runTask(ctx context.Context, task *SubagentTask, callback AsyncCallback) {
func (sm *SubagentManager) runTask(
ctx context.Context,
task *SubagentTask,
callback AsyncCallback,
) {
task.Status = "running"
task.Created = time.Now().UnixMilli()
@@ -176,7 +180,17 @@ func (sm *SubagentManager) runTask(ctx context.Context, task *SubagentTask, call
var err error
if spawner != nil {
result, err = spawner(ctx, task.Task, task.Label, task.AgentID, tools, maxTokens, temperature, hasMaxTokens, hasTemperature)
result, err = spawner(
ctx,
task.Task,
task.Label,
task.AgentID,
tools,
maxTokens,
temperature,
hasMaxTokens,
hasTemperature,
)
} else {
// Fallback to legacy RunToolLoop
systemPrompt := `You are a subagent. Complete the given task independently and report the result.
@@ -357,14 +371,21 @@ func (t *SubagentTool) Execute(ctx context.Context, args map[string]any) *ToolRe
label, _ := args["label"].(string)
// Build system prompt for subagent
systemPrompt := fmt.Sprintf(`You are a subagent. Complete the given task independently and provide a clear, concise result.
systemPrompt := fmt.Sprintf(
`You are a subagent. Complete the given task independently and provide a clear, concise result.
Task: %s`, task)
Task: %s`,
task,
)
if label != "" {
systemPrompt = fmt.Sprintf(`You are a subagent labeled "%s". Complete the given task independently and provide a clear, concise result.
systemPrompt = fmt.Sprintf(
`You are a subagent labeled "%s". Complete the given task independently and provide a clear, concise result.
Task: %s`, label, task)
Task: %s`,
label,
task,
)
}
// Use spawner if available (direct SpawnSubTurn call)
@@ -377,7 +398,6 @@ Task: %s`, label, task)
Temperature: t.temperature,
Async: false, // Synchronous execution
})
if err != nil {
return ErrorResult(fmt.Sprintf("Subagent execution failed: %v", err)).WithError(err)
}
pkg/utils/context.go
+2 -2
View File
@@ -65,7 +65,7 @@ func MeasureContextRunes(messages []providers.Message) int {
totalRunes += utf8.RuneCountInString(tc.Name)
// Arguments: serialize and count
if argsJSON, err := json.Marshal(tc.Arguments); err == nil {
totalRunes += utf8.RuneCountInString(string(argsJSON))
totalRunes += utf8.RuneCount(argsJSON)
} else {
// Fallback estimate if serialization fails
totalRunes += 100
@@ -136,7 +136,7 @@ func TruncateContextSmart(messages []providers.Message, maxRunes int) []provider
for _, tc := range msg.ToolCalls {
msgRunes += utf8.RuneCountInString(tc.Name)
if argsJSON, err := json.Marshal(tc.Arguments); err == nil {
msgRunes += utf8.RuneCountInString(string(argsJSON))
msgRunes += utf8.RuneCount(argsJSON)
} else {
msgRunes += 100
}
pkg/utils/context_test.go
+1 -1
View File
@@ -156,7 +156,7 @@ func TestMeasureContextRunes(t *testing.T) {
{
name: "unicode characters",
messages: []providers.Message{
{Role: "user", Content: "你好世界"}, // 4 Chinese characters
{Role: "user", Content: "\u4f60\u597d\u4e16\u754c"}, // 4 Chinese characters
},
want: 4,
},