This commit is contained in:
Costin Stroie 2026-05-15 09:24:52 +03:00 committed by GitHub
commit e2461838e1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 238 additions and 49 deletions

View file

@ -242,14 +242,14 @@ build-whatsapp-native: generate
@echo "Building for multiple platforms..."
@mkdir -p $(BUILD_DIR)
GOOS=linux GOARCH=amd64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-amd64 ./$(CMD_DIR)
GOOS=linux GOARCH=arm GOARM=7 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-arm ./$(CMD_DIR)
GOOS=linux GOARCH=arm64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-arm64 ./$(CMD_DIR)
GOOS=linux GOARCH=loong64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-loong64 ./$(CMD_DIR)
GOOS=linux GOARCH=riscv64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-riscv64 ./$(CMD_DIR)
GOOS=linux GOARCH=mipsle GOMIPS=softfloat $(GO) build -tags $(GO_BUILD_TAGS_NO_GOOLM),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-mipsle ./$(CMD_DIR)
$(call PATCH_MIPS_FLAGS,$(BUILD_DIR)/$(BINARY_NAME)-linux-mipsle)
GOOS=darwin GOARCH=arm64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-darwin-arm64 ./$(CMD_DIR)
GOOS=windows GOARCH=amd64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-windows-amd64.exe ./$(CMD_DIR)
#GOOS=linux GOARCH=arm GOARM=7 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-arm ./$(CMD_DIR)
#GOOS=linux GOARCH=arm64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-arm64 ./$(CMD_DIR)
#GOOS=linux GOARCH=loong64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-loong64 ./$(CMD_DIR)
#GOOS=linux GOARCH=riscv64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-riscv64 ./$(CMD_DIR)
#GOOS=linux GOARCH=mipsle GOMIPS=softfloat $(GO) build -tags $(GO_BUILD_TAGS_NO_GOOLM),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-mipsle ./$(CMD_DIR)
#$(call PATCH_MIPS_FLAGS,$(BUILD_DIR)/$(BINARY_NAME)-linux-mipsle)
#GOOS=darwin GOARCH=arm64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-darwin-arm64 ./$(CMD_DIR)
#GOOS=windows GOARCH=amd64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-windows-amd64.exe ./$(CMD_DIR)
## @$(GO) build $(GOFLAGS) -tags whatsapp_native -ldflags "$(LDFLAGS)" -o $(BINARY_PATH) ./$(CMD_DIR)
@echo "Build complete"
## @ln -sf $(BINARY_NAME)-$(PLATFORM)-$(ARCH) $(BUILD_DIR)/$(BINARY_NAME)

View file

@ -126,6 +126,41 @@ For advanced/test setups, you can override the builtin skills root with:
export PICOCLAW_BUILTIN_SKILLS=/path/to/skills
```
### Skill Catalog Token Optimization
By default, the skill catalog (the list of available skills shown to the LLM) is included in every LLM request. On providers without prompt caching (most OpenAI-compatible APIs), this costs tokens on every call including intermediate tool round-trips within a single turn.
Two opt-in flags under `agents.defaults.skill_catalog` reduce this cost:
```json
"agents": {
"defaults": {
"skill_catalog": {
"skip_on_tools": false,
"skip_on_subsequent": false
}
}
}
```
| Field | Default | Effect when `true` |
|---|---|---|
| `skip_on_tools` | `false` | Omits the catalog from tool-call continuation requests (mid-turn LLM round-trips). The LLM already received the catalog on the initial turn request. |
| `skip_on_subsequent` | `false` | Omits the catalog on turns after the first in a session. The catalog is automatically re-injected after context compaction, so the LLM always rediscovers skills when its history is summarized. |
Both flags default to `false` so existing deployments are unaffected. Enable both for maximum token savings:
```json
"skill_catalog": {
"skip_on_tools": true,
"skip_on_subsequent": true
}
```
Environment variable equivalents:
- `PICOCLAW_AGENTS_DEFAULTS_SKILL_CATALOG_SKIP_ON_TOOLS=true`
- `PICOCLAW_AGENTS_DEFAULTS_SKILL_CATALOG_SKIP_ON_SUBSEQUENT=true`
### Using Skills From Chat Channels
Once skills are installed, and MCP servers are configured, you can inspect and force them directly from a chat channel:

View file

@ -22,12 +22,13 @@ import (
)
type ContextBuilder struct {
workspace string
skillsLoader *skills.SkillsLoader
memory *MemoryStore
splitOnMarker bool
agentDiscovery func(agentID string) []AgentDescriptor
promptRegistry *PromptRegistry
workspace string
skillsLoader *skills.SkillsLoader
memory *MemoryStore
splitOnMarker bool
skillCatalogCfg config.SkillCatalogConfig
agentDiscovery func(agentID string) []AgentDescriptor
promptRegistry *PromptRegistry
// Cache for system prompt to avoid rebuilding on every call.
// This fixes issue #607: repeated reprocessing of the entire context.
@ -67,6 +68,11 @@ func (cb *ContextBuilder) WithSplitOnMarker(enabled bool) *ContextBuilder {
return cb
}
func (cb *ContextBuilder) WithSkillCatalogConfig(cfg config.SkillCatalogConfig) *ContextBuilder {
cb.skillCatalogCfg = cfg
return cb
}
func (cb *ContextBuilder) WithAgentDiscovery(
agentID string,
discover func(agentID string) []AgentDescriptor,
@ -221,25 +227,6 @@ func (cb *ContextBuilder) BuildSystemPromptParts() []PromptPart {
})
}
// Skills - show summary, AI can read full content with read_file tool
skillsSummary := cb.skillsLoader.BuildSkillsSummary()
if skillsSummary != "" {
add(PromptPart{
ID: "capability.skill_catalog",
Layer: PromptLayerCapability,
Slot: PromptSlotSkillCatalog,
Source: PromptSource{ID: PromptSourceSkillCatalog, Name: "skill:index"},
Title: "skill catalog",
Content: fmt.Sprintf(`# Skills
The following skills extend your capabilities. To use a skill, read its SKILL.md file using the read_file tool.
%s`, skillsSummary),
Stable: true,
Cache: PromptCacheEphemeral,
})
}
// Memory context
memoryContext := cb.memory.GetMemoryContext()
if memoryContext != "" {
@ -333,6 +320,13 @@ func (cb *ContextBuilder) EstimateSystemTokens(summary string, activeSkills []st
totalChars := utf8.RuneCountInString(staticPrompt) + dynamicContextChars
// Skill catalog is no longer in the static prompt; add it to the estimate
// (EstimateSystemTokens assumes a non-continuation turn).
if skillsSummary := cb.skillsLoader.BuildSkillsSummary(); skillsSummary != "" {
totalChars += utf8.RuneCountInString(skillsSummary) + 80 // header overhead
totalChars += 7 // separator
}
if skillsText := cb.buildActiveSkillsContext(activeSkills); skillsText != "" {
totalChars += utf8.RuneCountInString(skillsText)
totalChars += 7 // separator \n\n---\n\n
@ -722,6 +716,37 @@ func (cb *ContextBuilder) BuildMessagesFromPrompt(req PromptBuildRequest) []prov
}, &providers.CacheControl{Type: "ephemeral"}),
}
// Determine whether to inject the skill catalog.
// Both skip behaviors are opt-in via config (default: always include).
isToolContinuation := len(req.History) > 0 && req.History[len(req.History)-1].Role == "tool"
isFirstTurn := len(req.History) == 0
isAfterCompaction := req.Summary != ""
skipForTools := cb.skillCatalogCfg.SkipOnTools && isToolContinuation
skipForSubsequent := cb.skillCatalogCfg.SkipOnSubsequent &&
!isFirstTurn && !isAfterCompaction && !isToolContinuation
if !skipForTools && !skipForSubsequent {
if skillsSummary := cb.skillsLoader.BuildSkillsSummary(); skillsSummary != "" {
catalogPart := PromptPart{
ID: "capability.skill_catalog",
Layer: PromptLayerCapability,
Slot: PromptSlotSkillCatalog,
Source: PromptSource{ID: PromptSourceSkillCatalog, Name: "skill:index"},
Title: "skill catalog",
Content: fmt.Sprintf(
"# Skills\n\nThe following skills extend your capabilities. To use a skill, read its SKILL.md file using the read_file tool.\n\n%s",
skillsSummary,
),
Stable: true,
Cache: PromptCacheEphemeral,
}
stringParts = append(stringParts, catalogPart.Content)
contentBlocks = append(
contentBlocks,
promptContentBlock(catalogPart, &providers.CacheControl{Type: "ephemeral"}),
)
}
}
promptParts := append([]PromptPart(nil), req.Overlays...)
promptParts = append(promptParts, cb.buildActiveSkillsPromptParts(req.ActiveSkills)...)
if contributedParts, err := cb.promptRegistryOrDefault().Collect(context.Background(), req); err != nil {

View file

@ -8,6 +8,7 @@ import (
"testing"
"time"
"github.com/sipeed/picoclaw/pkg/config"
"github.com/sipeed/picoclaw/pkg/providers"
)
@ -31,6 +32,16 @@ func setupWorkspace(t *testing.T, files map[string]string) string {
return tmpDir
}
// systemPromptFromMessages extracts the Content of the first system message.
func systemPromptFromMessages(msgs []providers.Message) string {
for _, m := range msgs {
if m.Role == "system" {
return m.Content
}
}
return ""
}
// TestSingleSystemMessage verifies that BuildMessages always produces exactly one
// system message regardless of summary/history variations.
// Fix: multiple system messages break Anthropic (top-level system param) and
@ -468,8 +479,9 @@ description: global-v1
}
cb := NewContextBuilder(tmpDir)
sp1 := cb.BuildSystemPromptWithCache()
if !strings.Contains(sp1, "global-v1") {
// Skill catalog is injected per-request, not in the static cache; check via BuildMessagesFromPrompt.
sysMsg1 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{}))
if !strings.Contains(sysMsg1, "global-v1") {
t.Fatal("expected initial prompt to contain global skill description")
}
@ -493,11 +505,11 @@ description: global-v2
t.Fatal("sourceFilesChangedLocked() should detect global skill file content change")
}
sp2 := cb.BuildSystemPromptWithCache()
if !strings.Contains(sp2, "global-v2") {
sysMsg2 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{}))
if !strings.Contains(sysMsg2, "global-v2") {
t.Error("rebuilt prompt should contain updated global skill description")
}
if sp1 == sp2 {
if sysMsg1 == sysMsg2 {
t.Error("cache should be invalidated when global skill file content changes")
}
}
@ -528,8 +540,9 @@ description: builtin-v1
}
cb := NewContextBuilder(tmpDir)
sp1 := cb.BuildSystemPromptWithCache()
if !strings.Contains(sp1, "builtin-v1") {
// Skill catalog is injected per-request, not in the static cache; check via BuildMessagesFromPrompt.
sysMsg1 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{}))
if !strings.Contains(sysMsg1, "builtin-v1") {
t.Fatal("expected initial prompt to contain builtin skill description")
}
@ -553,11 +566,11 @@ description: builtin-v2
t.Fatal("sourceFilesChangedLocked() should detect builtin skill file content change")
}
sp2 := cb.BuildSystemPromptWithCache()
if !strings.Contains(sp2, "builtin-v2") {
sysMsg2 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{}))
if !strings.Contains(sysMsg2, "builtin-v2") {
t.Error("rebuilt prompt should contain updated builtin skill description")
}
if sp1 == sp2 {
if sysMsg1 == sysMsg2 {
t.Error("cache should be invalidated when builtin skill file content changes")
}
}
@ -575,8 +588,9 @@ description: delete-me-v1
defer os.RemoveAll(tmpDir)
cb := NewContextBuilder(tmpDir)
sp1 := cb.BuildSystemPromptWithCache()
if !strings.Contains(sp1, "delete-me-v1") {
// Skill catalog is injected per-request, not in the static cache; check via BuildMessagesFromPrompt.
sysMsg1 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{}))
if !strings.Contains(sysMsg1, "delete-me-v1") {
t.Fatal("expected initial prompt to contain skill description")
}
@ -592,15 +606,117 @@ description: delete-me-v1
t.Fatal("sourceFilesChangedLocked() should detect deleted skill file")
}
sp2 := cb.BuildSystemPromptWithCache()
if strings.Contains(sp2, "delete-me-v1") {
sysMsg2 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{}))
if strings.Contains(sysMsg2, "delete-me-v1") {
t.Error("rebuilt prompt should not contain deleted skill description")
}
if sp1 == sp2 {
if sysMsg1 == sysMsg2 {
t.Error("cache should be invalidated when skill file is deleted")
}
}
// TestSkillCatalogInjectionPolicy verifies catalog inclusion under various
// config combinations.
func TestSkillCatalogInjectionPolicy(t *testing.T) {
tmpDir := setupWorkspace(t, map[string]string{
"skills/demo/SKILL.md": "---\nname: demo\ndescription: \"demo skill\"\n---\n# Demo",
})
defer os.RemoveAll(tmpDir)
userMsg := providers.Message{Role: "user", Content: "hello"}
assistantMsg := providers.Message{Role: "assistant", Content: "hi"}
toolMsg := providers.Message{Role: "tool", Content: "result", ToolCallID: "tc1"}
contains := func(msgs []providers.Message) bool {
return strings.Contains(systemPromptFromMessages(msgs), "demo skill")
}
newCB := func(skipOnTools, skipOnSubsequent bool) *ContextBuilder {
return NewContextBuilder(tmpDir).WithSkillCatalogConfig(config.SkillCatalogConfig{
SkipOnTools: skipOnTools,
SkipOnSubsequent: skipOnSubsequent,
})
}
t.Run("default (both false): catalog always included", func(t *testing.T) {
cb := newCB(false, false)
for _, req := range []PromptBuildRequest{
{},
{History: []providers.Message{userMsg, assistantMsg}},
{History: []providers.Message{userMsg, assistantMsg, toolMsg}},
{History: []providers.Message{userMsg, assistantMsg}, Summary: "summary"},
} {
if !contains(cb.BuildMessagesFromPrompt(req)) {
t.Error("catalog should always be included when both flags are false")
}
}
})
t.Run("skip_on_tools: skips tool continuations only", func(t *testing.T) {
cb := newCB(true, false)
if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) {
t.Error("turn 1: catalog should be included")
}
if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{
History: []providers.Message{userMsg, assistantMsg},
})) {
t.Error("turn > 1 (no tool): catalog should be included")
}
if contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{
History: []providers.Message{userMsg, assistantMsg, toolMsg},
})) {
t.Error("tool continuation: catalog should be skipped")
}
})
t.Run("skip_on_subsequent: skips turns > 1, re-injects after compaction", func(t *testing.T) {
cb := newCB(false, true)
if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) {
t.Error("turn 1: catalog should be included")
}
if contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{
History: []providers.Message{userMsg, assistantMsg},
})) {
t.Error("turn > 1, no summary: catalog should be skipped")
}
if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{
History: []providers.Message{userMsg, assistantMsg},
Summary: "prior conversation summary",
})) {
t.Error("after compaction: catalog should be re-injected")
}
// tool continuation is NOT skipped when only skip_on_subsequent is set
if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{
History: []providers.Message{userMsg, assistantMsg, toolMsg},
})) {
t.Error("tool continuation (skip_on_subsequent only): catalog should be included")
}
})
t.Run("both true: skips tool turns and subsequent turns, re-injects after compaction", func(t *testing.T) {
cb := newCB(true, true)
if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) {
t.Error("turn 1: catalog should be included")
}
if contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{
History: []providers.Message{userMsg, assistantMsg, toolMsg},
})) {
t.Error("tool continuation: catalog should be skipped")
}
if contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{
History: []providers.Message{userMsg, assistantMsg},
})) {
t.Error("turn > 1, no summary: catalog should be skipped")
}
if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{
History: []providers.Message{userMsg, assistantMsg},
Summary: "prior conversation summary",
})) {
t.Error("after compaction: catalog should be re-injected")
}
})
}
// TestConcurrentBuildSystemPromptWithCache verifies that multiple goroutines
// can safely call BuildSystemPromptWithCache concurrently without producing
// empty results, panics, or data races.

View file

@ -134,7 +134,8 @@ func NewAgentInstance(
mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseBM25,
mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseRegex,
).
WithSplitOnMarker(cfg.Agents.Defaults.SplitOnMarker)
WithSplitOnMarker(cfg.Agents.Defaults.SplitOnMarker).
WithSkillCatalogConfig(cfg.Agents.Defaults.SkillCatalog)
agentID := routing.DefaultAgentID
agentName := ""

View file

@ -375,6 +375,17 @@ type ToolFeedbackConfig struct {
SeparateMessages bool `json:"separate_messages" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_SEPARATE_MESSAGES"`
}
type SkillCatalogConfig struct {
// SkipOnTools omits the skill catalog from tool-call continuation requests
// (mid-turn LLM round-trips). The LLM already received the catalog on the
// initial turn request. Default false (catalog always included).
SkipOnTools bool `json:"skip_on_tools" env:"PICOCLAW_AGENTS_DEFAULTS_SKILL_CATALOG_SKIP_ON_TOOLS"`
// SkipOnSubsequent omits the skill catalog on turns after the first in a
// session. The catalog is still re-injected after context compaction.
// Default false (catalog always included).
SkipOnSubsequent bool `json:"skip_on_subsequent" env:"PICOCLAW_AGENTS_DEFAULTS_SKILL_CATALOG_SKIP_ON_SUBSEQUENT"`
}
type AgentDefaults struct {
Workspace string `json:"workspace" env:"PICOCLAW_AGENTS_DEFAULTS_WORKSPACE"`
RestrictToWorkspace bool `json:"restrict_to_workspace" env:"PICOCLAW_AGENTS_DEFAULTS_RESTRICT_TO_WORKSPACE"`
@ -397,6 +408,7 @@ type AgentDefaults struct {
SubTurn SubTurnConfig `json:"subturn" envPrefix:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_"`
ToolFeedback ToolFeedbackConfig `json:"tool_feedback,omitempty"`
SplitOnMarker bool `json:"split_on_marker" env:"PICOCLAW_AGENTS_DEFAULTS_SPLIT_ON_MARKER"` // split messages on <|[SPLIT]|> marker
SkillCatalog SkillCatalogConfig `json:"skill_catalog,omitempty"`
ContextManager string `json:"context_manager,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_CONTEXT_MANAGER"`
ContextManagerConfig json.RawMessage `json:"context_manager_config,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_CONTEXT_MANAGER_CONFIG"`
MaxLLMRetries int `json:"max_llm_retries,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_LLM_RETRIES"`