diff --git a/cmd/opendbx/main_test.go b/cmd/opendbx/main_test.go index 0395050..a01403e 100644 --- a/cmd/opendbx/main_test.go +++ b/cmd/opendbx/main_test.go @@ -145,6 +145,22 @@ func TestGolden(t *testing.T) { } } +// TestPluginListRunsDiscovery — `plugin list` runs a real (spec-2.2) skill +// discovery and renders the summary, not a stub. The "active (" header is +// always present regardless of how many skills exist in the environment. +func TestPluginListRunsDiscovery(t *testing.T) { + stdout, _, err := runCmd(t, "plugin", "list") + if err != nil { + t.Fatalf("plugin list: %v", err) + } + if !strings.Contains(stdout, "active (") { + t.Errorf("plugin list should render a discovery summary, got: %q", stdout) + } + if strings.Contains(stdout, "not yet implemented") { + t.Errorf("plugin list is implemented now; got stub: %q", stdout) + } +} + func TestSubcommandStubs(t *testing.T) { subs := []struct { args []string @@ -154,7 +170,6 @@ func TestSubcommandStubs(t *testing.T) { {[]string{"cluster"}, "spec-9.X"}, {[]string{"admin", "migrate"}, "spec-4.8-version-migrations"}, {[]string{"mcp", "list"}, "spec-2.5"}, - {[]string{"plugin", "list"}, "spec-2.1-skill-md-format"}, {[]string{"auth", "status"}, "Stage 2+"}, {[]string{"doctor"}, "Stage 4+"}, {[]string{"update"}, "spec-4.7-install"}, diff --git a/cmd/opendbx/plugin.go b/cmd/opendbx/plugin.go index b69833f..15fb0d9 100644 --- a/cmd/opendbx/plugin.go +++ b/cmd/opendbx/plugin.go @@ -2,8 +2,9 @@ // // Author: sqlrush -// Plugin (alias plugins) subcommand skeleton. Real implementation in -// spec-2.1-skill-md-format. +// Plugin (alias plugins) subcommand. `plugin list` runs a one-shot skill +// discovery (spec-2.2); add/remove remain stubs until the full plugin tree +// (spec-2.18). package main @@ -11,6 +12,8 @@ import ( "fmt" "github.com/spf13/cobra" + + "github.com/sqlrush/opendbx/internal/entrypoints" ) func newPluginCommand(_ *Options) *cobra.Command { @@ -24,15 +27,27 @@ func newPluginCommand(_ *Options) *cobra.Command { Use: use, Short: short, RunE: func(cmd *cobra.Command, _ []string) error { - _, err := fmt.Fprintf(cmd.OutOrStdout(), "plugin %s not yet implemented in spec-2.1-skill-md-format.\n", use) + _, err := fmt.Fprintf(cmd.OutOrStdout(), "plugin %s not yet implemented (spec-2.18).\n", use) return err }, } } + list := &cobra.Command{ + Use: "list", + Short: "List discovered skills (active / shadowed / conflicts / errors)", + RunE: func(cmd *cobra.Command, _ []string) error { + summary, err := entrypoints.SkillsSummary() + if err != nil { + return err + } + _, err = fmt.Fprint(cmd.OutOrStdout(), summary) + return err + }, + } plugin.AddCommand( stub("add ", "Add a plugin"), stub("remove ", "Remove a plugin"), - stub("list", "List installed plugins"), + list, ) return plugin } diff --git a/cmd/opendbx/testdata/golden/plugin-help.txt b/cmd/opendbx/testdata/golden/plugin-help.txt index 98f848b..13c9c47 100644 --- a/cmd/opendbx/testdata/golden/plugin-help.txt +++ b/cmd/opendbx/testdata/golden/plugin-help.txt @@ -8,7 +8,7 @@ Aliases: Available Commands: add Add a plugin - list List installed plugins + list List discovered skills (active / shadowed / conflicts / errors) remove Remove a plugin Flags: diff --git a/git-hooks/spec-registry.txt b/git-hooks/spec-registry.txt index 542ae88..066b879 100644 --- a/git-hooks/spec-registry.txt +++ b/git-hooks/spec-registry.txt @@ -56,3 +56,4 @@ 1.25 1 49 main tag 1.21.1 1 50 sub tag 2.1 2 51 main tag +2.2 2 52 main tag diff --git a/internal/app/skills/discovery.go b/internal/app/skills/discovery.go new file mode 100644 index 0000000..0853f9c --- /dev/null +++ b/internal/app/skills/discovery.go @@ -0,0 +1,130 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +// File discovery.go — Discover orchestrates the scan→read→parse→validate→ +// resolve pipeline (spec-2.2 D-3). I/O lives here; all format/namespace logic +// is spec-2.1's pure functions. +// +// Order is load-bearing: +// - size cap is checked from Lstat BEFORE os.ReadFile (no unbounded read). +// - Validate warnings are collected even when Validate also returns a fatal +// error (warnings are data; spec-2.1 Q4). +// - disabled skills go to Ignored (visible), not silently dropped. +// - Validate runs before Resolve (spec-2.1 Resolve precondition). + +package skills + +import ( + "io" + "os" +) + +// Discover scans every root, isolates per-file failures into Errors, and +// resolves the surviving skills by precedence. It never aborts on a bad file +// or an absent optional root. +func Discover(opts DiscoverOptions) DiscoveryResult { + maxFiles := opts.MaxFilesPerRoot + if maxFiles == 0 { + maxFiles = defaultMaxFilesPerRoot + } + disabled := toSet(opts.Disabled) + + var valid []Skill + var res DiscoveryResult + + for _, root := range opts.Roots { + files, rerr := scanRoot(root, maxFiles) + if rerr != nil { + // ROOT_TOO_MANY_FILES still returns a truncated file list to process. + res.Errors = append(res.Errors, DiscoveryError{Path: root.Dir, Err: asErrcode(rerr)}) + } + for _, path := range files { + sk, ferr := loadSkill(path, root) + if ferr != nil { + res.Errors = append(res.Errors, DiscoveryError{Path: path, Err: asErrcode(ferr)}) + continue + } + rep, verr := Validate(sk) + for _, w := range rep.Warnings { + res.Warnings = append(res.Warnings, DiscoveryWarning{Path: path, Name: sk.Schema.Name, Warning: w}) + } + if verr != nil { + res.Errors = append(res.Errors, DiscoveryError{Path: path, Err: asErrcode(verr)}) + continue + } + if disabled[sk.Key()] { + res.Ignored = append(res.Ignored, IgnoredSkill{Skill: sk, Reason: "disabled"}) + continue + } + valid = append(valid, sk) + } + } + + r := Resolve(valid) // validate-before-resolve precondition satisfied + res.Active = r.Active + res.Shadowed = r.Shadowed + res.Conflicts = r.Conflicts // NOT duplicated into Errors; provenance lives here + return res +} + +// loadSkill safely reads and parses one skill file (review HIGH: fs safety). +// Defense in depth against symlink/device targets and unbounded reads: +// - Lstat + IsRegular rejects a symlink/FIFO/socket/device at the path before +// it is opened (covers DT_UNKNOWN filesystems where the scan's d_type check +// is unreliable). +// - After Open, f.Stat + IsRegular re-checks (guards a TOCTOU swap-to-device). +// - io.LimitReader bounds the read to maxSkillSize+1 so even a TOCTOU swap to +// a huge regular file cannot OOM — it surfaces as SKILL.TOO_LARGE. +// +// Residual: a TOCTOU swap to a <1 MiB regular file outside the root could be +// read; closing that needs O_NOFOLLOW (non-portable) and is deferred. The scan +// is a one-shot over the user's own dirs, so the race window is negligible. +func loadSkill(path string, root SkillRoot) (Skill, error) { + info, lerr := os.Lstat(path) + if lerr != nil { + return Skill{}, fileUnreadable(lerr) + } + if !info.Mode().IsRegular() { + return Skill{}, notRegularErr(path) + } + f, oerr := os.Open(path) //nolint:gosec // spec-2.2 D-3: path is a scanned skill file; Lstat-IsRegular above + f.Stat below + LimitReader bound the read. + if oerr != nil { + return Skill{}, fileUnreadable(oerr) + } + defer func() { _ = f.Close() }() + + fi, serr := f.Stat() + if serr != nil { + return Skill{}, fileUnreadable(serr) + } + if !fi.Mode().IsRegular() { + return Skill{}, notRegularErr(path) + } + if fi.Size() > maxSkillSize { + return Skill{}, tooLargeErr(fi.Size()) + } + + content, rerr := io.ReadAll(io.LimitReader(f, maxSkillSize+1)) + if rerr != nil { + return Skill{}, fileUnreadable(rerr) + } + if int64(len(content)) > maxSkillSize { + return Skill{}, tooLargeErr(int64(len(content))) + } + + src := SkillSource{Kind: root.Kind, Precedence: root.Precedence, Path: path, PluginID: root.PluginID} + return Parse(content, src) +} + +// toSet builds a lookup set from a slice (nil-safe). +func toSet(items []string) map[string]bool { + if len(items) == 0 { + return nil + } + m := make(map[string]bool, len(items)) + for _, it := range items { + m[it] = true + } + return m +} diff --git a/internal/app/skills/discovery_test.go b/internal/app/skills/discovery_test.go new file mode 100644 index 0000000..2da24be --- /dev/null +++ b/internal/app/skills/discovery_test.go @@ -0,0 +1,239 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +package skills + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// twoRoots returns a low- and high-precedence root over two temp dirs. +func twoRoots(t *testing.T) (lowDir, highDir string, opts DiscoverOptions) { + t.Helper() + lowDir, highDir = t.TempDir(), t.TempDir() + opts = DiscoverOptions{Roots: []SkillRoot{ + {Kind: SourceUserGlobal, Precedence: 1, Dir: lowDir}, + {Kind: SourceProject, Precedence: 2, Dir: highDir}, + }} + return +} + +func TestDiscover_Empty(t *testing.T) { + t.Parallel() + res := Discover(DiscoverOptions{}) + if len(res.Active) != 0 || len(res.Errors) != 0 { + t.Errorf("empty options should yield empty result: %+v", res) + } +} + +func TestDiscover_ShadowByPrecedence(t *testing.T) { + t.Parallel() + low, high, opts := twoRoots(t) + writeSkill(t, filepath.Join(low, "top.md"), "top-sql") + writeSkill(t, filepath.Join(high, "top.md"), "top-sql") + res := Discover(opts) + if len(res.Active) != 1 || res.Active[0].Source.Precedence != 2 { + t.Fatalf("higher precedence should win: %+v", res.Active) + } + if len(res.Shadowed) != 1 || res.Shadowed[0].Source.Precedence != 1 { + t.Fatalf("lower precedence shadowed: %+v", res.Shadowed) + } + // Conflicts carry provenance; they are NOT duplicated into Errors. + if len(res.Errors) != 0 { + t.Errorf("shadow is not an error: %+v", res.Errors) + } +} + +// TestDiscover_BadFileIsolation — each bad-file class is isolated to Errors +// while a good skill in the same scan still becomes Active. +func TestDiscover_BadFileIsolation(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeSkill(t, filepath.Join(dir, "good.md"), "good") + // bad YAML + _ = os.WriteFile(filepath.Join(dir, "badyaml.md"), []byte("---\nname: : :\n---\n"), 0o644) + // missing required field (no description) → validate failure + _ = os.WriteFile(filepath.Join(dir, "noval.md"), []byte("---\nname: noval\n---\nbody\n"), 0o644) + // no frontmatter + _ = os.WriteFile(filepath.Join(dir, "nofm.md"), []byte("# just markdown\n"), 0o644) + + res := Discover(DiscoverOptions{Roots: []SkillRoot{root(dir)}}) + + if len(res.Active) != 1 || res.Active[0].Schema.Name != "good" { + t.Fatalf("good skill should survive isolation: %+v", res.Active) + } + codes := map[string]bool{} + for _, de := range res.Errors { + codes[ClassifyError(de)] = true + } + for _, want := range []string{ErrParseError.Code(), ErrValidationFailed.Code(), ErrNoFrontmatter.Code()} { + if !codes[want] { + t.Errorf("missing isolated error %s; got %v", want, codes) + } + } +} + +// TestDiscover_SizeCapBeforeRead — an over-limit file is rejected via stat, +// never fully read (review HIGH: size cap before ReadFile). +func TestDiscover_SizeCapBeforeRead(t *testing.T) { + t.Parallel() + dir := t.TempDir() + big := filepath.Join(dir, "big.md") + if err := os.WriteFile(big, make([]byte, maxSkillSize+1), 0o644); err != nil { + t.Fatal(err) + } + writeSkill(t, filepath.Join(dir, "ok.md"), "ok") + res := Discover(DiscoverOptions{Roots: []SkillRoot{root(dir)}}) + if len(res.Active) != 1 { + t.Fatalf("ok skill should survive: %+v", res.Active) + } + found := false + for _, de := range res.Errors { + if ClassifyError(de) == ErrTooLarge.Code() { + found = true + } + } + if !found { + t.Errorf("over-limit file should produce TOO_LARGE: %+v", res.Errors) + } +} + +// TestDiscover_WarningsCollected — non-fatal warnings (non-kebab name, unknown +// field) surface in DiscoveryResult.Warnings, not dropped. +func TestDiscover_WarningsCollected(t *testing.T) { + t.Parallel() + dir := t.TempDir() + content := "---\nname: Not_Kebab\ndescription: d\nopendbx_extra: hi\n---\nbody\n" + _ = os.WriteFile(filepath.Join(dir, "w.md"), []byte(content), 0o644) + res := Discover(DiscoverOptions{Roots: []SkillRoot{root(dir)}}) + if len(res.Active) != 1 { + t.Fatalf("valid-but-warned skill should be active: %+v", res.Active) + } + if len(res.Warnings) < 2 { + t.Errorf("expected non-kebab + unknown-field warnings, got %+v", res.Warnings) + } +} + +func TestDiscover_DisabledToIgnored(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeSkill(t, filepath.Join(dir, "a.md"), "keep") + writeSkill(t, filepath.Join(dir, "b.md"), "drop") + res := Discover(DiscoverOptions{Roots: []SkillRoot{root(dir)}, Disabled: []string{"drop"}}) + if len(res.Active) != 1 || res.Active[0].Schema.Name != "keep" { + t.Fatalf("disabled skill must not be active: %+v", res.Active) + } + if len(res.Ignored) != 1 || res.Ignored[0].Skill.Schema.Name != "drop" { + t.Errorf("disabled skill should be visible in Ignored: %+v", res.Ignored) + } +} + +// TestDiscover_MembershipContract — every valid name is Active XOR +// unresolvable; no name is both. +func TestDiscover_MembershipContract(t *testing.T) { + t.Parallel() + dir := t.TempDir() + // two files, same name, SAME root → Unresolvable + _ = os.WriteFile(filepath.Join(dir, "x1.md"), []byte("---\nname: dup\ndescription: d\n---\nb\n"), 0o644) + _ = os.WriteFile(filepath.Join(dir, "x2.md"), []byte("---\nname: dup\ndescription: d\n---\nb\n"), 0o644) + writeSkill(t, filepath.Join(dir, "solo.md"), "solo") + res := Discover(DiscoverOptions{Roots: []SkillRoot{root(dir)}}) + + active := map[string]bool{} + for _, s := range res.Active { + active[s.Schema.Name] = true + } + unresolvable := map[string]bool{} + for _, c := range res.Conflicts { + if c.Kind == ConflictUnresolvable { + unresolvable[c.Name] = true + } + } + if !active["solo"] || active["dup"] { + t.Errorf("solo active, dup not active: active=%v", active) + } + if !unresolvable["dup"] { + t.Errorf("dup should be unresolvable: %v", unresolvable) + } + if active["dup"] && unresolvable["dup"] { + t.Error("dup is both active and unresolvable (contract violation)") + } +} + +// TestDiscover_ExtraRoundTrip — discovery preserves Schema.Extra byte-faithful +// through Parse + Resolve (2.1 deep-copy isolation). +func TestDiscover_ExtraRoundTrip(t *testing.T) { + t.Parallel() + dir := t.TempDir() + content := "---\nname: e\ndescription: d\nopendbx_databases:\n - postgres\n - mysql\n---\nbody\n" + _ = os.WriteFile(filepath.Join(dir, "e.md"), []byte(content), 0o644) + res := Discover(DiscoverOptions{Roots: []SkillRoot{root(dir)}}) + if len(res.Active) != 1 { + t.Fatalf("want 1 active: %+v", res.Active) + } + dbs, ok := res.Active[0].Schema.Extra["opendbx_databases"].([]any) + if !ok || len(dbs) != 2 { + t.Errorf("Extra round-trip broken: %+v", res.Active[0].Schema.Extra) + } +} + +// TestDiscover_WarningsAndFatalSameFile — a file that both warns (non-kebab + +// unknown field) AND fatally fails validation (missing description) contributes +// to BOTH Warnings and Errors, and is not Active (review RC-4 / codex). +func TestDiscover_WarningsAndFatalSameFile(t *testing.T) { + t.Parallel() + dir := t.TempDir() + content := "---\nname: Not_Kebab\nopendbx_extra: x\n---\nbody\n" // no description → fatal + _ = os.WriteFile(filepath.Join(dir, "both.md"), []byte(content), 0o644) + res := Discover(DiscoverOptions{Roots: []SkillRoot{root(dir)}}) + if len(res.Warnings) < 2 { + t.Errorf("warnings must survive a fatal validate error, got %+v", res.Warnings) + } + if len(res.Errors) != 1 || ClassifyError(res.Errors[0]) != ErrValidationFailed.Code() { + t.Errorf("want 1 VALIDATION_FAILED, got %+v", res.Errors) + } + if len(res.Active) != 0 { + t.Errorf("fatally invalid skill must not be active: %+v", res.Active) + } +} + +// TestLoadSkill_RejectsSymlink — loadSkill refuses a symlinked file even if the +// scan's d_type missed it (DT_UNKNOWN backstop / TOCTOU). +func TestLoadSkill_RejectsSymlink(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("symlink semantics differ on windows") + } + target := filepath.Join(t.TempDir(), "target.md") + writeSkill(t, target, "tgt") + link := filepath.Join(t.TempDir(), "link.md") + if err := os.Symlink(target, link); err != nil { + t.Skipf("symlink unsupported: %v", err) + } + _, err := loadSkill(link, root(filepath.Dir(link))) + assertCode(t, err, ErrFileUnreadable.Code()) +} + +// TestLoadSkill_MissingFile — a file removed between scan and load is isolated +// as FILE_UNREADABLE (TOCTOU error path). +func TestLoadSkill_MissingFile(t *testing.T) { + t.Parallel() + _, err := loadSkill(filepath.Join(t.TempDir(), "gone.md"), root(t.TempDir())) + assertCode(t, err, ErrFileUnreadable.Code()) +} + +func TestSummarizeDiscovery_Renders(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeSkill(t, filepath.Join(dir, "a.md"), "alpha") + res := Discover(DiscoverOptions{Roots: []SkillRoot{root(dir)}}) + out := SummarizeDiscovery(res) + if !strings.Contains(out, "active (1)") || !strings.Contains(out, "alpha") { + t.Errorf("summary missing active skill: %q", out) + } +} diff --git a/internal/app/skills/errors.go b/internal/app/skills/errors.go index b50f97d..1f35caa 100644 --- a/internal/app/skills/errors.go +++ b/internal/app/skills/errors.go @@ -85,4 +85,27 @@ var ( "SKILL.md failed one or more validation rules", "fix the reported field violations; see the wrapped details for each", ) + + // --- spec-2.2 discovery (filesystem I/O) --- + + // ErrRootUnreadable — a skill root directory could not be read (permission, + // is-a-symlink, is-a-file, or a Required root is absent). + ErrRootUnreadable = errcode.Register( + "SKILL.ROOT_UNREADABLE", + "skill root directory is not readable", + "check the directory exists and is a non-symlink dir with read permission; or remove it from skill_search_paths", + ) + // ErrFileUnreadable — a SKILL.md file could not be stat'd or read. + ErrFileUnreadable = errcode.Register( + "SKILL.FILE_UNREADABLE", + "SKILL.md file could not be read", + "check the file permissions; or remove the file", + ) + // ErrRootTooManyFiles — a skill root holds more files than the scan cap; + // the extra files were skipped (DoS guard). + ErrRootTooManyFiles = errcode.Register( + "SKILL.ROOT_TOO_MANY_FILES", + "skill root has more files than the scan limit", + "reduce the number of skill files in the directory; the cap is a DoS guard", + ) ) diff --git a/internal/app/skills/errors_test.go b/internal/app/skills/errors_test.go index 5953192..ab8c7d2 100644 --- a/internal/app/skills/errors_test.go +++ b/internal/app/skills/errors_test.go @@ -17,6 +17,8 @@ func allSkillSentinels() []errcode.Sentinel { ErrNoFrontmatter, ErrUnterminatedFrontmatter, ErrParseError, ErrTooLarge, ErrTooDeep, ErrMissingName, ErrInvalidName, ErrMissingDescription, ErrNamespaceConflict, ErrValidationFailed, + // spec-2.2 discovery + ErrRootUnreadable, ErrFileUnreadable, ErrRootTooManyFiles, } } @@ -37,7 +39,7 @@ func TestErrorsRegistered(t *testing.T) { } seen[s.Code()] = true } - if len(seen) != 10 { - t.Errorf("expected 10 SKILL.* codes, got %d", len(seen)) + if len(seen) != 13 { + t.Errorf("expected 13 SKILL.* codes, got %d", len(seen)) } } diff --git a/internal/app/skills/result.go b/internal/app/skills/result.go new file mode 100644 index 0000000..f372b72 --- /dev/null +++ b/internal/app/skills/result.go @@ -0,0 +1,121 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +// File result.go — discovery result + error/warning/ignored shapes + error +// classification (spec-2.2 D-4/D-5). +// +// Membership contract (DoD): every valid skill name appears in EXACTLY one of +// - Active (one winner), or +// - Conflicts with Kind==ConflictUnresolvable (no winner). +// ConflictShadowed losers go to Shadowed; the winner is in Active. Conflicts +// are NOT duplicated into Errors — they carry full provenance (Source.Path of +// every shadowed file) in res.Conflicts (review HIGH/CRIT). + +package skills + +import ( + "errors" + "os" + + "github.com/sqlrush/opendbx/internal/platform/errcode" +) + +// Sentinel causes carried as the PathError root cause so the wrapped message +// is descriptive. +var ( + errSymlinkRoot = errors.New("skill root is a symlink (not followed)") + errNotDir = errors.New("skill root is not a directory") + errNotRegular = errors.New("not a regular file (symlink/device/fifo not followed)") +) + +// DiscoveryError is one filesystem/parse/validate failure, isolated so the +// rest of discovery proceeds. Err is always normalized to an errcode.Error +// (asErrcode) so ClassifyError can read its Code. +type DiscoveryError struct { + Path string + Err error +} + +// DiscoveryWarning carries a non-fatal spec-2.1 Validate warning with the file +// it came from. Collected even when the same file also has a fatal error. +type DiscoveryWarning struct { + Path string + Name string + Warning Warning +} + +// IgnoredSkill records a skill excluded by configuration (disabled), kept +// visible rather than silently dropped (规则 7). +type IgnoredSkill struct { + Skill Skill + Reason string +} + +// DiscoveryResult is the outcome of Discover. All slices are nil when empty. +type DiscoveryResult struct { + Active []Skill + Shadowed []Skill + Conflicts []Conflict + Warnings []DiscoveryWarning + Ignored []IgnoredSkill + Errors []DiscoveryError +} + +// ClassifyError returns the errcode Code of a DiscoveryError (e.g. +// "SKILL.PARSE_ERROR"), or "" if the error is not an errcode.Error. Every +// DiscoveryError.Err is normalized via asErrcode, so this returns a non-empty +// code for every collected error. +func ClassifyError(de DiscoveryError) string { + var ec errcode.Error + if errors.As(de.Err, &ec) { + return ec.Code() + } + return "" +} + +// asErrcode normalizes any error into an errcode.Error so downstream +// classification never sees a bare error. Errors already carrying an errcode +// (Parse/Validate/Conflict.Err and the discovery sentinels) pass through; +// anything else is wrapped under SKILL.FILE_UNREADABLE as a conservative +// default. +func asErrcode(err error) error { + if err == nil { + return nil + } + var ec errcode.Error + if errors.As(err, &ec) { + return err + } + return errcode.Wrap(ErrFileUnreadable.Code(), err, "", "") +} + +// fileUnreadable wraps an os stat/read error under SKILL.FILE_UNREADABLE, +// preserving the OS root cause for errors.Is/As. +func fileUnreadable(err error) error { + return errcode.Wrap(ErrFileUnreadable.Code(), err, "", "") +} + +// rootUnreadable wraps an os error under SKILL.ROOT_UNREADABLE. +func rootUnreadable(err error) error { + return errcode.Wrap(ErrRootUnreadable.Code(), err, "", "") +} + +// tooLargeErr reports a file that exceeds the size cap without reading it. +func tooLargeErr(size int64) error { + return errcode.Newf(ErrTooLarge.Code(), + "SKILL.md is %d bytes, exceeds the %d byte limit", size, maxSkillSize) +} + +// notRegularErr reports a path that is not a regular file (symlink, FIFO, +// socket, device) and so must not be read (review HIGH: fs safety). +func notRegularErr(path string) error { + return errcode.Wrap(ErrFileUnreadable.Code(), + &os.PathError{Op: "load", Path: path, Err: errNotRegular}, "", "") +} + +// errcodeTooManyFiles reports a root whose file count exceeded the cap. +func errcodeTooManyFiles(dir string, found, cap int) error { + return errcode.Newf(ErrRootTooManyFiles.Code(), + "root %q has %d files, exceeds the %d limit; extra files skipped", dir, found, cap) +} diff --git a/internal/app/skills/roots.go b/internal/app/skills/roots.go new file mode 100644 index 0000000..79db44c --- /dev/null +++ b/internal/app/skills/roots.go @@ -0,0 +1,119 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +// File roots.go — skill root model + total-injective precedence (spec-2.2 D-1). +// +// Precedence must be a TOTAL INJECTIVE function over every root — the fixed +// scopes (builtin/user/project × .claude/.opendbx) AND the dynamic ones (N +// plugin caches, M config search paths). AssignPrecedence guarantees this by +// sorting RootSpecs into a canonical low→high order and handing each a +// sequential integer, so no two roots ever tie (review HIGH-3). Cross-root +// same-name then always resolves to a deterministic Shadowed; only two files +// of the same name WITHIN one root tie (→ Unresolvable, surfaced as duplicate). + +package skills + +import "sort" + +// defaultMaxFilesPerRoot caps how many skill files one root may contribute +// before the scan truncates and reports SKILL.ROOT_TOO_MANY_FILES (DoS guard). +const defaultMaxFilesPerRoot = 1000 + +// Precedence bands (low→high). A skill from a higher band wins. Bands are +// descriptive; AssignPrecedence collapses (band, subOrder, dir) into a dense +// injective rank, so band spacing never needs to leave room for sub-orders. +// Exported so bootstrap (BuildSkillRoots) can place roots on the ladder. +const ( + BandBuiltin = 0 + BandPlugin = 1 + BandUser = 2 + BandProject = 3 + BandSearchPath = 4 +) + +// Dual-path variant sub-order within a scope: .opendbx outranks .claude +// (Q3 — opendbx host overrides the portable CC source). +const ( + VariantClaude = 0 + VariantOpendbx = 1 +) + +// RootSpec describes a skill root before precedence assignment. bootstrap +// builds these from HomeDir/cwd/plugins/searchPaths; AssignPrecedence ranks +// them. SubOrder is the deterministic within-band tiebreak (dual-path variant, +// sorted plugin index, or config-list index). +type RootSpec struct { + Kind SourceKind + Band int + SubOrder int + Dir string + PluginID string + Required bool // absent Required root → SKILL.ROOT_UNREADABLE; absent optional → empty +} + +// SkillRoot is a precedence-ranked directory to scan. +type SkillRoot struct { + Kind SourceKind + Precedence int // injective; higher wins + Dir string + PluginID string + Required bool +} + +// DiscoverOptions is the input to Discover. Roots carry injective Precedence +// (from AssignPrecedence). Disabled names are matched by Skill.Key() +// (case-sensitive). MaxFilesPerRoot 0 → defaultMaxFilesPerRoot. +type DiscoverOptions struct { + Roots []SkillRoot + Disabled []string + MaxFilesPerRoot int +} + +// AssignPrecedence ranks specs into SkillRoots with a dense injective +// Precedence. Order key: (Band, SubOrder, Dir) — Dir is the final tiebreak so +// the ranking is deterministic regardless of input order. The same directory +// listed under more than one spec is DEDUPED to its highest-ranked occurrence +// (review: a dir must not shadow itself), so every returned root has a distinct +// Dir and a distinct Precedence. +func AssignPrecedence(specs []RootSpec) []SkillRoot { + ordered := make([]RootSpec, len(specs)) + copy(ordered, specs) + sort.SliceStable(ordered, func(i, j int) bool { + a, b := ordered[i], ordered[j] + if a.Band != b.Band { + return a.Band < b.Band + } + if a.SubOrder != b.SubOrder { + return a.SubOrder < b.SubOrder + } + return a.Dir < b.Dir + }) + + // Dedup by Dir, keeping the highest-ranked occurrence (last in ascending + // order). Walk descending, keep first-seen, then restore ascending order. + seen := make(map[string]bool, len(ordered)) + deduped := make([]RootSpec, 0, len(ordered)) + for i := len(ordered) - 1; i >= 0; i-- { + if seen[ordered[i].Dir] { + continue + } + seen[ordered[i].Dir] = true + deduped = append(deduped, ordered[i]) + } + for l, r := 0, len(deduped)-1; l < r; l, r = l+1, r-1 { + deduped[l], deduped[r] = deduped[r], deduped[l] + } + + out := make([]SkillRoot, len(deduped)) + for i, s := range deduped { + out[i] = SkillRoot{ + Kind: s.Kind, + Precedence: i, // dense, injective; higher index = higher precedence + Dir: s.Dir, + PluginID: s.PluginID, + Required: s.Required, + } + } + return out +} diff --git a/internal/app/skills/roots_test.go b/internal/app/skills/roots_test.go new file mode 100644 index 0000000..030f76e --- /dev/null +++ b/internal/app/skills/roots_test.go @@ -0,0 +1,100 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +package skills + +import "testing" + +// TestAssignPrecedence_Injective — every root gets a distinct precedence +// regardless of plugin/search-path count (review HIGH-3: no cross-root ties). +func TestAssignPrecedence_Injective(t *testing.T) { + t.Parallel() + specs := []RootSpec{ + {Kind: SourceBuiltin, Band: BandBuiltin, SubOrder: 0, Dir: "/builtin"}, + {Kind: SourcePluginCache, Band: BandPlugin, SubOrder: 0, Dir: "/p/a/skills", PluginID: "a"}, + {Kind: SourcePluginCache, Band: BandPlugin, SubOrder: 1, Dir: "/p/b/skills", PluginID: "b"}, + {Kind: SourceUserGlobal, Band: BandUser, SubOrder: VariantClaude, Dir: "/h/.claude/skills"}, + {Kind: SourceUserGlobal, Band: BandUser, SubOrder: VariantOpendbx, Dir: "/h/.opendbx/skills"}, + {Kind: SourceProject, Band: BandProject, SubOrder: VariantClaude, Dir: "/c/.claude/skills"}, + {Kind: SourceProject, Band: BandProject, SubOrder: VariantOpendbx, Dir: "/c/.opendbx/skills"}, + {Kind: SourceProject, Band: BandSearchPath, SubOrder: 0, Dir: "/custom1", Required: true}, + {Kind: SourceProject, Band: BandSearchPath, SubOrder: 1, Dir: "/custom2", Required: true}, + } + roots := AssignPrecedence(specs) + if len(roots) != len(specs) { + t.Fatalf("len = %d; want %d", len(roots), len(specs)) + } + seen := map[int]string{} + for _, r := range roots { + if prev, dup := seen[r.Precedence]; dup { + t.Errorf("precedence %d tied: %s and %s", r.Precedence, prev, r.Dir) + } + seen[r.Precedence] = r.Dir + } +} + +// TestAssignPrecedence_Order — bands rank low→high; .opendbx outranks .claude; +// search paths outrank everything (Q3 + Q8 ladder). +func TestAssignPrecedence_Order(t *testing.T) { + t.Parallel() + specs := []RootSpec{ + {Band: BandSearchPath, SubOrder: 0, Dir: "/search"}, + {Band: BandBuiltin, SubOrder: 0, Dir: "/builtin"}, + {Band: BandProject, SubOrder: VariantOpendbx, Dir: "/proj/.opendbx"}, + {Band: BandProject, SubOrder: VariantClaude, Dir: "/proj/.claude"}, + {Band: BandUser, SubOrder: VariantClaude, Dir: "/user/.claude"}, + } + roots := AssignPrecedence(specs) + prec := map[string]int{} + for _, r := range roots { + prec[r.Dir] = r.Precedence + } + // Each dir must outrank the previous one (strictly ascending ladder). + ladder := []string{"/builtin", "/user/.claude", "/proj/.claude", "/proj/.opendbx", "/search"} + for i := 1; i < len(ladder); i++ { + if prec[ladder[i-1]] >= prec[ladder[i]] { + t.Errorf("ladder order wrong: %s(%d) should be < %s(%d)", + ladder[i-1], prec[ladder[i-1]], ladder[i], prec[ladder[i]]) + } + } +} + +// TestAssignPrecedence_DedupsDir — the same dir under two specs collapses to +// one root (highest-ranked), so a dir cannot shadow itself (review codex). +func TestAssignPrecedence_DedupsDir(t *testing.T) { + t.Parallel() + specs := []RootSpec{ + {Band: BandUser, SubOrder: 0, Dir: "/shared/skills"}, + {Band: BandSearchPath, SubOrder: 0, Dir: "/shared/skills", Required: true}, // same dir, higher band + {Band: BandProject, SubOrder: 0, Dir: "/proj/skills"}, + } + roots := AssignPrecedence(specs) + if len(roots) != 2 { + t.Fatalf("duplicate dir should collapse to 1 root: %d roots %+v", len(roots), roots) + } + for _, r := range roots { + if r.Dir == "/shared/skills" && !r.Required { + t.Errorf("dedup should keep the highest-ranked (Required) occurrence: %+v", r) + } + } +} + +// TestAssignPrecedence_Deterministic — input order does not change the result. +func TestAssignPrecedence_Deterministic(t *testing.T) { + t.Parallel() + a := []RootSpec{ + {Band: BandUser, SubOrder: 0, Dir: "/b"}, + {Band: BandUser, SubOrder: 0, Dir: "/a"}, + } + b := []RootSpec{ + {Band: BandUser, SubOrder: 0, Dir: "/a"}, + {Band: BandUser, SubOrder: 0, Dir: "/b"}, + } + ra, rb := AssignPrecedence(a), AssignPrecedence(b) + for i := range ra { + if ra[i].Dir != rb[i].Dir || ra[i].Precedence != rb[i].Precedence { + t.Errorf("non-deterministic: %v vs %v", ra, rb) + } + } +} diff --git a/internal/app/skills/scan.go b/internal/app/skills/scan.go new file mode 100644 index 0000000..dfa0a58 --- /dev/null +++ b/internal/app/skills/scan.go @@ -0,0 +1,119 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +// File scan.go — directory scanning for skill files (spec-2.2 D-2). Pure I/O, +// no parsing. Filesystem-safety is the focus: +// - Lstat(root.Dir): reject a symlink/non-dir root; an absent OPTIONAL root +// is empty (not an error) — a fresh install has no .claude/.opendbx dirs; +// only an absent REQUIRED root (explicit skill_search_paths) errors. +// - entry-level symlinks are skipped (no following). +// - MaxFilesPerRoot truncates + reports SKILL.ROOT_TOO_MANY_FILES. +// The 1 MiB size cap is enforced in discovery.go BEFORE reading (stat-then-read). + +package skills + +import ( + "io" + "os" + "path/filepath" + "sort" +) + +const skillFileName = "SKILL.md" + +// scanBatch is how many directory entries are read per ReadDir call. Streaming +// in batches bounds memory on a pathological directory (a million-entry dir is +// never materialized at once; review HIGH: ReadDir OOM guard). +const scanBatch = 256 + +// scanRoot lists the SKILL.md files under one root: flat-form `.md` +// directly in the root, and dir-form `/SKILL.md` one level down. It does +// not read file contents. When err is ROOT_TOO_MANY_FILES the returned files +// are the (truncated) prefix and the caller still processes them. +func scanRoot(root SkillRoot, maxFiles int) (files []string, err error) { + info, lerr := os.Lstat(root.Dir) + if lerr != nil { + if os.IsNotExist(lerr) { + if root.Required { + return nil, rootUnreadable(lerr) + } + return nil, nil // optional root absent → empty, not an error + } + return nil, rootUnreadable(lerr) + } + if info.Mode()&os.ModeSymlink != 0 { + return nil, rootUnreadable(&os.PathError{Op: "scan", Path: root.Dir, Err: errSymlinkRoot}) + } + if !info.IsDir() { + return nil, rootUnreadable(&os.PathError{Op: "scan", Path: root.Dir, Err: errNotDir}) + } + + d, oerr := os.Open(root.Dir) + if oerr != nil { + return nil, rootUnreadable(oerr) + } + defer func() { _ = d.Close() }() + + // Stream entries in batches so a huge directory is never fully materialized; + // stop early once maxFiles skill files have been collected. + overLimit := false + for !overLimit { + batch, rerr := d.ReadDir(scanBatch) + for _, e := range batch { + p, ok := classifyEntry(root.Dir, e) + if !ok { + continue + } + if maxFiles > 0 && len(files) >= maxFiles { + overLimit = true + break + } + files = append(files, p) + } + if rerr == io.EOF { + break + } + if rerr != nil { + return files, rootUnreadable(rerr) + } + } + + sort.Strings(files) + if overLimit { + return files, errcodeTooManyFiles(root.Dir, maxFiles, maxFiles) + } + return files, nil +} + +// classifyEntry maps a directory entry to a skill file path, or (._, false) if +// it is not a skill file. Symlinks and non-regular files are rejected: the +// d_type symlink check is a fast path, and isRegularFile (Lstat-based) is the +// reliable backstop for filesystems that return DT_UNKNOWN. +func classifyEntry(dir string, e os.DirEntry) (string, bool) { + if e.Type()&os.ModeSymlink != 0 { + return "", false // never follow symlinked entries + } + switch { + case e.IsDir(): + p := filepath.Join(dir, e.Name(), skillFileName) + if isRegularFile(p) { + return p, true + } + case filepath.Ext(e.Name()) == ".md": + p := filepath.Join(dir, e.Name()) + if isRegularFile(p) { // backstop: rejects DT_UNKNOWN symlinks / FIFO / device + return p, true + } + } + return "", false +} + +// isRegularFile reports whether p exists and is a regular, non-symlink file. +func isRegularFile(p string) bool { + info, err := os.Lstat(p) + if err != nil { + return false + } + return info.Mode().IsRegular() +} diff --git a/internal/app/skills/scan_test.go b/internal/app/skills/scan_test.go new file mode 100644 index 0000000..d6b6880 --- /dev/null +++ b/internal/app/skills/scan_test.go @@ -0,0 +1,122 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +package skills + +import ( + "errors" + "os" + "path/filepath" + "runtime" + "testing" +) + +func writeSkill(t *testing.T, path, name string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + content := "---\nname: " + name + "\ndescription: a skill\n---\nbody\n" + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func root(dir string) SkillRoot { return SkillRoot{Kind: SourceProject, Precedence: 1, Dir: dir} } + +func TestScanRoot_FlatAndDirForm(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeSkill(t, filepath.Join(dir, "flat.md"), "flat") + writeSkill(t, filepath.Join(dir, "dirform", "SKILL.md"), "dirform") + // a non-.md file is ignored. + _ = os.WriteFile(filepath.Join(dir, "README.txt"), []byte("x"), 0o644) + // a subdir without SKILL.md is ignored. + _ = os.MkdirAll(filepath.Join(dir, "empty"), 0o755) + + files, err := scanRoot(root(dir), 1000) + if err != nil { + t.Fatalf("scanRoot: %v", err) + } + if len(files) != 2 { + t.Fatalf("found %d files; want 2 (%v)", len(files), files) + } + bases := map[string]bool{filepath.Base(files[0]): true, filepath.Base(files[1]): true} + if !bases["flat.md"] || !bases["SKILL.md"] { + t.Errorf("expected flat.md + dir-form SKILL.md, got %v", files) + } +} + +func TestScanRoot_MissingOptionalIsEmpty(t *testing.T) { + t.Parallel() + files, err := scanRoot(SkillRoot{Dir: filepath.Join(t.TempDir(), "does-not-exist")}, 1000) + if err != nil || files != nil { + t.Errorf("absent optional root should be empty, got files=%v err=%v", files, err) + } +} + +func TestScanRoot_MissingRequiredErrors(t *testing.T) { + t.Parallel() + _, err := scanRoot(SkillRoot{Dir: filepath.Join(t.TempDir(), "nope"), Required: true}, 1000) + assertCode(t, err, ErrRootUnreadable.Code()) +} + +func TestScanRoot_RootIsFile(t *testing.T) { + t.Parallel() + f := filepath.Join(t.TempDir(), "afile") + _ = os.WriteFile(f, []byte("x"), 0o644) + _, err := scanRoot(root(f), 1000) + assertCode(t, err, ErrRootUnreadable.Code()) +} + +func TestScanRoot_SymlinkRootRejected(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("symlink semantics differ on windows") + } + target := t.TempDir() + writeSkill(t, filepath.Join(target, "x.md"), "x") + link := filepath.Join(t.TempDir(), "link") + if err := os.Symlink(target, link); err != nil { + t.Skipf("symlink unsupported: %v", err) + } + _, err := scanRoot(root(link), 1000) + assertCode(t, err, ErrRootUnreadable.Code()) +} + +func TestScanRoot_SymlinkEntrySkipped(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("symlink semantics differ on windows") + } + dir := t.TempDir() + writeSkill(t, filepath.Join(dir, "real.md"), "real") + other := filepath.Join(t.TempDir(), "other.md") + writeSkill(t, other, "other") + if err := os.Symlink(other, filepath.Join(dir, "linked.md")); err != nil { + t.Skipf("symlink unsupported: %v", err) + } + files, err := scanRoot(root(dir), 1000) + if err != nil { + t.Fatal(err) + } + if len(files) != 1 || filepath.Base(files[0]) != "real.md" { + t.Errorf("symlinked entry should be skipped, got %v", files) + } +} + +func TestScanRoot_MaxFilesTruncates(t *testing.T) { + t.Parallel() + dir := t.TempDir() + for _, n := range []string{"a", "b", "c", "d"} { + writeSkill(t, filepath.Join(dir, n+".md"), n) + } + files, err := scanRoot(root(dir), 2) + if !errors.Is(err, ErrRootTooManyFiles) { + t.Fatalf("want ROOT_TOO_MANY_FILES, got %v", err) + } + if len(files) != 2 { + t.Errorf("truncated to %d; want 2", len(files)) + } +} diff --git a/internal/app/skills/summary.go b/internal/app/skills/summary.go new file mode 100644 index 0000000..6ca71b7 --- /dev/null +++ b/internal/app/skills/summary.go @@ -0,0 +1,82 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +// File summary.go — minimal discovery lister (spec-2.2 D-7). Plain stdlib +// formatting only: app/skills must stay a leaf (no render/table/color import). +// The /debug skills command wires this string into its output at the call site +// (entrypoints/diagnose), not here. + +package skills + +import ( + "fmt" + "sort" + "strings" +) + +// SummarizeDiscovery renders a human-readable, deterministic summary of a +// discovery result: active skills, shadowed, conflicts, warnings, ignored, and +// errors. It does not invoke anything. +func SummarizeDiscovery(res DiscoveryResult) string { + var b strings.Builder + + fmt.Fprintf(&b, "active (%d):\n", len(res.Active)) + for _, s := range res.Active { + fmt.Fprintf(&b, " %s [%s prec=%d] %s\n", s.Schema.Name, s.Source.Kind, s.Source.Precedence, s.Source.Path) + } + + if len(res.Shadowed) > 0 { + fmt.Fprintf(&b, "shadowed (%d):\n", len(res.Shadowed)) + for _, s := range res.Shadowed { + fmt.Fprintf(&b, " %s [%s prec=%d] %s\n", s.Schema.Name, s.Source.Kind, s.Source.Precedence, s.Source.Path) + } + } + + if len(res.Conflicts) > 0 { + fmt.Fprintf(&b, "conflicts (%d):\n", len(res.Conflicts)) + for _, c := range res.Conflicts { + kind := "shadowed" + if c.Kind == ConflictUnresolvable { + kind = "unresolvable" + } + fmt.Fprintf(&b, " %s [%s] paths=%s\n", c.Name, kind, strings.Join(conflictPaths(c), ", ")) + } + } + + if len(res.Warnings) > 0 { + fmt.Fprintf(&b, "warnings (%d):\n", len(res.Warnings)) + for _, w := range res.Warnings { + fmt.Fprintf(&b, " %s %s: %s\n", w.Path, w.Name, w.Warning.Detail) + } + } + + if len(res.Ignored) > 0 { + fmt.Fprintf(&b, "ignored (%d):\n", len(res.Ignored)) + for _, ig := range res.Ignored { + fmt.Fprintf(&b, " %s (%s) %s\n", ig.Skill.Schema.Name, ig.Reason, ig.Skill.Source.Path) + } + } + + if len(res.Errors) > 0 { + fmt.Fprintf(&b, "errors (%d):\n", len(res.Errors)) + for _, de := range res.Errors { + fmt.Fprintf(&b, " %s [%s]\n", de.Path, ClassifyError(de)) + } + } + + return b.String() +} + +// conflictPaths returns the source paths of every skill in a conflict, sorted. +func conflictPaths(c Conflict) []string { + paths := make([]string, 0, len(c.Shadowed)+1) + if c.Winner != nil { + paths = append(paths, c.Winner.Source.Path) + } + for _, s := range c.Shadowed { + paths = append(paths, s.Source.Path) + } + sort.Strings(paths) + return paths +} diff --git a/internal/app/skills/summary_test.go b/internal/app/skills/summary_test.go new file mode 100644 index 0000000..0733bc3 --- /dev/null +++ b/internal/app/skills/summary_test.go @@ -0,0 +1,50 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +package skills + +import ( + "strings" + "testing" + + "github.com/sqlrush/opendbx/internal/platform/errcode" +) + +// TestSummarizeDiscovery_AllSections exercises every render branch (active, +// shadowed, conflicts incl conflictPaths, warnings, ignored, errors) so a +// formatting regression in any section is caught (review go MED-2). +func TestSummarizeDiscovery_AllSections(t *testing.T) { + t.Parallel() + mk := func(name, path string, prec int) Skill { + return Skill{Schema: Schema{Name: name}, Source: SkillSource{Kind: SourceProject, Precedence: prec, Path: path}} + } + winner := mk("dup", "/hi/dup.md", 2) + res := DiscoveryResult{ + Active: []Skill{mk("alpha", "/a/alpha.md", 1)}, + Shadowed: []Skill{mk("dup", "/lo/dup.md", 1)}, + Conflicts: []Conflict{{ + Name: "dup", Kind: ConflictShadowed, Winner: &winner, + Shadowed: []Skill{mk("dup", "/lo/dup.md", 1)}, + }, { + Name: "amb", Kind: ConflictUnresolvable, + Shadowed: []Skill{mk("amb", "/x/amb.md", 5), mk("amb", "/x/amb2.md", 5)}, + }}, + Warnings: []DiscoveryWarning{{Path: "/a/w.md", Name: "warned", Warning: Warning{Detail: "non-kebab"}}}, + Ignored: []IgnoredSkill{{Skill: mk("off", "/a/off.md", 1), Reason: "disabled"}}, + Errors: []DiscoveryError{{Path: "/bad/x.md", Err: errcode.New(ErrParseError.Code(), "", "")}}, + } + out := SummarizeDiscovery(res) + for _, want := range []string{ + "active (1)", "alpha", + "shadowed (1)", + "conflicts (2)", "unresolvable", "/x/amb.md", + "warnings (1)", "non-kebab", + "ignored (1)", "off", "disabled", + "errors (1)", "SKILL.PARSE_ERROR", + } { + if !strings.Contains(out, want) { + t.Errorf("summary missing %q\n---\n%s", want, out) + } + } +} diff --git a/internal/bootstrap/skills.go b/internal/bootstrap/skills.go new file mode 100644 index 0000000..d64ec81 --- /dev/null +++ b/internal/bootstrap/skills.go @@ -0,0 +1,132 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +// File skills.go — wires config + paths into skill discovery options +// (spec-2.2 D-6). BuildSkillRoots is a PURE function of its inputs (no fs I/O — +// the scanning happens later in skills.Discover), so the home/cwd/plugin +// derivation is unit-testable without a real filesystem. +// +// Skill roots are derived DIRECTLY from HomeDir + CWD (review HIGH): config +// SourcePaths point at config FILES (and on macOS live under Application +// Support), which are the wrong basis for ~/.claude/skills-style roots. + +package bootstrap + +import ( + "os" + "path/filepath" + "sort" + + "github.com/sqlrush/opendbx/internal/app/skills" + "github.com/sqlrush/opendbx/internal/platform/config" +) + +// PluginDir is one installed plugin's skills directory + its stable ID. +type PluginDir struct { + ID string + Dir string +} + +// SkillPathInputs are the absolute filesystem anchors for skill discovery. +// HomeDir/CWD drive the default .claude/.opendbx roots; BuiltinDir is the +// optional bundled-skills root (empty in v1); PluginDirs are installed plugins. +type SkillPathInputs struct { + HomeDir string + CWD string + BuiltinDir string + PluginDirs []PluginDir +} + +// skillsSubdir names the per-scope skill directory under .claude / .opendbx. +const skillsSubdir = "skills" + +// BuildSkillRoots constructs discovery options from the path anchors and the +// plugin config. Roots get injective precedence via skills.AssignPrecedence +// (builtin < plugin < user < project < search-path; .opendbx outranks .claude +// within a scope). Config search paths are Required (an absent one is an +// error, unlike the always-optional defaults). +func BuildSkillRoots(in SkillPathInputs, cfg config.PluginsConfig) skills.DiscoverOptions { + var specs []skills.RootSpec + + if in.BuiltinDir != "" { + specs = append(specs, skills.RootSpec{ + Kind: skills.SourceBuiltin, Band: skills.BandBuiltin, SubOrder: 0, + Dir: in.BuiltinDir, + }) + } + + // Sort plugins by ID (then Dir) so SubOrder — and thus cross-plugin + // same-name precedence — is deterministic regardless of caller order + // (locked Q8). A copy keeps the input slice immutable. + plugins := append([]PluginDir(nil), in.PluginDirs...) + sort.Slice(plugins, func(i, j int) bool { + if plugins[i].ID != plugins[j].ID { + return plugins[i].ID < plugins[j].ID + } + return plugins[i].Dir < plugins[j].Dir + }) + for i, pd := range plugins { + specs = append(specs, skills.RootSpec{ + Kind: skills.SourcePluginCache, Band: skills.BandPlugin, SubOrder: i, + Dir: pd.Dir, PluginID: pd.ID, + }) + } + + if in.HomeDir != "" { + specs = append(specs, + skills.RootSpec{Kind: skills.SourceUserGlobal, Band: skills.BandUser, SubOrder: skills.VariantClaude, + Dir: filepath.Join(in.HomeDir, ".claude", skillsSubdir)}, + skills.RootSpec{Kind: skills.SourceUserGlobal, Band: skills.BandUser, SubOrder: skills.VariantOpendbx, + Dir: filepath.Join(in.HomeDir, ".opendbx", skillsSubdir)}, + ) + } + + if in.CWD != "" { + specs = append(specs, + skills.RootSpec{Kind: skills.SourceProject, Band: skills.BandProject, SubOrder: skills.VariantClaude, + Dir: filepath.Join(in.CWD, ".claude", skillsSubdir)}, + skills.RootSpec{Kind: skills.SourceProject, Band: skills.BandProject, SubOrder: skills.VariantOpendbx, + Dir: filepath.Join(in.CWD, ".opendbx", skillsSubdir)}, + ) + } + + for i, p := range cfg.SkillSearchPaths { + dir := p + if !filepath.IsAbs(dir) { + dir = filepath.Join(in.CWD, dir) + } + specs = append(specs, skills.RootSpec{ + Kind: skills.SourceProject, Band: skills.BandSearchPath, SubOrder: i, + Dir: filepath.Clean(dir), Required: true, + }) + } + + return skills.DiscoverOptions{ + Roots: skills.AssignPrecedence(specs), + Disabled: cfg.DisabledSkills, + } +} + +// DiscoverSkills resolves the real home/cwd anchors and runs a one-shot +// discovery (spec-2.2 D-7 wiring; v1 has no builtin/plugin roots). It is the +// runnable entry the /debug-style lister calls. +// +// A failed UserHomeDir/Getwd yields an empty anchor: the corresponding default +// roots are simply skipped (they are optional, so absent → empty), and any +// relative skill_search_paths then become unanchored Required roots that +// surface as SKILL.ROOT_UNREADABLE in the result — observable, not silent. +func DiscoverSkills(cfg *config.Config) skills.DiscoveryResult { + home, _ := os.UserHomeDir() + cwd, _ := os.Getwd() + opts := BuildSkillRoots(SkillPathInputs{HomeDir: home, CWD: cwd}, cfg.Plugins) + return skills.Discover(opts) +} + +// DiscoverSkillsSummary runs discovery and formats it. Kept here (not in +// entrypoints) so the skills-package dependency stays in the app layer: +// entrypoints → bootstrap → app/skills is legal; entrypoints → app/skills is +// not (layer rule). +func DiscoverSkillsSummary(cfg *config.Config) string { + return skills.SummarizeDiscovery(DiscoverSkills(cfg)) +} diff --git a/internal/bootstrap/skills_test.go b/internal/bootstrap/skills_test.go new file mode 100644 index 0000000..2739c2f --- /dev/null +++ b/internal/bootstrap/skills_test.go @@ -0,0 +1,117 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +package bootstrap + +import ( + "path/filepath" + "testing" + + "github.com/sqlrush/opendbx/internal/app/skills" + "github.com/sqlrush/opendbx/internal/platform/config" +) + +func dirByName(roots []skills.SkillRoot) map[string]skills.SkillRoot { + m := make(map[string]skills.SkillRoot, len(roots)) + for _, r := range roots { + m[r.Dir] = r + } + return m +} + +// TestBuildSkillRoots_DerivesFromHomeAndCWD — roots come from HomeDir + CWD +// directly, NOT from config SourcePaths (review HIGH-1). +func TestBuildSkillRoots_DerivesFromHomeAndCWD(t *testing.T) { + t.Parallel() + in := SkillPathInputs{HomeDir: "/home/u", CWD: "/proj"} + opts := BuildSkillRoots(in, config.PluginsConfig{}) + dirs := dirByName(opts.Roots) + for _, want := range []string{ + "/home/u/.claude/skills", "/home/u/.opendbx/skills", + "/proj/.claude/skills", "/proj/.opendbx/skills", + } { + if _, ok := dirs[want]; !ok { + t.Errorf("missing derived root %q (have %v)", want, keys(dirs)) + } + } +} + +// TestBuildSkillRoots_Ladder — project outranks user; .opendbx outranks +// .claude; search paths outrank everything (Q3 + Q8). +func TestBuildSkillRoots_Ladder(t *testing.T) { + t.Parallel() + in := SkillPathInputs{HomeDir: "/h", CWD: "/c"} + cfg := config.PluginsConfig{SkillSearchPaths: []string{"/custom/skills"}} + opts := BuildSkillRoots(in, cfg) + p := map[string]int{} + for _, r := range opts.Roots { + p[r.Dir] = r.Precedence + } + checks := [][2]string{ + {"/h/.claude/skills", "/h/.opendbx/skills"}, + {"/h/.opendbx/skills", "/c/.claude/skills"}, + {"/c/.claude/skills", "/c/.opendbx/skills"}, + {"/c/.opendbx/skills", "/custom/skills"}, + } + for _, c := range checks { + if p[c[0]] >= p[c[1]] { + t.Errorf("precedence %s(%d) should be < %s(%d)", c[0], p[c[0]], c[1], p[c[1]]) + } + } +} + +// TestBuildSkillRoots_SearchPathRequiredAndCleaned — config search paths are +// Required and resolved against cwd. +func TestBuildSkillRoots_SearchPathRequiredAndCleaned(t *testing.T) { + t.Parallel() + in := SkillPathInputs{HomeDir: "/h", CWD: "/c"} + cfg := config.PluginsConfig{SkillSearchPaths: []string{"rel/skills", "/abs/skills"}} + opts := BuildSkillRoots(in, cfg) + dirs := dirByName(opts.Roots) + rel, ok := dirs[filepath.Join("/c", "rel", "skills")] + if !ok || !rel.Required { + t.Errorf("relative search path should resolve under cwd + be Required: %v", keys(dirs)) + } + if abs, ok := dirs["/abs/skills"]; !ok || !abs.Required { + t.Errorf("absolute search path should be kept + Required") + } +} + +// TestBuildSkillRoots_Plugins — installed plugin dirs become injective roots. +func TestBuildSkillRoots_Plugins(t *testing.T) { + t.Parallel() + in := SkillPathInputs{ + HomeDir: "/h", CWD: "/c", + PluginDirs: []PluginDir{{ID: "a", Dir: "/cache/a/skills"}, {ID: "b", Dir: "/cache/b/skills"}}, + } + opts := BuildSkillRoots(in, config.PluginsConfig{}) + seen := map[int]bool{} + for _, r := range opts.Roots { + if seen[r.Precedence] { + t.Errorf("precedence %d not injective", r.Precedence) + } + seen[r.Precedence] = true + } + dirs := dirByName(opts.Roots) + if dirs["/cache/a/skills"].PluginID != "a" { + t.Errorf("plugin id not carried: %+v", dirs["/cache/a/skills"]) + } +} + +func TestBuildSkillRoots_DisabledPassthrough(t *testing.T) { + t.Parallel() + opts := BuildSkillRoots(SkillPathInputs{HomeDir: "/h", CWD: "/c"}, + config.PluginsConfig{DisabledSkills: []string{"x", "y"}}) + if len(opts.Disabled) != 2 { + t.Errorf("disabled list not passed through: %v", opts.Disabled) + } +} + +func keys(m map[string]skills.SkillRoot) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +} diff --git a/internal/entrypoints/skills_relay.go b/internal/entrypoints/skills_relay.go new file mode 100644 index 0000000..76af906 --- /dev/null +++ b/internal/entrypoints/skills_relay.go @@ -0,0 +1,24 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +// Skills relay (spec-2.2 D-7). Routes cmd/opendbx → bootstrap.DiscoverSkills + +// skills.SummarizeDiscovery so cmd keeps its single platform exception +// (version only). Minimal lister surface; the full /skills command tree is +// spec-2.18. + +package entrypoints + +import "github.com/sqlrush/opendbx/internal/bootstrap" + +// SkillsSummary loads the default config, runs a one-shot skill discovery, and +// returns a human-readable summary (active/shadowed/conflicts/warnings/ +// ignored/errors). It performs no invocation. The discovery + formatting live +// in bootstrap so entrypoints does not import app/skills directly (layer rule). +func SkillsSummary() (string, error) { + cfg, err := LoadConfigDefault() + if err != nil { + return "", err + } + return bootstrap.DiscoverSkillsSummary(cfg), nil +} diff --git a/internal/platform/config/config.go b/internal/platform/config/config.go index 8b7a4a8..c1e1656 100644 --- a/internal/platform/config/config.go +++ b/internal/platform/config/config.go @@ -38,6 +38,7 @@ type Config struct { Trace TraceConfig `yaml:"trace" json:"trace"` Scheduler SchedulerConfig `yaml:"scheduler" json:"scheduler"` Diagnose DiagnoseConfig `yaml:"diagnose" json:"diagnose"` + Plugins PluginsConfig `yaml:"plugins" json:"plugins"` // spec-2.2 skill discovery // DefaultConnection selects the active connection by alias when no // --connection-alias CLI flag is given (spec-1.19 D-1/D-6). diff --git a/internal/platform/config/plugins.go b/internal/platform/config/plugins.go new file mode 100644 index 0000000..814b569 --- /dev/null +++ b/internal/platform/config/plugins.go @@ -0,0 +1,79 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +// File plugins.go — PluginsConfig (spec-2.2 D-6). Skill discovery configuration: +// extra search paths + disabled skill names. The actual filesystem root +// derivation lives in bootstrap (BuildSkillRoots); config only holds + validates +// the declarative values. + +package config + +import ( + "path/filepath" + "strconv" + "strings" +) + +// maxSkillSearchPaths caps how many extra skill roots a config may declare +// (DoS / footgun guard). +const maxSkillSearchPaths = 10 + +// PluginsConfig declares skill-discovery inputs (spec-2.2). Extension point for +// future plugin features; only skill fields are populated now (§3.7 append-only). +type PluginsConfig struct { + // SkillSearchPaths are extra directories to scan for SKILL.md, beyond the + // default ~/.claude, ~/.opendbx, ./.claude, ./.opendbx roots. + SkillSearchPaths []string `yaml:"skill_search_paths,omitempty" json:"skill_search_paths,omitempty"` + // DisabledSkills are skill names (Skill.Key()) to exclude from discovery. + DisabledSkills []string `yaml:"disabled_skills,omitempty" json:"disabled_skills,omitempty"` +} + +// validatePlugins checks the plugin/skill discovery config. Rejects lexical +// path traversal (a `..` component that escapes after Clean) in search paths +// and caps their count. Path canonicalisation (Abs/Clean) happens later in +// bootstrap with the real cwd, and discovery rejects a symlinked root dir +// (scanRoot Lstat). +// +// v1 scope (single-user): this lexical guard + the symlink-root rejection +// cover the realistic threats. Full realpath containment (EvalSymlinks + +// cwd-subtree enforcement for project-sourced paths, blocking intermediate +// symlink components) is deferred to the enterprise/multi-user deployment +// (spec-3.9), where untrusted shared config is the threat model. +func validatePlugins(cfg *Config, errs *ValidationErrors) { + if cfg == nil { + return + } + src := cfg.Source("Plugins").String() + paths := cfg.Plugins.SkillSearchPaths + + if len(paths) > maxSkillSearchPaths { + *errs = append(*errs, ValidationError{ + Path: "Plugins.SkillSearchPaths", Rule: "max", + Expected: "at most " + strconv.Itoa(maxSkillSearchPaths) + " entries", + Actual: strconv.Itoa(len(paths)), Source: src, + }) + } + for i, p := range paths { + if hasDotDot(p) { + *errs = append(*errs, ValidationError{ + Path: "Plugins.SkillSearchPaths[" + strconv.Itoa(i) + "]", + Rule: "no-traversal", + Expected: "no .. path component (traversal guard)", + Actual: p, Source: src, + }) + } + } +} + +// hasDotDot reports whether a path contains a ".." component (before or after +// cleaning), guarding against directory traversal. +func hasDotDot(p string) bool { + cleaned := filepath.ToSlash(filepath.Clean(p)) + for _, seg := range strings.Split(cleaned, "/") { + if seg == ".." { + return true + } + } + return false +} diff --git a/internal/platform/config/plugins_test.go b/internal/platform/config/plugins_test.go new file mode 100644 index 0000000..dab182e --- /dev/null +++ b/internal/platform/config/plugins_test.go @@ -0,0 +1,91 @@ +// Copyright 2026 opendbx contributors. See LICENSE. +// +// Author: sqlrush + +package config + +import ( + "strings" + "testing" +) + +func TestValidatePlugins_OK(t *testing.T) { + t.Parallel() + cfg := &Config{Plugins: PluginsConfig{ + SkillSearchPaths: []string{"/abs/skills", "rel/skills"}, + DisabledSkills: []string{"noisy-skill"}, + }} + var errs ValidationErrors + validatePlugins(cfg, &errs) + if len(errs) != 0 { + t.Errorf("clean plugins config should pass: %v", errs) + } +} + +func TestValidatePlugins_RejectsTraversal(t *testing.T) { + t.Parallel() + cfg := &Config{Plugins: PluginsConfig{ + SkillSearchPaths: []string{"../../etc", "ok/skills", "a/../../b"}, + }} + var errs ValidationErrors + validatePlugins(cfg, &errs) + n := 0 + for _, e := range errs { + if e.Rule == "no-traversal" { + n++ + } + } + if n != 2 { + t.Errorf("expected 2 traversal rejections, got %d (%v)", n, errs) + } +} + +func TestValidatePlugins_CapsCount(t *testing.T) { + t.Parallel() + many := make([]string, maxSkillSearchPaths+1) + for i := range many { + many[i] = "p" + } + cfg := &Config{Plugins: PluginsConfig{SkillSearchPaths: many}} + var errs ValidationErrors + validatePlugins(cfg, &errs) + found := false + for _, e := range errs { + if e.Path == "Plugins.SkillSearchPaths" && e.Rule == "max" { + found = true + } + } + if !found { + t.Errorf("expected max-count violation, got %v", errs) + } +} + +func TestHasDotDot(t *testing.T) { + t.Parallel() + // Clean resolves non-escaping ".." (a/b/.. → a), so only an ESCAPING ".." + // component survives — that is exactly the traversal we reject. + cases := map[string]bool{ + "/abs/ok": false, + "rel/ok": false, + "../escape": true, // leading .. escapes the base + "a/../../b": true, // cleans to ../b — escapes + "a/b/..": false, // cleans to a — safe, stays within + "..hidden/ok": false, // ".." only as a full component + "/abs/../back": false, // cleans to /back — absolute, no escaping .. + } + for p, want := range cases { + if got := hasDotDot(p); got != want { + t.Errorf("hasDotDot(%q) = %v; want %v", p, got, want) + } + } +} + +func TestValidatePlugins_ErrorMessageFormat(t *testing.T) { + t.Parallel() + cfg := &Config{Plugins: PluginsConfig{SkillSearchPaths: []string{"../x"}}} + var errs ValidationErrors + validatePlugins(cfg, &errs) + if len(errs) == 0 || !strings.Contains(errs.Error(), "traversal") { + t.Errorf("traversal error should mention traversal: %v", errs) + } +} diff --git a/internal/platform/config/validation.go b/internal/platform/config/validation.go index a3a6187..84d3f8f 100644 --- a/internal/platform/config/validation.go +++ b/internal/platform/config/validation.go @@ -92,6 +92,7 @@ func validateCrossField(cfg *Config, errs *ValidationErrors) { } validateConnections(cfg, errs) + validatePlugins(cfg, errs) // spec-2.2 D-6: skill search path traversal + count guard } // validateConnections enforces the spec-1.19 ConnectionConfig cross-field diff --git a/internal/platform/errcode/testdata/error-codes-frozen.txt b/internal/platform/errcode/testdata/error-codes-frozen.txt index d5bd850..21ec90d 100644 --- a/internal/platform/errcode/testdata/error-codes-frozen.txt +++ b/internal/platform/errcode/testdata/error-codes-frozen.txt @@ -50,12 +50,15 @@ RENDER.STREAM_FILTERED RENDER.STREAM_TRUNCATED RENDER.UNSUPPORTED_NODE REPORT.WRITE_FAILED +SKILL.FILE_UNREADABLE SKILL.INVALID_NAME SKILL.MISSING_DESCRIPTION SKILL.MISSING_NAME SKILL.NAMESPACE_CONFLICT SKILL.NO_FRONTMATTER SKILL.PARSE_ERROR +SKILL.ROOT_TOO_MANY_FILES +SKILL.ROOT_UNREADABLE SKILL.TOO_DEEP SKILL.TOO_LARGE SKILL.UNTERMINATED_FRONTMATTER