diff --git a/CHANGELOG.md b/CHANGELOG.md index b0057111..6af7871c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Eval-level default expect checks under `cases.defaults.expect`, with append-and-deduplicate semantics for list checks and case-level overrides for `exit_code` and `golden_file`. +- Skill references now support explicit doublestar `include` and `exclude` + patterns for both run-agent and judge Skill installation. ### Fixed - Claude Code session lookup now normalizes Windows workspace paths to the diff --git a/docs/guide/writing-evals.md b/docs/guide/writing-evals.md index 933fba88..fc2e508f 100644 --- a/docs/guide/writing-evals.md +++ b/docs/guide/writing-evals.md @@ -77,6 +77,13 @@ mcp: skills: - source: local_path # local_path (a directory on disk) path: . # Path to the Skill + include: # Optional doublestar globs relative to path; empty means all files + - SKILL.md + - references/** + - scripts/** + exclude: # Optional; applied after include, so exclude wins + - references/drafts/** + - .qoder/repowiki/** # ========== 5. Agent Engine ========== engine: @@ -118,6 +125,15 @@ report: artifacts: [transcript] ``` +`skills[].include` and `skills[].exclude` use +[doublestar](https://github.com/bmatcuk/doublestar) syntax (`*` matches one +path segment and `**` crosses directories). Patterns are relative to +`skills[].path`, use `/` separators, and `exclude` takes precedence over +`include`. When `include` is omitted, every file is selected by default. +The evaluation harness directory `evals/` is always excluded. If an include +list is provided, include `SKILL.md` explicitly so the installed directory +remains a valid Skill. The same fields are supported by `judge.skills`. + `cases.parallelism` is the file-level default. To override it for a single run, use `skill-up run --parallelism N` without modifying `eval.yaml`. Allowed range: **1 to 256**. ### Engine kwargs (agent-specific switches) diff --git a/docs/zh/guide/writing-evals.md b/docs/zh/guide/writing-evals.md index 65929262..a5eae4d6 100644 --- a/docs/zh/guide/writing-evals.md +++ b/docs/zh/guide/writing-evals.md @@ -77,6 +77,13 @@ mcp: skills: - source: local_path # local_path（本地目录） path: . # Skill 所在路径 + include: # 可选：相对 path 的 doublestar glob；不配置表示包含全部文件 + - SKILL.md + - references/** + - scripts/** + exclude: # 可选：在 include 后应用，因此 exclude 优先 + - references/drafts/** + - .qoder/repowiki/** # ========== 5. Agent Engine ========== engine: @@ -117,6 +124,13 @@ report: artifacts: [transcript] # 报告中包含的产物 ``` +`skills[].include` 和 `skills[].exclude` 使用 +[doublestar](https://github.com/bmatcuk/doublestar) 语法（`*` 匹配单层路径， +`**` 可跨目录）。pattern 相对 `skills[].path`，统一使用 `/` 分隔； +`exclude` 优先于 `include`。未配置 `include` 时默认选择全部文件。 +评测框架目录 `evals/` 始终排除。配置 include 列表时，应显式包含 +`SKILL.md`，确保安装后的目录仍是有效 Skill。`judge.skills` 同样支持这两个字段。 + `cases.parallelism` 是配置文件中的默认用例并行数；临时运行时可以用 `skill-up run --parallelism N` 覆盖它，不需要修改 `eval.yaml`。命令行覆盖值必须在 1 到 256 之间。 ### 采集 workspace 产物（`collect_artifacts`） diff --git a/internal/agent/agent.go b/internal/agent/agent.go index fba92ace..cd487df4 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -259,7 +259,7 @@ func (a *BaseAgent) installSkillDefault(ctx context.Context, rt Runtime, skillCf if target == "" && a.Cfg.SkillPath != "" { target = filepath.Join(a.Cfg.SkillPath, filepath.Base(skillCfg.Source)) } - return installSkill(ctx, rt, skillCfg.Source, target) + return installSkill(ctx, rt, skillCfg.Source, target, skillCfg.Include, skillCfg.Exclude) } func persistRuntimeArtifact(ctx context.Context, rt Runtime, targetPath, content string) error { diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index 474d9a1e..a54cdc9f 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" goruntime "runtime" + "slices" "strings" "testing" @@ -38,7 +39,6 @@ func TestListSkillFiles_ExcludesEvals(t *testing.T) { if err := os.WriteFile(filepath.Join(subdir, "README.md"), []byte("readme"), 0o600); err != nil { t.Fatal(err) } - // Create evals directory (should be excluded) evalsDir := filepath.Join(dir, "evals") if err := os.MkdirAll(evalsDir, 0o755); err != nil { @@ -57,7 +57,7 @@ func TestListSkillFiles_ExcludesEvals(t *testing.T) { t.Fatal(err) } - files, err := ListSkillFiles(dir) + files, err := ListSkillFiles(dir, nil, nil) if err != nil { t.Fatalf("ListSkillFiles failed: %v", err) } @@ -74,7 +74,6 @@ func TestListSkillFiles_ExcludesEvals(t *testing.T) { if !fileSet[filepath.Join("subdir", "README.md")] { t.Error("subdir/README.md should be included") } - // Check excluded files if fileSet["evals/test.yaml"] { t.Error("evals/test.yaml should be excluded") @@ -84,6 +83,94 @@ func TestListSkillFiles_ExcludesEvals(t *testing.T) { } } +func TestListSkillFiles_DefaultIncludesHiddenFiles(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + fixtures := map[string]string{ + ".claude/memory.md": "memory", + ".qoder/repowiki/knowledge/zh/_module.yaml": "yaml", + ".qoder/repowiki/knowledge/zh/_module.yamlx": "yamlx", + } + for name, content := range fixtures { + filePath := filepath.Join(dir, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(filePath), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filePath, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + + files, err := ListSkillFiles(dir, nil, nil) + if err != nil { + t.Fatalf("ListSkillFiles failed: %v", err) + } + for name := range fixtures { + if !slices.Contains(files, filepath.FromSlash(name)) { + t.Errorf("%q should be included by default, got %v", name, files) + } + } +} + +func TestListSkillFiles_AppliesIncludeThenExclude(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + fixtures := map[string]string{ + "SKILL.md": "# Skill", + "README.md": "not selected", + "resources/config.yaml": "enabled: true", + "resources/private/secret.yaml": "secret: true", + ".claude/memory.md": "important memory", + ".qoder/repowiki/knowledge/zh/_module.yaml": "generated: true", + ".qoder/repowiki/knowledge/zh/notes.tmp": "generated", + "resources/nested/generated/temporary-file.tmp": "generated", + } + for name, content := range fixtures { + path := filepath.Join(dir, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + + include := []string{"SKILL.md", "resources/**", ".claude/**", ".qoder/**"} + exclude := []string{".qoder/repowiki/**", "resources/private/**", "**/*.tmp"} + files, err := ListSkillFiles(dir, include, exclude) + if err != nil { + t.Fatalf("ListSkillFiles failed: %v", err) + } + + for _, want := range []string{"SKILL.md", "resources/config.yaml", ".claude/memory.md"} { + if !slices.Contains(files, filepath.FromSlash(want)) { + t.Errorf("expected %q to be included, got %v", want, files) + } + } + for _, unwanted := range []string{ + "README.md", + "resources/private/secret.yaml", + ".qoder/repowiki/knowledge/zh/_module.yaml", + ".qoder/repowiki/knowledge/zh/notes.tmp", + "resources/nested/generated/temporary-file.tmp", + } { + if slices.Contains(files, filepath.FromSlash(unwanted)) { + t.Errorf("expected %q to be excluded, got %v", unwanted, files) + } + } +} + +func TestListSkillFiles_InvalidPattern(t *testing.T) { + t.Parallel() + + _, err := ListSkillFiles(t.TempDir(), []string{"["}, nil) + if err == nil || !strings.Contains(err.Error(), "invalid skill file pattern") { + t.Fatalf("ListSkillFiles error = %v, want invalid pattern", err) + } +} + func TestInstallSkill_PreservesExecutableScripts(t *testing.T) { t.Parallel() if goruntime.GOOS == "windows" { @@ -110,7 +197,7 @@ func TestInstallSkill_PreservesExecutableScripts(t *testing.T) { } defer func() { _ = rt.Close() }() - if err := installSkill(context.Background(), rt, src, "skill"); err != nil { + if err := installSkill(context.Background(), rt, src, "skill", nil, nil); err != nil { t.Fatalf("installSkill failed: %v", err) } @@ -123,11 +210,47 @@ func TestInstallSkill_PreservesExecutableScripts(t *testing.T) { } } +func TestInstallSkill_AppliesFilters(t *testing.T) { + t.Parallel() + + source := t.TempDir() + fixtures := map[string]string{ + "SKILL.md": "# Skill", + ".claude/memory.md": "required memory", + ".qoder/repowiki/knowledge/zh/_module.yaml": "generated metadata", + } + for name, content := range fixtures { + path := filepath.Join(source, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + + rt := &runtime.NoneRuntime{} + if err := rt.Create(context.Background()); err != nil { + t.Fatal(err) + } + defer func() { _ = rt.Close() }() + + if err := installSkill(context.Background(), rt, source, "skill", nil, []string{".qoder/repowiki/**"}); err != nil { + t.Fatalf("installSkill failed: %v", err) + } + if _, err := os.Stat(filepath.Join(rt.Workspace(), "skill", ".claude", "memory.md")); err != nil { + t.Fatalf("included memory file missing: %v", err) + } + if _, err := os.Stat(filepath.Join(rt.Workspace(), "skill", ".qoder", "repowiki", "knowledge", "zh", "_module.yaml")); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("excluded RepoWiki file stat error = %v, want not exist", err) + } +} + func TestListSkillFiles_EmptyDir(t *testing.T) { t.Parallel() dir := t.TempDir() - files, err := ListSkillFiles(dir) + files, err := ListSkillFiles(dir, nil, nil) if err != nil { t.Fatalf("ListSkillFiles failed: %v", err) } diff --git a/internal/agent/cli.go b/internal/agent/cli.go index e2b965c0..747c80e0 100644 --- a/internal/agent/cli.go +++ b/internal/agent/cli.go @@ -3,6 +3,7 @@ package agent import ( "bytes" "context" + "errors" "fmt" "path/filepath" "regexp" @@ -83,6 +84,9 @@ func (a *CLIAgent) InstallSkill(ctx context.Context, rt Runtime, skillCfg runtim if a.Cfg.InstallSkillCmd == "" { return a.installSkillDefault(ctx, rt, skillCfg) } + if len(skillCfg.Include) > 0 || len(skillCfg.Exclude) > 0 { + return errors.New("skill include/exclude filters are not supported with a custom InstallSkillCmd") + } tmpl, err := template.New("installSkill").Parse(a.Cfg.InstallSkillCmd) if err != nil { diff --git a/internal/agent/cli_test.go b/internal/agent/cli_test.go index beffd90e..bc37cf40 100644 --- a/internal/agent/cli_test.go +++ b/internal/agent/cli_test.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" goruntime "runtime" + "strings" "testing" "github.com/alibaba/skill-up/internal/platform" @@ -224,6 +225,22 @@ func TestCLIAgent_InstallSkillWithCmd(t *testing.T) { } } +func TestCLIAgent_InstallSkillWithCmdRejectsFilters(t *testing.T) { + t.Parallel() + + ag := &CLIAgent{BaseAgent: BaseAgent{Cfg: Config{ + Name: "test-agent", + InstallSkillCmd: "echo install", + }}} + err := ag.InstallSkill(context.Background(), nil, runtime.SkillConfig{ + Source: "my-skill", + Exclude: []string{".qoder/repowiki/**"}, + }) + if err == nil || !strings.Contains(err.Error(), "not supported with a custom InstallSkillCmd") { + t.Fatalf("InstallSkill error = %v, want unsupported filters", err) + } +} + func TestCLIAgent_InstallMCPEmpty(t *testing.T) { t.Parallel() diff --git a/internal/agent/skill.go b/internal/agent/skill.go index 042702b4..856da770 100644 --- a/internal/agent/skill.go +++ b/internal/agent/skill.go @@ -2,45 +2,134 @@ package agent import ( "context" + "fmt" "os" "path/filepath" "strings" + + "github.com/bmatcuk/doublestar/v4" ) // ListSkillFiles returns a list of files to sync for a skill, -// excluding the evals directory and its contents. -func ListSkillFiles(sourceDir string) ([]string, error) { - var files []string - if err := filepath.Walk(sourceDir, func(path string, info os.FileInfo, err error) error { +// applying include patterns followed by exclude patterns. The evals directory +// is always excluded because it belongs to the evaluation harness. +func ListSkillFiles(sourceDir string, include, exclude []string) ([]string, error) { + if err := validateSkillFilePatterns(include); err != nil { + return nil, err + } + if err := validateSkillFilePatterns(exclude); err != nil { + return nil, err + } + + selector := skillFileSelector{sourceDir: sourceDir, include: include, exclude: exclude} + if err := filepath.Walk(sourceDir, selector.visit); err != nil { + return nil, err + } + + return selector.files, nil +} + +type skillFileSelector struct { + sourceDir string + include []string + exclude []string + files []string +} + +func (s *skillFileSelector) visit(path string, info os.FileInfo, walkErr error) error { + if walkErr != nil { + return walkErr + } + rel, err := filepath.Rel(s.sourceDir, path) + if err != nil { + return err + } + relSlash := filepath.ToSlash(rel) + if relSlash == "evals" { + return filepath.SkipDir + } + + excluded, err := matchesSkillPatterns(s.exclude, relSlash) + if err != nil { + return err + } + if !excluded && info.IsDir() { + excluded, err = matchesSkillDirectory(s.exclude, relSlash) if err != nil { return err } - rel, err := filepath.Rel(sourceDir, path) - if err != nil { - return err + } + if excluded { + if info.IsDir() { + return filepath.SkipDir } - if rel == "evals" || strings.HasPrefix(rel, "evals/") { - if info.IsDir() { - return filepath.SkipDir - } + return nil + } + if info.IsDir() { + s.files = append(s.files, rel) + return nil + } + + included, err := s.isIncluded(relSlash) + if err != nil { + return err + } + if included { + s.files = append(s.files, rel) + } + return nil +} - return nil +func (s *skillFileSelector) isIncluded(rel string) (bool, error) { + if len(s.include) == 0 { + return true, nil + } + return matchesSkillPatterns(s.include, rel) +} + +func validateSkillFilePatterns(patterns []string) error { + for _, pattern := range patterns { + if !doublestar.ValidatePattern(pattern) { + return fmt.Errorf("invalid skill file pattern %q", pattern) } - files = append(files, rel) + } + return nil +} - return nil - }); err != nil { - return nil, err +func matchesSkillPatterns(patterns []string, rel string) (bool, error) { + for _, pattern := range patterns { + matched, err := doublestar.Match(pattern, rel) + if err != nil { + return false, fmt.Errorf("invalid skill file pattern %q: %w", pattern, err) + } + if matched { + return true, nil + } } + return false, nil +} - return files, nil +func matchesSkillDirectory(patterns []string, rel string) (bool, error) { + for _, pattern := range patterns { + if !strings.HasSuffix(pattern, "/**") { + continue + } + matched, err := doublestar.Match(strings.TrimSuffix(pattern, "/**"), rel) + if err != nil { + return false, fmt.Errorf("invalid skill file pattern %q: %w", pattern, err) + } + if matched { + return true, nil + } + } + return false, nil } // installSkill uploads a skill directory to the target path, -// excluding the evals directory and its contents. +// applying its configured include and exclude patterns. // target is relative to workspace, runtime handles path resolution. -func installSkill(ctx context.Context, rt Runtime, source, target string) error { // nolint: unparam // ctx required by interface - files, err := ListSkillFiles(source) +func installSkill(ctx context.Context, rt Runtime, source, target string, include, exclude []string) error { // nolint: unparam // ctx required by interface + files, err := ListSkillFiles(source, include, exclude) if err != nil { return err } diff --git a/internal/config/loader_test.go b/internal/config/loader_test.go index 0312ea72..8d1cb556 100644 --- a/internal/config/loader_test.go +++ b/internal/config/loader_test.go @@ -135,12 +135,37 @@ report: if len(cfg.Cases.Files) != 2 { t.Errorf("expected 2 case files, got %d", len(cfg.Cases.Files)) } - if cfg.Judge.Type != "script" { t.Errorf("expected judge.type 'script', got '%s'", cfg.Judge.Type) } } +func TestLoader_LoadEvalConfig_SkillFilters(t *testing.T) { + t.Parallel() + + evalPath := filepath.Join(t.TempDir(), "eval.yaml") + content := `schema_version: v1alpha1 +skills: + - source: local_path + path: . + include: [SKILL.md, "resources/**"] + exclude: ["resources/private/**"] +` + if err := os.WriteFile(evalPath, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + cfg, err := NewLoader(evalPath).LoadEvalConfig() + if err != nil { + t.Fatalf("LoadEvalConfig failed: %v", err) + } + if len(cfg.Skills) != 1 || !slices.Equal(cfg.Skills[0].Include, []string{"SKILL.md", "resources/**"}) { + t.Fatalf("skills[0].include = %v", cfg.Skills) + } + if !slices.Equal(cfg.Skills[0].Exclude, []string{"resources/private/**"}) { + t.Fatalf("skills[0].exclude = %v", cfg.Skills[0].Exclude) + } +} + func TestLoader_LoadEvalConfig_EngineKwargs(t *testing.T) { t.Parallel() @@ -291,6 +316,8 @@ judge: skills: - source: local_path path: evals/fixtures/default-judge + include: [SKILL.md, "references/**"] + exclude: ["references/drafts/**"] - source: local_path path: evals/fixtures/security-judge target: ~/.claude/skills/security-judge @@ -311,6 +338,12 @@ judge: if got := cfg.Judge.Skills[0].Path; got != "evals/fixtures/default-judge" { t.Fatalf("Judge.Skills[0].Path = %q", got) } + if !slices.Equal(cfg.Judge.Skills[0].Include, []string{"SKILL.md", "references/**"}) { + t.Fatalf("Judge.Skills[0].Include = %v", cfg.Judge.Skills[0].Include) + } + if !slices.Equal(cfg.Judge.Skills[0].Exclude, []string{"references/drafts/**"}) { + t.Fatalf("Judge.Skills[0].Exclude = %v", cfg.Judge.Skills[0].Exclude) + } if got := cfg.Judge.Skills[1].Target; got != "~/.claude/skills/security-judge" { t.Fatalf("Judge.Skills[1].Target = %q", got) } diff --git a/internal/config/schema.go b/internal/config/schema.go index 59f96901..5374347c 100644 --- a/internal/config/schema.go +++ b/internal/config/schema.go @@ -87,9 +87,11 @@ type MCPServer struct { // SkillRef describes a skill to install. type SkillRef struct { - Source string `yaml:"source"` // local_path, registry - Path string `yaml:"path,omitempty"` - Target string `yaml:"target,omitempty"` + Source string `yaml:"source"` // local_path, registry + Path string `yaml:"path,omitempty"` + Target string `yaml:"target,omitempty"` + Include []string `yaml:"include,omitempty"` + Exclude []string `yaml:"exclude,omitempty"` } // EngineConfig defines the Agent Engine configuration. diff --git a/internal/config/validator.go b/internal/config/validator.go index 2377afc9..0d06194b 100644 --- a/internal/config/validator.go +++ b/internal/config/validator.go @@ -4,6 +4,7 @@ import ( "fmt" "path" "regexp" + "slices" "strings" "github.com/bmatcuk/doublestar/v4" @@ -97,6 +98,7 @@ func (v *Validator) ValidateEvalConfig(cfg *EvalConfig) error { errs = append(errs, validateCollectArtifacts("cases.defaults.collect_artifacts", cfg.Cases.Defaults.CollectArtifacts)...) errs = append(errs, validateExpect("cases.defaults.expect", cfg.Cases.Defaults.Expect)...) + errs = append(errs, validateSkillRefs("skills", cfg.Skills)...) errs = append(errs, validateJudgeTypeAndLocalFields(cfg.Judge)...) if len(errs) > 0 { @@ -281,6 +283,33 @@ func validateSkillRefs(field string, refs []SkillRef) []string { if strings.TrimSpace(ref.Path) == "" { errs = append(errs, fmt.Sprintf("%s[%d].path is required", field, i)) } + errs = append(errs, validateSkillPatterns(fmt.Sprintf("%s[%d].include", field, i), ref.Include)...) + errs = append(errs, validateSkillPatterns(fmt.Sprintf("%s[%d].exclude", field, i), ref.Exclude)...) + } + return errs +} + +func validateSkillPatterns(field string, patterns []string) []string { + var errs []string + for i, pattern := range patterns { + if strings.TrimSpace(pattern) == "" { + errs = append(errs, fmt.Sprintf("%s[%d] must not be empty", field, i)) + continue + } + if path.IsAbs(pattern) { + errs = append(errs, fmt.Sprintf("%s[%d] must be relative to the skill path", field, i)) + continue + } + if strings.Contains(pattern, `\`) { + errs = append(errs, fmt.Sprintf("%s[%d] must use '/' as the path separator", field, i)) + continue + } + if slices.Contains(strings.Split(pattern, "/"), "..") { + errs = append(errs, fmt.Sprintf("%s[%d] must not contain a parent-directory segment", field, i)) + } + if !doublestar.ValidatePattern(pattern) { + errs = append(errs, fmt.Sprintf("%s[%d] is not a valid glob pattern: %q", field, i, pattern)) + } } return errs } diff --git a/internal/config/validator_test.go b/internal/config/validator_test.go index 5a067c27..df2615a4 100644 --- a/internal/config/validator_test.go +++ b/internal/config/validator_test.go @@ -1211,6 +1211,78 @@ func TestValidator_JudgeSkills(t *testing.T) { } } +func TestValidator_JudgeSkillPatterns(t *testing.T) { + t.Parallel() + + cfg := &EvalConfig{ + SchemaVersion: "v1alpha1", + Environment: Environment{Type: "none"}, + Engine: EngineConfig{Name: "claude_code"}, + Cases: CasesConfig{Files: []string{"evals/cases/test.yaml"}}, + Judge: JudgeConfig{ + Type: "agent_judge", + Model: "test-model", + Criteria: []string{"criterion"}, + Skills: []SkillRef{{ + Source: "local_path", + Path: "evals/fixtures/judge-skill", + Exclude: []string{"["}, + }}, + }, + } + err := NewValidator().ValidateEvalConfig(cfg) + want := "judge.skills[0].exclude[0] is not a valid glob pattern" + if err == nil || !strings.Contains(err.Error(), want) { + t.Fatalf("ValidateEvalConfig() error = %v, want containing %q", err, want) + } +} + +func TestValidator_SkillPatterns(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + include []string + exclude []string + errMsg string + }{ + {name: "valid", include: []string{"SKILL.md", "references/**"}, exclude: []string{"references/drafts/**"}}, + {name: "empty", include: []string{""}, errMsg: "skills[0].include[0] must not be empty"}, + {name: "invalid glob", exclude: []string{"["}, errMsg: "skills[0].exclude[0] is not a valid glob pattern"}, + {name: "absolute", exclude: []string{"/tmp/**"}, errMsg: "skills[0].exclude[0] must be relative to the skill path"}, + {name: "backslash", exclude: []string{`.qoder\repowiki\**`}, errMsg: "skills[0].exclude[0] must use '/' as the path separator"}, + {name: "parent segment", include: []string{"../SKILL.md"}, errMsg: "skills[0].include[0] must not contain a parent-directory segment"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cfg := &EvalConfig{ + SchemaVersion: "v1alpha1", + Environment: Environment{Type: "none"}, + Engine: EngineConfig{Name: "claude_code"}, + Cases: CasesConfig{Files: []string{"evals/cases/test.yaml"}}, + Skills: []SkillRef{{ + Source: "local_path", + Path: ".", + Include: tt.include, + Exclude: tt.exclude, + }}, + } + err := NewValidator().ValidateEvalConfig(cfg) + if tt.errMsg == "" { + if err != nil { + t.Fatalf("ValidateEvalConfig() error = %v, want nil", err) + } + return + } + if err == nil || !strings.Contains(err.Error(), tt.errMsg) { + t.Fatalf("ValidateEvalConfig() error = %v, want containing %q", err, tt.errMsg) + } + }) + } +} + // caseIDSrc pairs an effective case ID with the cases.files entry it was // loaded from, as the loader populates CaseConfig.ID / CaseConfig.SourceFile. type caseIDSrc struct { diff --git a/internal/evaluator/evaluator.go b/internal/evaluator/evaluator.go index 90499c71..8cd0f62c 100644 --- a/internal/evaluator/evaluator.go +++ b/internal/evaluator/evaluator.go @@ -12,6 +12,7 @@ import ( "maps" "os" "path/filepath" + "slices" "strings" "sync" "time" @@ -1142,8 +1143,10 @@ func resolveSkillConfig(skillDir string, ref config.SkillRef) runtime.SkillConfi source = filepath.Join(skillDir, source) } return runtime.SkillConfig{ - Source: source, - Target: ref.Target, + Source: source, + Target: ref.Target, + Include: slices.Clone(ref.Include), + Exclude: slices.Clone(ref.Exclude), } } diff --git a/internal/evaluator/evaluator_test.go b/internal/evaluator/evaluator_test.go index b6207cfd..63fe7c1f 100644 --- a/internal/evaluator/evaluator_test.go +++ b/internal/evaluator/evaluator_test.go @@ -9,6 +9,7 @@ import ( "os/exec" "path/filepath" goruntime "runtime" + "slices" "strings" "sync" "sync/atomic" @@ -541,7 +542,12 @@ func TestSetupCaseEnvironmentRunsSetupAndInstallsAgentMCPAndSkill(t *testing.T) Type: "opensandbox", SetupSteps: []config.SetupStep{{Run: "printf setup > marker.txt"}}, }, - Skills: []config.SkillRef{{Path: ".", Target: "custom-target"}}, + Skills: []config.SkillRef{{ + Path: ".", + Target: "custom-target", + Include: []string{"SKILL.md", "references/**"}, + Exclude: []string{"references/drafts/**"}, + }}, }, }) @@ -561,6 +567,10 @@ func TestSetupCaseEnvironmentRunsSetupAndInstallsAgentMCPAndSkill(t *testing.T) if lastSkill.Source != skillDir || lastSkill.Target != "custom-target" { t.Fatalf("last skill config = %+v, want source skill dir and custom target", lastSkill) } + if !slices.Equal(lastSkill.Include, []string{"SKILL.md", "references/**"}) || + !slices.Equal(lastSkill.Exclude, []string{"references/drafts/**"}) { + t.Fatalf("last skill filters = include %v exclude %v", lastSkill.Include, lastSkill.Exclude) + } withoutSkillAgent := &mockAgent{name: "agent"} if err := e.setupCaseEnvironment(context.Background(), rt, &config.CaseConfig{ID: "case-a"}, "without_skill", withoutSkillAgent, runtime.MCPConfig{}); err != nil { @@ -1152,7 +1162,13 @@ func TestExecuteCase_InstallsJudgeSkillsOnJudgeAgentOnly(t *testing.T) { Model: "judge-model", Criteria: []string{"uses rubric"}, Skills: []config.SkillRef{ - {Source: "local_path", Path: "evals/fixtures/judge-skill", Target: "~/.claude/skills/judge-skill"}, + { + Source: "local_path", + Path: "evals/fixtures/judge-skill", + Target: "~/.claude/skills/judge-skill", + Include: []string{"SKILL.md", "references/**"}, + Exclude: []string{"references/drafts/**"}, + }, {Source: "local_path", Path: "evals/fixtures/security-judge"}, }, }, @@ -1186,6 +1202,10 @@ func TestExecuteCase_InstallsJudgeSkillsOnJudgeAgentOnly(t *testing.T) { if judgeAgent.skills[0].Target != "~/.claude/skills/judge-skill" { t.Fatalf("first judge skill target = %q", judgeAgent.skills[0].Target) } + if !slices.Equal(judgeAgent.skills[0].Include, []string{"SKILL.md", "references/**"}) || + !slices.Equal(judgeAgent.skills[0].Exclude, []string{"references/drafts/**"}) { + t.Fatalf("first judge skill filters = include %v exclude %v", judgeAgent.skills[0].Include, judgeAgent.skills[0].Exclude) + } if len(result.JudgeSkills) != 2 || result.JudgeSkills[0].Path != "evals/fixtures/judge-skill" { t.Fatalf("result JudgeSkills = %#v", result.JudgeSkills) } diff --git a/internal/judge/judge_skill.go b/internal/judge/judge_skill.go index 3ce760c6..df1c5c9c 100644 --- a/internal/judge/judge_skill.go +++ b/internal/judge/judge_skill.go @@ -2,16 +2,19 @@ package judge import ( "path/filepath" + "slices" "github.com/alibaba/skill-up/internal/config" ) // SkillInfo describes a judge Skill configured for agent_judge. type SkillInfo struct { - Source string `json:"source,omitempty"` - Path string `json:"path,omitempty"` - Target string `json:"target,omitempty"` - Name string `json:"name,omitempty"` + Source string `json:"source,omitempty"` + Path string `json:"path,omitempty"` + Target string `json:"target,omitempty"` + Include []string `json:"include,omitempty"` + Exclude []string `json:"exclude,omitempty"` + Name string `json:"name,omitempty"` } // SkillInfosFromRefs converts configured Skill refs into report-safe metadata. @@ -23,10 +26,12 @@ func SkillInfosFromRefs(refs []config.SkillRef) []SkillInfo { for _, ref := range refs { name := skillInfoName(ref.Path) infos = append(infos, SkillInfo{ - Source: ref.Source, - Path: ref.Path, - Target: ref.Target, - Name: name, + Source: ref.Source, + Path: ref.Path, + Target: ref.Target, + Include: slices.Clone(ref.Include), + Exclude: slices.Clone(ref.Exclude), + Name: name, }) } return infos diff --git a/internal/judge/judge_skill_test.go b/internal/judge/judge_skill_test.go index cf151eb3..e2f2819d 100644 --- a/internal/judge/judge_skill_test.go +++ b/internal/judge/judge_skill_test.go @@ -1,6 +1,7 @@ package judge import ( + "slices" "testing" "github.com/alibaba/skill-up/internal/config" @@ -12,7 +13,12 @@ func TestSkillInfosFromRefs_OmitsDotName(t *testing.T) { infos := SkillInfosFromRefs([]config.SkillRef{ {Source: "local_path"}, {Source: "local_path", Path: "."}, - {Source: "local_path", Path: "evals/fixtures/judge-skill"}, + { + Source: "local_path", + Path: "evals/fixtures/judge-skill", + Include: []string{"SKILL.md", "references/**"}, + Exclude: []string{"references/drafts/**"}, + }, }) if len(infos) != 3 { @@ -24,4 +30,8 @@ func TestSkillInfosFromRefs_OmitsDotName(t *testing.T) { if infos[2].Name != "judge-skill" { t.Fatalf("third Name = %q, want judge-skill", infos[2].Name) } + if !slices.Equal(infos[2].Include, []string{"SKILL.md", "references/**"}) || + !slices.Equal(infos[2].Exclude, []string{"references/drafts/**"}) { + t.Fatalf("third filters = include %v exclude %v", infos[2].Include, infos[2].Exclude) + } } diff --git a/internal/report/junit.go b/internal/report/junit.go index f2a5be3c..e42609c5 100644 --- a/internal/report/junit.go +++ b/internal/report/junit.go @@ -186,6 +186,8 @@ func buildJudgeSkillProperties(cr CaseResult) *junitProperties { junitProperty{Name: prefix + "source", Value: skill.Source}, junitProperty{Name: prefix + "path", Value: skill.Path}, junitProperty{Name: prefix + "target", Value: skill.Target}, + junitProperty{Name: prefix + "include", Value: strings.Join(skill.Include, ",")}, + junitProperty{Name: prefix + "exclude", Value: strings.Join(skill.Exclude, ",")}, junitProperty{Name: prefix + "name", Value: skill.Name}, ) } diff --git a/internal/report/reporter_test.go b/internal/report/reporter_test.go index 3e7cd731..32ef125b 100644 --- a/internal/report/reporter_test.go +++ b/internal/report/reporter_test.go @@ -33,7 +33,14 @@ func sampleInput() Input { DurationMs: 45200, Turns: 5, JudgeSkills: []judge.SkillInfo{ - {Source: "local_path", Path: "evals/fixtures/judge-skill", Target: "~/.claude/skills/judge-skill", Name: "judge-skill"}, + { + Source: "local_path", + Path: "evals/fixtures/judge-skill", + Target: "~/.claude/skills/judge-skill", + Include: []string{"SKILL.md", "references/**"}, + Exclude: []string{"references/drafts/**"}, + Name: "judge-skill", + }, }, Grading: &judge.Result{ Status: judge.StatusPass, @@ -122,6 +129,11 @@ func TestJSONReporter_Write(t *testing.T) { if len(parsed.CaseResults[0].JudgeSkills) != 1 || parsed.CaseResults[0].JudgeSkills[0].Path != "evals/fixtures/judge-skill" { t.Fatalf("judge_skills not preserved in JSON: %#v", parsed.CaseResults[0].JudgeSkills) } + parsedSkill := parsed.CaseResults[0].JudgeSkills[0] + if len(parsedSkill.Include) != 2 || parsedSkill.Include[1] != "references/**" || + len(parsedSkill.Exclude) != 1 || parsedSkill.Exclude[0] != "references/drafts/**" { + t.Fatalf("judge skill filters not preserved in JSON: %#v", parsedSkill) + } } func TestJSONReporter_ContainsAssertions(t *testing.T) { @@ -187,6 +199,12 @@ func TestJUnitReporter_Write(t *testing.T) { if !strings.Contains(content, `name="judge.skills.0.path" value="evals/fixtures/judge-skill"`) { t.Fatal("junit should include judge skill path property") } + if !strings.Contains(content, `name="judge.skills.0.include" value="SKILL.md,references/**"`) { + t.Fatal("junit should include judge skill include property") + } + if !strings.Contains(content, `name="judge.skills.0.exclude" value="references/drafts/**"`) { + t.Fatal("junit should include judge skill exclude property") + } } func TestJUnitReporter_FailureDetails(t *testing.T) { diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index c5681a51..da688b5c 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -208,8 +208,10 @@ type MCPConfig struct { // SkillConfig identifies a skill source and its target install location. type SkillConfig struct { - Source string - Target string + Source string + Target string + Include []string + Exclude []string } // NewRuntime creates a Runtime based on the config type. diff --git a/skills/skill-upper/references/eval-yaml.md b/skills/skill-upper/references/eval-yaml.md index d02594ff..19946686 100644 --- a/skills/skill-upper/references/eval-yaml.md +++ b/skills/skill-upper/references/eval-yaml.md @@ -20,6 +20,8 @@ mcp: skills: - source: local_path path: . + include: [SKILL.md, "references/**", "scripts/**"] # 可选；不配置表示全部文件 + exclude: [".qoder/repowiki/**"] # 可选；exclude 优先 engine: name: claude_code # claude_code | codex | qodercli（也兼容 qoder-cli） @@ -69,6 +71,11 @@ report: `collect_artifacts`（`cases.defaults` 级，或单个 `case.yaml` 内追加）用 [doublestar](https://github.com/bmatcuk/doublestar) glob（`*` 单层、`**` 跨目录）声明要采集的 workspace 文件。无论 Agent 成功/失败/超时，命中文件都会保留相对路径下载到 `///outputs/workspace/`。两层按并集去重合并。它与 `report.artifacts`（产物*类型*）、`agent_judge` 的 git diff（字符串）正交。 +`skills[].include` / `skills[].exclude` 同样使用 doublestar glob，路径相对 +`skills[].path` 且使用 `/` 分隔。`include` 为空时默认包含全部文件； +`exclude` 后应用并优先。`evals/` 始终不会安装。显式配置 include 时要包含 +`SKILL.md`。`judge.skills` 也支持同样的过滤字段。 + `judge.skills` 仅支持 `judge.type: agent_judge`，用于给 judge agent 安装可复用的评分 Rubric Skill。它不会安装到主运行 agent；顶层 `skills` 也不会自动安装到 judge。benchmark 下 `with_skill` / `without_skill` 都会安装 judge Skills，因为它们属于评分工具。路径相对 Skill 根目录解析，安装依赖具体 Agent adapter 的原生 Skill 支持；不要把 Skill 文件内容复制进 `criteria`。 ## 运行环境 diff --git a/skills/skill-upper/references/judge-types.md b/skills/skill-upper/references/judge-types.md index 35d4a820..8dfe9685 100644 --- a/skills/skill-upper/references/judge-types.md +++ b/skills/skill-upper/references/judge-types.md @@ -48,6 +48,8 @@ judge: skills: - source: local_path path: evals/fixtures/judge-rubric + include: [SKILL.md, "references/**"] + exclude: ["references/drafts/**"] criteria: - "输出中识别了真实存在的 bug，并符合 judge-rubric 中的评分细则" - "没有将正确代码误报为 bug" @@ -62,6 +64,7 @@ judge: - 能拆出确定性条件时先用 `rule_based` / `expect` 挡一道 - 需要长 Rubric、领域规则、复用评分规范时，把它们放进 `judge.skills` - `judge.skills` 只会安装给 judge agent，不会污染被测 run agent；安装依赖具体 Agent adapter 的 Skill 支持，不会回退为 prompt 拼接 +- `judge.skills[].include` / `exclude` 与顶层 `skills` 语义一致：相对 `path` 的 doublestar glob，且 exclude 优先 ## script — 自定义脚本