From bf0b67f9af2e7d68e1a66a8f77d7a04f9bdc1ede Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 4 Jul 2026 16:46:37 -0400 Subject: [PATCH] feat(skillpack): SKILL.md-subscription battery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New additive, nil-safe battery for subscribing to skill packages in the Anthropic agent-skills format (SKILL.md manifest + bundled files): - Manifest/ParseManifest: SKILL.md frontmatter+body parse & validation (name/description required, allowed-tools passthrough, kebab/length limits) - Tree/Pack/LoadPack: self-contained file set, order-independent content digest (the pin identity + change signal), bundled-file listing, traversal- safe staging - Source (DirSource, GitSource): Fetch returns tree + resolved ref; git clones to temp, reads subpath into memory, cleans up (self-contained tree) - Subscription + Store + content-addressed PackCache, with Memory defaults - Syncer: Subscribe pins; Check records a PENDING update but never moves the pin; Apply is the only re-pin (supply-chain guard — upstream can't silently change what an agent runs) - Activate: resolved packs -> majordomo agent.Skill (catalog instructions + one skill_use tool) for progressive disclosure; Stage materializes files Third distinct 'skill' concept, deliberately separate from executus/skill (saved-agent noun) and majordomo/skill (eager capability bundle). Mort-side wiring (convars, .skillpack commands, Agent.SkillPacks, allowed-tools shim) is a later, separate step. Full unit + hermetic local-git tests; gofmt/vet clean; race-tested. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 8 ++ skillpack/activation.go | 126 +++++++++++++++++++++++ skillpack/activation_test.go | 124 ++++++++++++++++++++++ skillpack/manifest.go | 192 +++++++++++++++++++++++++++++++++++ skillpack/manifest_test.go | 82 +++++++++++++++ skillpack/memory.go | 114 +++++++++++++++++++++ skillpack/pack.go | 131 ++++++++++++++++++++++++ skillpack/pack_test.go | 80 +++++++++++++++ skillpack/skillpack.go | 54 ++++++++++ skillpack/source.go | 133 ++++++++++++++++++++++++ skillpack/source_test.go | 97 ++++++++++++++++++ skillpack/store.go | 27 +++++ skillpack/subscription.go | 64 ++++++++++++ skillpack/sync.go | 191 ++++++++++++++++++++++++++++++++++ skillpack/sync_test.go | 176 ++++++++++++++++++++++++++++++++ 15 files changed, 1599 insertions(+) create mode 100644 skillpack/activation.go create mode 100644 skillpack/activation_test.go create mode 100644 skillpack/manifest.go create mode 100644 skillpack/manifest_test.go create mode 100644 skillpack/memory.go create mode 100644 skillpack/pack.go create mode 100644 skillpack/pack_test.go create mode 100644 skillpack/skillpack.go create mode 100644 skillpack/source.go create mode 100644 skillpack/source_test.go create mode 100644 skillpack/store.go create mode 100644 skillpack/subscription.go create mode 100644 skillpack/sync.go create mode 100644 skillpack/sync_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 3b6af72..a626b78 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,6 +85,14 @@ BATTERIES (opt-in siblings, each nil-safe + a default): (throttled Save/Complete/Fail) + Memory budget/ DBBudget rolling-7d + NoOp (run.Budget); [P4 ✓] BudgetStorage iface + Memory default + skillpack/ SKILL.md-subscription battery: Manifest + [P5 ✓] + Source (Dir/Git) + Subscription/Store + + content-addressed PackCache + Syncer + (pending-only; Apply re-pins) + Activate → + majordomo agent.Skill (catalog + skill_use, + progressive disclosure) + Memory defaults. + NOT executus/skill (saved-agent noun) nor + majordomo/skill (eager capability bundle). contrib/store/ SECOND module (+ modernc.org/sqlite): [P4 ✓] pure-Go SQLite impls of ALL store seams: budget + diff --git a/skillpack/activation.go b/skillpack/activation.go new file mode 100644 index 0000000..abe8515 --- /dev/null +++ b/skillpack/activation.go @@ -0,0 +1,126 @@ +package skillpack + +import ( + "context" + "fmt" + "sort" + "strings" + + mdagent "gitea.stevedudenhoeffer.com/steve/majordomo/agent" + "gitea.stevedudenhoeffer.com/steve/majordomo/llm" + mdskill "gitea.stevedudenhoeffer.com/steve/majordomo/skill" +) + +// Resolve loads the pinned Pack for each enabled subscription from the cache. It +// is how a host turns "this agent subscribes to these packs" into activatable +// packs at run time without touching the network. A pinned digest missing from +// the cache is an error (the host should have cached it at pin/apply time). +// Disabled subscriptions are skipped. +func Resolve(ctx context.Context, cache PackCache, subs []Subscription) ([]*Pack, error) { + out := make([]*Pack, 0, len(subs)) + for i := range subs { + s := &subs[i] + if !s.Enabled { + continue + } + tree, err := cache.Get(ctx, s.PinnedDigest) + if err != nil { + return nil, fmt.Errorf("skillpack: resolving %q: %w", s.Name, err) + } + pack, err := LoadPack(tree) + if err != nil { + return nil, fmt.Errorf("skillpack: loading %q: %w", s.Name, err) + } + out = append(out, pack) + } + return out, nil +} + +// Catalog renders the always-in-prompt block for a set of packs: one line per +// pack (name + description) plus how to load one. This is the whole prompt cost +// of a subscription — the bodies stay out until skill_use is called. +func Catalog(packs []*Pack) string { + if len(packs) == 0 { + return "" + } + sorted := append([]*Pack(nil), packs...) + sort.Slice(sorted, func(i, j int) bool { return sorted[i].Manifest.Name < sorted[j].Manifest.Name }) + + var b strings.Builder + b.WriteString("You have access to skills — packaged instructions for specific tasks. ") + b.WriteString("When a task matches one, call skill_use with its name to load its full instructions before proceeding.\n\n") + b.WriteString("Available skills:\n") + for _, p := range sorted { + fmt.Fprintf(&b, "- %s: %s\n", p.Manifest.Name, p.Manifest.Description) + } + return strings.TrimRight(b.String(), "\n") +} + +type skillUseArgs struct { + Name string `json:"name" description:"the exact name of the skill to load, from the Available skills list"` +} + +// Activate turns a set of resolved packs into a majordomo agent.Skill: its +// Instructions are the Catalog, and it contributes a single skill_use tool that +// returns a named pack's full body (progressive disclosure). Attach the result +// to an agent with agent.WithSkill. Returns nil when there are no packs, which +// agent.WithSkill tolerates (a nil skill contributes nothing). +// +// stagedDir, if non-empty, is the directory a host has staged the packs' bundled +// files into (see Stage); skill_use appends the concrete path so the model knows +// where to read scripts/references with its file tools. Leave it empty when the +// host has no staging. +func Activate(packs []*Pack, stagedDir string) mdagent.Skill { + if len(packs) == 0 { + return nil + } + byName := make(map[string]*Pack, len(packs)) + for _, p := range packs { + byName[p.Manifest.Name] = p + } + + tool := llm.DefineTool("skill_use", + "Load the full instructions for a skill by name before doing a task it covers. Returns the skill's instructions and a list of any bundled files.", + func(_ context.Context, args skillUseArgs) (any, error) { + p, ok := byName[strings.TrimSpace(args.Name)] + if !ok { + return fmt.Sprintf("No skill named %q. Use one of the names from the Available skills list.", args.Name), nil + } + return renderPackBody(p, stagedDir), nil + }) + + tb := llm.NewToolbox("skillpack", tool) + return mdskill.New("skillpacks", + mdskill.WithInstructions(Catalog(packs)), + mdskill.WithToolbox(tb), + ) +} + +// renderPackBody is what skill_use returns: the pack's instructions plus a +// pointer to its bundled files (with the staged path when known). +func renderPackBody(p *Pack, stagedDir string) string { + var b strings.Builder + fmt.Fprintf(&b, "# Skill: %s\n\n%s\n", p.Manifest.Name, p.Manifest.Body) + if len(p.Bundled) > 0 { + b.WriteString("\nBundled files") + if stagedDir != "" { + fmt.Fprintf(&b, " (under %s)", strings.TrimRight(stagedDir, "/")+"/"+p.Manifest.Name) + } + b.WriteString(":\n") + for _, f := range p.Bundled { + fmt.Fprintf(&b, "- %s\n", f) + } + } + return strings.TrimRight(b.String(), "\n") +} + +// Stage materializes a pack's files under baseDir// so a host can +// mount them (read-only is the host's concern) into a sandbox the agent's file +// tools can read. Returns the pack's staged directory. +func Stage(p *Pack, baseDir string) (string, error) { + dir := baseDir + "/" + p.Manifest.Name + if err := p.Tree.WriteTo(dir); err != nil { + return "", err + } + return dir, nil +} diff --git a/skillpack/activation_test.go b/skillpack/activation_test.go new file mode 100644 index 0000000..eb13e08 --- /dev/null +++ b/skillpack/activation_test.go @@ -0,0 +1,124 @@ +package skillpack + +import ( + "context" + "encoding/json" + "strings" + "testing" +) + +func mustPack(t *testing.T, name, body string, extra map[string]string) *Pack { + t.Helper() + tr := packTree(name, body) + for k, v := range extra { + tr[k] = []byte(v) + } + p, err := LoadPack(tr) + if err != nil { + t.Fatal(err) + } + return p +} + +func TestCatalog(t *testing.T) { + packs := []*Pack{ + mustPack(t, "zebra", "z", nil), + mustPack(t, "alpha", "a", nil), + } + cat := Catalog(packs) + if !strings.Contains(cat, "skill_use") { + t.Error("catalog should tell the model how to load a skill") + } + ai := strings.Index(cat, "alpha") + zi := strings.Index(cat, "zebra") + if ai < 0 || zi < 0 || ai > zi { + t.Errorf("catalog should list packs sorted by name:\n%s", cat) + } + if Catalog(nil) != "" { + t.Error("empty catalog should be empty string") + } +} + +func TestActivate_SkillUseTool(t *testing.T) { + ctx := context.Background() + packs := []*Pack{ + mustPack(t, "pdf", "Use pdfplumber.", map[string]string{"scripts/x.py": "print()"}), + } + sk := Activate(packs, "/stage") + if sk == nil { + t.Fatal("expected a non-nil skill") + } + if sk.Instructions() != Catalog(packs) { + t.Error("skill instructions should be the catalog") + } + tb := sk.Tools() + tool, ok := tb.Get("skill_use") + if !ok { + t.Fatal("skill_use tool missing from toolbox") + } + + // load an existing pack + out, err := tool.Handler(ctx, json.RawMessage(`{"name":"pdf"}`)) + if err != nil { + t.Fatal(err) + } + body, _ := out.(string) + if !strings.Contains(body, "Use pdfplumber.") { + t.Errorf("skill_use body missing instructions: %q", body) + } + if !strings.Contains(body, "scripts/x.py") || !strings.Contains(body, "/stage/pdf") { + t.Errorf("skill_use should list bundled files under the staged dir: %q", body) + } + + // unknown pack returns guidance, not an error + out, err = tool.Handler(ctx, json.RawMessage(`{"name":"nope"}`)) + if err != nil { + t.Fatal(err) + } + if s, _ := out.(string); !strings.Contains(s, "No skill named") { + t.Errorf("unknown skill should return guidance: %q", s) + } +} + +func TestActivate_Empty(t *testing.T) { + if Activate(nil, "") != nil { + t.Error("no packs should activate to a nil skill") + } +} + +func TestResolveFromCache(t *testing.T) { + ctx := context.Background() + cache := NewMemoryPackCache() + p := mustPack(t, "alpha", "a", nil) + cache.Put(ctx, p.Digest, p.Tree) + + subs := []Subscription{ + {Name: "alpha", PinnedDigest: p.Digest, Enabled: true}, + {Name: "disabled", PinnedDigest: p.Digest, Enabled: false}, + } + packs, err := Resolve(ctx, cache, subs) + if err != nil { + t.Fatal(err) + } + if len(packs) != 1 || packs[0].Manifest.Name != "alpha" { + t.Fatalf("resolve should skip disabled subs; got %d packs", len(packs)) + } + + // missing from cache is an error + subs = []Subscription{{Name: "ghost", PinnedDigest: "deadbeef", Enabled: true}} + if _, err := Resolve(ctx, cache, subs); err == nil { + t.Fatal("expected error resolving an uncached pin") + } +} + +func TestStage(t *testing.T) { + dir := t.TempDir() + p := mustPack(t, "pdf", "b", map[string]string{"scripts/x.py": "print()"}) + staged, err := Stage(p, dir) + if err != nil { + t.Fatal(err) + } + if !strings.HasSuffix(staged, "/pdf") { + t.Errorf("staged dir = %q", staged) + } +} diff --git a/skillpack/manifest.go b/skillpack/manifest.go new file mode 100644 index 0000000..9216aab --- /dev/null +++ b/skillpack/manifest.go @@ -0,0 +1,192 @@ +package skillpack + +import ( + "bufio" + "bytes" + "fmt" + "strings" + + "gopkg.in/yaml.v3" +) + +// ManifestName is the required filename at a pack's root. +const ManifestName = "SKILL.md" + +// Limits on manifest fields, matching the Anthropic agent-skills constraints so +// packs authored against that ecosystem validate here unchanged. +const ( + maxNameLen = 64 + maxDescriptionLen = 1024 + maxBodyBytes = 1 << 20 // 1 MiB of instruction text is already excessive +) + +// Manifest is a parsed SKILL.md: YAML frontmatter plus the markdown body. Only +// Name and Description are required; everything else is optional and passes +// through so a host can honor it (or ignore it) without this package growing a +// policy opinion. +type Manifest struct { + // Name is the pack's stable identifier (kebab-case, unique within a host's + // subscriptions). It is what the model passes to skill_use. + Name string + // Description is the one-liner shown in the catalog — the ONLY text loaded + // into the prompt up front, so it must convey when to reach for the skill. + Description string + // License is an optional SPDX-ish tag, informational only. + License string + // AllowedTools is the pack author's declared tool allow-list. It is advisory + // here: a host MAY intersect it with the agent's real toolset, but it can + // only ever NARROW, never grant (see the host wiring, not this package). + AllowedTools []string + // Metadata is arbitrary passthrough frontmatter (e.g. version) the host may + // use; this package does not interpret it. + Metadata map[string]string + // Body is the markdown instruction text after the frontmatter — the payload + // skill_use returns on demand. + Body string +} + +// ParseManifest parses a SKILL.md byte slice into a validated Manifest. The +// input must begin with a `---` YAML frontmatter block; the remainder is the +// body. It returns a descriptive error on malformed frontmatter or a field that +// violates the limits, so a bad pack fails loudly at subscribe/sync time rather +// than silently activating. +func ParseManifest(raw []byte) (*Manifest, error) { + front, body, err := splitFrontmatter(raw) + if err != nil { + return nil, err + } + + // Decode into a permissive intermediate: SKILL.md uses hyphenated keys + // (allowed-tools) and lets metadata values be scalars of any type. + var fm struct { + Name string `yaml:"name"` + Description string `yaml:"description"` + License string `yaml:"license"` + AllowedTools stringList `yaml:"allowed-tools"` + Metadata map[string]any `yaml:"metadata"` + } + if err := yaml.Unmarshal(front, &fm); err != nil { + return nil, fmt.Errorf("skillpack: invalid SKILL.md frontmatter: %w", err) + } + + m := &Manifest{ + Name: strings.TrimSpace(fm.Name), + Description: strings.TrimSpace(fm.Description), + License: strings.TrimSpace(fm.License), + AllowedTools: []string(fm.AllowedTools), + Body: strings.TrimSpace(string(body)), + } + if len(fm.Metadata) > 0 { + m.Metadata = make(map[string]string, len(fm.Metadata)) + for k, v := range fm.Metadata { + m.Metadata[k] = fmt.Sprintf("%v", v) + } + } + + if err := m.Validate(); err != nil { + return nil, err + } + return m, nil +} + +// Validate reports the first field that violates the manifest contract. +func (m *Manifest) Validate() error { + switch { + case m.Name == "": + return fmt.Errorf("skillpack: SKILL.md missing required 'name'") + case len(m.Name) > maxNameLen: + return fmt.Errorf("skillpack: name %q exceeds %d chars", m.Name, maxNameLen) + case !isKebab(m.Name): + return fmt.Errorf("skillpack: name %q must be lowercase kebab-case (a-z, 0-9, -)", m.Name) + case m.Description == "": + return fmt.Errorf("skillpack: SKILL.md missing required 'description'") + case len(m.Description) > maxDescriptionLen: + return fmt.Errorf("skillpack: description exceeds %d chars", maxDescriptionLen) + case len(m.Body) > maxBodyBytes: + return fmt.Errorf("skillpack: body exceeds %d bytes", maxBodyBytes) + } + return nil +} + +// splitFrontmatter separates a leading `---`-delimited YAML block from the body. +// Leading blank lines/BOM are tolerated. A missing or unterminated block is an +// error — a SKILL.md without frontmatter has no name/description to catalog. +func splitFrontmatter(raw []byte) (front, body []byte, err error) { + s := bufio.NewScanner(bytes.NewReader(raw)) + s.Buffer(make([]byte, 0, 64*1024), maxBodyBytes+64*1024) + + var frontLines [][]byte + var bodyLines [][]byte + state := 0 // 0=before open fence, 1=in frontmatter, 2=in body + sawOpen := false + for s.Scan() { + line := s.Bytes() + trimmed := bytes.TrimRight(line, "\r") + switch state { + case 0: + if len(bytes.TrimSpace(trimmed)) == 0 { + continue // skip leading blanks + } + if string(bytes.TrimSpace(trimmed)) != "---" { + return nil, nil, fmt.Errorf("skillpack: SKILL.md must start with a '---' frontmatter block") + } + sawOpen = true + state = 1 + case 1: + if string(bytes.TrimSpace(trimmed)) == "---" { + state = 2 + continue + } + frontLines = append(frontLines, append([]byte(nil), trimmed...)) + case 2: + bodyLines = append(bodyLines, append([]byte(nil), trimmed...)) + } + } + if err := s.Err(); err != nil { + return nil, nil, fmt.Errorf("skillpack: reading SKILL.md: %w", err) + } + if !sawOpen || state != 2 { + return nil, nil, fmt.Errorf("skillpack: SKILL.md frontmatter block is not terminated by a closing '---'") + } + return bytes.Join(frontLines, []byte("\n")), bytes.Join(bodyLines, []byte("\n")), nil +} + +// stringList decodes either a YAML sequence or a comma-separated scalar into a +// []string, so `allowed-tools: [Read, Bash]` and `allowed-tools: "Read, Bash"` +// both work. +type stringList []string + +func (l *stringList) UnmarshalYAML(node *yaml.Node) error { + var seq []string + if err := node.Decode(&seq); err == nil { + *l = trimAll(seq) + return nil + } + var scalar string + if err := node.Decode(&scalar); err != nil { + return err + } + *l = trimAll(strings.Split(scalar, ",")) + return nil +} + +func trimAll(in []string) []string { + out := in[:0] + for _, s := range in { + if t := strings.TrimSpace(s); t != "" { + out = append(out, t) + } + } + return out +} + +func isKebab(s string) bool { + for _, r := range s { + switch { + case r >= 'a' && r <= 'z', r >= '0' && r <= '9', r == '-': + default: + return false + } + } + return s != "" +} diff --git a/skillpack/manifest_test.go b/skillpack/manifest_test.go new file mode 100644 index 0000000..9bbcc71 --- /dev/null +++ b/skillpack/manifest_test.go @@ -0,0 +1,82 @@ +package skillpack + +import ( + "strings" + "testing" +) + +const goodManifest = `--- +name: pdf-processing +description: Extract text and tables from PDF files and fill forms. +license: MIT +allowed-tools: [Read, Bash] +metadata: + version: 1.2.0 +--- +# PDF Processing + +Use pdfplumber for extraction. +` + +func TestParseManifest_Good(t *testing.T) { + m, err := ParseManifest([]byte(goodManifest)) + if err != nil { + t.Fatalf("ParseManifest: %v", err) + } + if m.Name != "pdf-processing" { + t.Errorf("name = %q", m.Name) + } + if !strings.HasPrefix(m.Description, "Extract text") { + t.Errorf("description = %q", m.Description) + } + if m.License != "MIT" { + t.Errorf("license = %q", m.License) + } + if len(m.AllowedTools) != 2 || m.AllowedTools[0] != "Read" || m.AllowedTools[1] != "Bash" { + t.Errorf("allowed-tools = %v", m.AllowedTools) + } + if m.Metadata["version"] != "1.2.0" { + t.Errorf("metadata version = %q", m.Metadata["version"]) + } + if !strings.Contains(m.Body, "pdfplumber") || strings.Contains(m.Body, "---") { + t.Errorf("body not cleanly extracted: %q", m.Body) + } +} + +func TestParseManifest_AllowedToolsScalar(t *testing.T) { + m, err := ParseManifest([]byte("---\nname: n\ndescription: d\nallowed-tools: \"Read, Bash , Grep\"\n---\nbody\n")) + if err != nil { + t.Fatal(err) + } + if len(m.AllowedTools) != 3 || m.AllowedTools[2] != "Grep" { + t.Errorf("scalar allowed-tools = %v", m.AllowedTools) + } +} + +func TestParseManifest_Errors(t *testing.T) { + cases := map[string]string{ + "no frontmatter": "# just a heading\n", + "unterminated": "---\nname: x\ndescription: y\n", + "missing name": "---\ndescription: y\n---\nb\n", + "missing desc": "---\nname: x\n---\nb\n", + "bad name uppercase": "---\nname: PdfProcessing\ndescription: d\n---\nb\n", + "bad name space": "---\nname: pdf processing\ndescription: d\n---\nb\n", + "bad yaml": "---\nname: [unclosed\n---\nb\n", + } + for label, in := range cases { + if _, err := ParseManifest([]byte(in)); err == nil { + t.Errorf("%s: expected error, got nil", label) + } + } +} + +func TestParseManifest_LeadingBlanksAndCRLF(t *testing.T) { + in := "\r\n\n---\r\nname: ok-name\r\ndescription: fine\r\n---\r\nbody line\r\n" + m, err := ParseManifest([]byte(in)) + if err != nil { + t.Fatalf("tolerant parse: %v", err) + } + if m.Name != "ok-name" || m.Body != "body line" { + t.Errorf("got name=%q body=%q", m.Name, m.Body) + } +} diff --git a/skillpack/memory.go b/skillpack/memory.go new file mode 100644 index 0000000..ce9c3dc --- /dev/null +++ b/skillpack/memory.go @@ -0,0 +1,114 @@ +package skillpack + +import ( + "context" + "sort" + "sync" +) + +// Memory is a zero-dependency in-process Store — a light host or a test gets +// subscription persistence with no DB. Returned values are copies, so callers +// can mutate them without corrupting the store. +type Memory struct { + mu sync.RWMutex + subs map[string]*Subscription // by ID +} + +// NewMemory returns an empty in-memory Store. +func NewMemory() *Memory { + return &Memory{subs: map[string]*Subscription{}} +} + +var _ Store = (*Memory)(nil) + +func (m *Memory) Initialize(context.Context) error { return nil } + +func (m *Memory) Save(_ context.Context, s *Subscription) error { + m.mu.Lock() + defer m.mu.Unlock() + cp := *s + m.subs[s.ID] = &cp + return nil +} + +func (m *Memory) Get(_ context.Context, id string) (*Subscription, error) { + m.mu.RLock() + defer m.mu.RUnlock() + s, ok := m.subs[id] + if !ok { + return nil, ErrNotFound + } + cp := *s + return &cp, nil +} + +func (m *Memory) GetByName(_ context.Context, name string) (*Subscription, error) { + m.mu.RLock() + defer m.mu.RUnlock() + for _, s := range m.subs { + if s.Name == name { + cp := *s + return &cp, nil + } + } + return nil, ErrNotFound +} + +func (m *Memory) List(context.Context) ([]Subscription, error) { + m.mu.RLock() + defer m.mu.RUnlock() + out := make([]Subscription, 0, len(m.subs)) + for _, s := range m.subs { + out = append(out, *s) + } + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + return out, nil +} + +func (m *Memory) Delete(_ context.Context, id string) error { + m.mu.Lock() + defer m.mu.Unlock() + delete(m.subs, id) + return nil +} + +// MemoryPackCache is a zero-dependency in-process PackCache. Trees are copied on +// the way in and out so a cached pin is immutable in practice. +type MemoryPackCache struct { + mu sync.RWMutex + trees map[string]Tree +} + +// NewMemoryPackCache returns an empty in-memory PackCache. +func NewMemoryPackCache() *MemoryPackCache { + return &MemoryPackCache{trees: map[string]Tree{}} +} + +var _ PackCache = (*MemoryPackCache)(nil) + +func (c *MemoryPackCache) Put(_ context.Context, digest string, t Tree) error { + c.mu.Lock() + defer c.mu.Unlock() + c.trees[digest] = cloneTree(t) + return nil +} + +func (c *MemoryPackCache) Get(_ context.Context, digest string) (Tree, error) { + c.mu.RLock() + defer c.mu.RUnlock() + t, ok := c.trees[digest] + if !ok { + return nil, ErrNotFound + } + return cloneTree(t), nil +} + +func cloneTree(t Tree) Tree { + cp := make(Tree, len(t)) + for k, v := range t { + b := make([]byte, len(v)) + copy(b, v) + cp[k] = b + } + return cp +} diff --git a/skillpack/pack.go b/skillpack/pack.go new file mode 100644 index 0000000..009e073 --- /dev/null +++ b/skillpack/pack.go @@ -0,0 +1,131 @@ +package skillpack + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "io/fs" + "os" + "path" + "path/filepath" + "sort" + "strings" +) + +// Tree is a pack's file set: relative slash-separated path -> file bytes, +// including the SKILL.md itself. It is self-contained (no live filesystem +// handle) so it can be cached, digested, and staged without worrying about the +// lifetime of a clone or temp dir. +type Tree map[string][]byte + +// Digest is the content address of the tree: a SHA-256 over every file's path +// and bytes, order-independent. Two trees with identical contents produce the +// same digest regardless of how they were fetched — this is the pin identity +// and the change-detection signal (a git SHA is provenance, but the digest is +// what says "the bytes an agent runs changed"). +func (t Tree) Digest() string { + paths := t.Paths() + h := sha256.New() + for _, p := range paths { + fh := sha256.Sum256(t[p]) + // path \x00 filehash \n — the NUL prevents path/content boundary games. + fmt.Fprintf(h, "%s\x00%s\n", p, hex.EncodeToString(fh[:])) + } + return hex.EncodeToString(h.Sum(nil)) +} + +// Paths returns the tree's file paths, sorted. +func (t Tree) Paths() []string { + out := make([]string, 0, len(t)) + for p := range t { + out = append(out, p) + } + sort.Strings(out) + return out +} + +// WriteTo materializes the tree under dir (creating it and any parents). It is +// how a host stages a pack's files for a sandbox; the host owns mount/read-only +// policy. Paths are cleaned and constrained to dir — a tree entry that escapes +// (via .. or an absolute path) is rejected rather than written outside dir. +func (t Tree) WriteTo(dir string) error { + for _, p := range t.Paths() { + dest := filepath.Join(dir, filepath.FromSlash(p)) + if !within(dir, dest) { + return fmt.Errorf("skillpack: refusing to stage %q outside %q", p, dir) + } + if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { + return err + } + if err := os.WriteFile(dest, t[p], 0o644); err != nil { + return err + } + } + return nil +} + +// Pack is a fetched, parsed pack: its manifest, its file tree, the tree's +// content digest, and the non-manifest ("bundled") file paths a host can stage. +type Pack struct { + Manifest *Manifest + Tree Tree + Digest string + // Bundled is every tree path except the SKILL.md, sorted — the scripts and + // reference files skill_use points the model at. + Bundled []string +} + +// LoadPack parses a fetched Tree into a Pack: it requires a root SKILL.md, +// parses+validates it, computes the digest, and lists the bundled files. +func LoadPack(t Tree) (*Pack, error) { + raw, ok := t[ManifestName] + if !ok { + return nil, ErrNoManifest + } + m, err := ParseManifest(raw) + if err != nil { + return nil, err + } + bundled := make([]string, 0, len(t)) + for _, p := range t.Paths() { + if p != ManifestName { + bundled = append(bundled, p) + } + } + return &Pack{Manifest: m, Tree: t, Digest: t.Digest(), Bundled: bundled}, nil +} + +// readTree reads an entire fs.FS (rooted at ".") into a Tree, skipping +// directories. It is the shared reader for DirSource and GitSource, so both +// produce identical self-contained trees. +func readTree(fsys fs.FS) (Tree, error) { + t := Tree{} + err := fs.WalkDir(fsys, ".", func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + return nil + } + b, err := fs.ReadFile(fsys, p) + if err != nil { + return err + } + t[path.Clean(p)] = b + return nil + }) + if err != nil { + return nil, err + } + return t, nil +} + +// within reports whether dest is inside dir (defense against path traversal in +// a staged tree). +func within(dir, dest string) bool { + rel, err := filepath.Rel(dir, dest) + if err != nil { + return false + } + return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} diff --git a/skillpack/pack_test.go b/skillpack/pack_test.go new file mode 100644 index 0000000..159d8a9 --- /dev/null +++ b/skillpack/pack_test.go @@ -0,0 +1,80 @@ +package skillpack + +import ( + "os" + "path/filepath" + "testing" +) + +func sampleTree() Tree { + return Tree{ + ManifestName: []byte(goodManifest), + "scripts/fill.py": []byte("print('hi')\n"), + "references/spec.md": []byte("# spec\n"), + } +} + +func TestTreeDigest_StableAndContentSensitive(t *testing.T) { + a := sampleTree() + b := sampleTree() + if a.Digest() != b.Digest() { + t.Fatal("identical trees must share a digest") + } + b["scripts/fill.py"] = []byte("print('bye')\n") + if a.Digest() == b.Digest() { + t.Fatal("content change must change the digest") + } + // Adding a file changes the digest. + c := sampleTree() + c["extra.txt"] = []byte("x") + if a.Digest() == c.Digest() { + t.Fatal("added file must change the digest") + } +} + +func TestLoadPack(t *testing.T) { + p, err := LoadPack(sampleTree()) + if err != nil { + t.Fatal(err) + } + if p.Manifest.Name != "pdf-processing" { + t.Errorf("name = %q", p.Manifest.Name) + } + if len(p.Bundled) != 2 || p.Bundled[0] != "references/spec.md" || p.Bundled[1] != "scripts/fill.py" { + t.Errorf("bundled = %v (want sorted, sans SKILL.md)", p.Bundled) + } + if p.Digest == "" { + t.Error("digest empty") + } +} + +func TestLoadPack_NoManifest(t *testing.T) { + if _, err := LoadPack(Tree{"readme.md": []byte("x")}); err != ErrNoManifest { + t.Fatalf("want ErrNoManifest, got %v", err) + } +} + +func TestTreeWriteTo(t *testing.T) { + dir := t.TempDir() + if err := sampleTree().WriteTo(dir); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(filepath.Join(dir, "scripts", "fill.py")) + if err != nil { + t.Fatal(err) + } + if string(got) != "print('hi')\n" { + t.Errorf("staged content = %q", got) + } +} + +func TestTreeWriteTo_RejectsTraversal(t *testing.T) { + dir := t.TempDir() + evil := Tree{"../escape.txt": []byte("nope")} + if err := evil.WriteTo(dir); err == nil { + t.Fatal("expected traversal rejection") + } + if _, err := os.Stat(filepath.Join(filepath.Dir(dir), "escape.txt")); err == nil { + t.Fatal("traversal file was written outside dir") + } +} diff --git a/skillpack/skillpack.go b/skillpack/skillpack.go new file mode 100644 index 0000000..de95694 --- /dev/null +++ b/skillpack/skillpack.go @@ -0,0 +1,54 @@ +// Package skillpack is the SKILL.md-subscription battery: it lets an agent host +// subscribe to skill packages published as directories/git repos in the +// Anthropic "agent skills" format (a SKILL.md manifest plus optional bundled +// scripts and reference files) and activate them for a run with progressive +// disclosure. +// +// It is a THIRD, distinct concept from the two "skill" nouns already in the +// stack — do not conflate them: +// +// - majordomo/skill — a lightweight capability bundle (instructions + tools) +// appended to an agent eagerly at construction. +// - executus/skill — a heavyweight persisted "saved agent" noun. +// - executus/skillpack (this package) — an externally-authored, versioned, +// on-demand-loaded instruction pack fetched from a Source and pinned by +// content digest. +// +// Progressive disclosure is the reason this is not just a majordomo/skill: +// majordomo skills inject their whole instruction text into the system prompt +// up front, which does not scale to a catalog of large third-party packs. Here +// only each pack's name+description sits in the prompt permanently (the +// Catalog); the full body is loaded lazily when the model calls the single +// skill_use tool (see Activate). +// +// Design shape (each piece is nil-safe / host-agnostic, mirroring the other +// executus batteries): +// +// - Manifest / ParseManifest — parse+validate a SKILL.md. +// - Tree / Pack / LoadPack — a fetched pack's files, content digest, and +// parsed manifest. +// - Source (Dir, Git) — where packs come from; Fetch returns the file +// tree and the source's resolved ref. +// - Subscription + Store — the persisted "this host tracks this pack at +// this pinned digest" record; Memory is the zero-dep default. +// - PackCache — content-addressed store of pinned pack trees +// so activation never re-fetches; Memory default. +// - Syncer — checks the tracked ref and records a PENDING +// update; applying it is an explicit, separate re-pin (supply-chain guard — +// upstream can never silently change what an agent runs). +// - Catalog / Activate / Stage — turn a set of resolved packs into a +// majordomo agent.Skill (catalog instructions + skill_use tool) and +// materialize a pack's files for a sandbox. +// +// The host (e.g. mort) supplies policy: which sources are allowed, who may +// subscribe, and where staged files are mounted. This package supplies only the +// mechanism. +package skillpack + +import "errors" + +// ErrNotFound is returned when a subscription or cached pack lookup misses. +var ErrNotFound = errors.New("skillpack: not found") + +// ErrNoManifest is returned when a fetched tree has no SKILL.md at its root. +var ErrNoManifest = errors.New("skillpack: tree has no SKILL.md") diff --git a/skillpack/source.go b/skillpack/source.go new file mode 100644 index 0000000..45f0878 --- /dev/null +++ b/skillpack/source.go @@ -0,0 +1,133 @@ +package skillpack + +import ( + "context" + "fmt" + "os" + "os/exec" + "path" + "path/filepath" + "strings" +) + +// Source is where a pack's files come from. Fetch retrieves the tree at ref and +// returns it together with the source's own resolved ref (a git commit SHA, or +// the content digest for a plain directory) — provenance a host can show and +// pin against. ref semantics are source-specific and may be empty ("the +// default": a dir's current contents, a repo's default branch). +type Source interface { + Fetch(ctx context.Context, ref string) (Tree, string, error) + // Kind is a short stable tag ("dir", "git") for persistence + display. + Kind() string + // String is a human-readable identifier (path or URL[/subpath]). + String() string +} + +// DirSource reads a pack from a local directory. ref is ignored (a directory +// has no versions); the resolved ref is the content digest. Useful for +// first-party/builtin packs shipped on disk and for tests. +type DirSource struct { + Path string +} + +func (d DirSource) Kind() string { return "dir" } +func (d DirSource) String() string { return d.Path } + +func (d DirSource) Fetch(_ context.Context, _ string) (Tree, string, error) { + info, err := os.Stat(d.Path) + if err != nil { + return nil, "", fmt.Errorf("skillpack: dir source %q: %w", d.Path, err) + } + if !info.IsDir() { + return nil, "", fmt.Errorf("skillpack: dir source %q is not a directory", d.Path) + } + t, err := readTree(os.DirFS(d.Path)) + if err != nil { + return nil, "", err + } + return t, t.Digest(), nil +} + +// GitSource fetches a pack from a git repository, optionally from a Subpath +// within it (for repos that publish several packs). ref is any git commit-ish +// (branch, tag, or SHA); empty means the default branch. The resolved ref is +// the checked-out commit SHA. +// +// Fetch clones into a temp dir, reads the subpath tree into memory, and removes +// the clone before returning — the returned Tree is self-contained, so there is +// no clone lifetime to manage and nothing left on disk. Git runs via the system +// `git`; GitRunner is overridable for tests. +type GitSource struct { + URL string + Subpath string + // GitRunner runs a git command in dir and returns combined output. Nil uses + // the system git. + GitRunner func(ctx context.Context, dir string, args ...string) ([]byte, error) +} + +func (g GitSource) Kind() string { return "git" } + +func (g GitSource) String() string { + if g.Subpath != "" { + return g.URL + "//" + g.Subpath + } + return g.URL +} + +func (g GitSource) run(ctx context.Context, dir string, args ...string) ([]byte, error) { + if g.GitRunner != nil { + return g.GitRunner(ctx, dir, args...) + } + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + return out, fmt.Errorf("skillpack: git %s: %w: %s", strings.Join(args, " "), err, strings.TrimSpace(string(out))) + } + return out, nil +} + +func (g GitSource) Fetch(ctx context.Context, ref string) (Tree, string, error) { + tmp, err := os.MkdirTemp("", "skillpack-git-*") + if err != nil { + return nil, "", err + } + defer os.RemoveAll(tmp) + + if _, err := g.run(ctx, "", "clone", "--quiet", g.URL, tmp); err != nil { + return nil, "", err + } + if ref != "" { + if _, err := g.run(ctx, tmp, "checkout", "--quiet", ref); err != nil { + return nil, "", err + } + } + shaOut, err := g.run(ctx, tmp, "rev-parse", "HEAD") + if err != nil { + return nil, "", err + } + sha := strings.TrimSpace(string(shaOut)) + + root := tmp + if g.Subpath != "" { + clean := path.Clean("/" + g.Subpath) // normalize, strip leading ../ + root = filepath.Join(tmp, filepath.FromSlash(strings.TrimPrefix(clean, "/"))) + if !within(tmp, root) { + return nil, "", fmt.Errorf("skillpack: subpath %q escapes the repo", g.Subpath) + } + if info, err := os.Stat(root); err != nil || !info.IsDir() { + return nil, "", fmt.Errorf("skillpack: subpath %q not found in %s", g.Subpath, g.URL) + } + } + t, err := readTree(os.DirFS(root)) + if err != nil { + return nil, "", err + } + // Drop a nested .git if the subpath was the repo root. + for p := range t { + if p == ".git" || strings.HasPrefix(p, ".git/") { + delete(t, p) + } + } + return t, sha, nil +} diff --git a/skillpack/source_test.go b/skillpack/source_test.go new file mode 100644 index 0000000..59b7899 --- /dev/null +++ b/skillpack/source_test.go @@ -0,0 +1,97 @@ +package skillpack + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "testing" +) + +func writePack(t *testing.T, dir string) { + t.Helper() + if err := os.MkdirAll(filepath.Join(dir, "scripts"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, ManifestName), []byte(goodManifest), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "scripts", "fill.py"), []byte("print('hi')\n"), 0o644); err != nil { + t.Fatal(err) + } +} + +func TestDirSource(t *testing.T) { + dir := t.TempDir() + writePack(t, dir) + + tree, ref, err := DirSource{Path: dir}.Fetch(context.Background(), "") + if err != nil { + t.Fatal(err) + } + if ref != tree.Digest() { + t.Errorf("dir resolved ref should be the content digest") + } + p, err := LoadPack(tree) + if err != nil { + t.Fatal(err) + } + if p.Manifest.Name != "pdf-processing" || len(p.Bundled) != 1 { + t.Errorf("loaded pack wrong: name=%q bundled=%v", p.Manifest.Name, p.Bundled) + } +} + +func TestDirSource_NotADir(t *testing.T) { + f := filepath.Join(t.TempDir(), "file") + os.WriteFile(f, []byte("x"), 0o644) + if _, _, err := (DirSource{Path: f}).Fetch(context.Background(), ""); err == nil { + t.Fatal("expected error for non-directory source") + } +} + +// TestGitSource drives a real local git repo (no network) to exercise clone + +// checkout + subpath + SHA resolution. Skipped when git is unavailable. +func TestGitSource(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + repo := t.TempDir() + git := func(args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = repo + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@t", "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@t") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + } + git("init", "-q", "-b", "main") + // pack lives under packs/pdf/ + sub := filepath.Join(repo, "packs", "pdf") + writePack(t, sub) + git("add", "-A") + git("commit", "-q", "-m", "add pack") + + src := GitSource{URL: repo, Subpath: "packs/pdf"} + tree, sha, err := src.Fetch(context.Background(), "main") + if err != nil { + t.Fatal(err) + } + if len(sha) != 40 { + t.Errorf("resolved ref should be a full SHA, got %q", sha) + } + if _, ok := tree[ManifestName]; !ok { + t.Errorf("subpath tree missing SKILL.md; got %v", tree.Paths()) + } + if _, ok := tree[".git"]; ok { + t.Error(".git leaked into the tree") + } + p, err := LoadPack(tree) + if err != nil { + t.Fatal(err) + } + if p.Manifest.Name != "pdf-processing" { + t.Errorf("name = %q", p.Manifest.Name) + } +} diff --git a/skillpack/store.go b/skillpack/store.go new file mode 100644 index 0000000..e8e047f --- /dev/null +++ b/skillpack/store.go @@ -0,0 +1,27 @@ +package skillpack + +import "context" + +// Store is the persistence seam for subscriptions (metadata + the current pin). +// It is deliberately small; a host backs it with its DB, Memory is the zero-dep +// default, and contrib/store can add durable SQLite alongside the other +// executus store impls. +type Store interface { + Initialize(ctx context.Context) error + Save(ctx context.Context, s *Subscription) error + Get(ctx context.Context, id string) (*Subscription, error) + GetByName(ctx context.Context, name string) (*Subscription, error) + List(ctx context.Context) ([]Subscription, error) + Delete(ctx context.Context, id string) error +} + +// PackCache is the content-addressed store of pinned pack trees, keyed by +// content digest. It exists so activating an agent never re-fetches from the +// Source (no clone per run) and so a pinned digest's exact bytes survive even if +// upstream later force-pushes or disappears. A host may back it with disk; +// Memory is the default. Because the key IS the content digest, entries are +// immutable and safe to share across subscriptions that pin the same bytes. +type PackCache interface { + Put(ctx context.Context, digest string, t Tree) error + Get(ctx context.Context, digest string) (Tree, error) +} diff --git a/skillpack/subscription.go b/skillpack/subscription.go new file mode 100644 index 0000000..46ec7f8 --- /dev/null +++ b/skillpack/subscription.go @@ -0,0 +1,64 @@ +package skillpack + +import "time" + +// Subscription is a host's persisted "I track this pack, pinned here" record. It +// is metadata only — the pinned pack's bytes live in a PackCache keyed by +// PinnedDigest. A subscription is only ever advanced to new content by an +// explicit Apply (see Syncer): a sync records a PendingDigest, it never moves +// the pin. That is the supply-chain guard — a compromised or careless upstream +// cannot change what an agent runs without a human re-pin. +type Subscription struct { + // ID is a stable host-assigned identifier. + ID string + // Name is the pack's manifest name (unique per host); what an agent lists in + // its SkillPacks and what skill_use receives. + Name string + // Description is the pinned manifest's description, cached so the catalog + // renders without opening the PackCache. + Description string + + // Source coordinates. + SourceKind string // "dir" | "git" + SourceURL string // dir path or git URL + Subpath string // git subpath, if any + // TrackRef is the git commit-ish the host follows (branch/tag/SHA); empty = + // default branch. Sync fetches THIS; the pin only moves on Apply. + TrackRef string + + // Pinned* describe the currently-active content. + PinnedDigest string // content digest = PackCache key + change signal + PinnedSourceRef string // source's resolved ref (git SHA) — provenance + PinnedAt time.Time + PinnedBy string + + // Pending* describe an update a sync found but has NOT applied. Empty + // PendingDigest = no pending update. A pending digest equal to the pinned + // one is impossible by construction (Syncer clears it). + PendingDigest string + PendingSourceRef string + PendingAt time.Time + + // Enabled lets a host keep a subscription but deactivate it without + // deleting the pin/history. + Enabled bool +} + +// HasPending reports whether a sync found an unapplied update. +func (s *Subscription) HasPending() bool { + return s.PendingDigest != "" && s.PendingDigest != s.PinnedDigest +} + +// pinTo advances the active pin to a fetched pack and clears any pending state. +// Used by initial pin and by Apply. +func (s *Subscription) pinTo(p *Pack, sourceRef, by string, now time.Time) { + s.Name = p.Manifest.Name + s.Description = p.Manifest.Description + s.PinnedDigest = p.Digest + s.PinnedSourceRef = sourceRef + s.PinnedAt = now + s.PinnedBy = by + s.PendingDigest = "" + s.PendingSourceRef = "" + s.PendingAt = time.Time{} +} diff --git a/skillpack/sync.go b/skillpack/sync.go new file mode 100644 index 0000000..89a9c70 --- /dev/null +++ b/skillpack/sync.go @@ -0,0 +1,191 @@ +package skillpack + +import ( + "context" + "fmt" + "time" + + "github.com/google/uuid" +) + +// Syncer ties a Store, a PackCache, and Sources together into the subscription +// lifecycle: subscribe (initial pin), check (record a PENDING update, never move +// the pin), and apply (the explicit re-pin). It owns the supply-chain invariant +// — the only call that changes the bytes an agent runs is Apply, always with an +// actor recorded. +type Syncer struct { + Store PackCache // content store for pinned trees + Subs Store // subscription metadata store + + // SourceFor builds the Source for a stored subscription. A host overrides + // this to enforce its allow-list (reject a disallowed URL/kind before any + // fetch). Nil uses DefaultSourceFor (dir + git, no allow-list). + SourceFor func(*Subscription) (Source, error) + + // Now/NewID are injectable for deterministic tests. + Now func() time.Time + NewID func() string +} + +func (y *Syncer) now() time.Time { + if y.Now != nil { + return y.Now() + } + return time.Now() +} + +func (y *Syncer) newID() string { + if y.NewID != nil { + return y.NewID() + } + return uuid.NewString() +} + +func (y *Syncer) sourceFor(s *Subscription) (Source, error) { + if y.SourceFor != nil { + return y.SourceFor(s) + } + return DefaultSourceFor(s) +} + +// DefaultSourceFor reconstructs a Source from a subscription's stored +// coordinates, with no allow-list. A host that cares about which sources are +// permitted should set Syncer.SourceFor instead of using this. +func DefaultSourceFor(s *Subscription) (Source, error) { + switch s.SourceKind { + case "dir": + return DirSource{Path: s.SourceURL}, nil + case "git": + return GitSource{URL: s.SourceURL, Subpath: s.Subpath}, nil + default: + return nil, fmt.Errorf("skillpack: unknown source kind %q", s.SourceKind) + } +} + +// fetchPack fetches src at ref, caches the resulting tree, and returns the +// parsed pack plus the source's resolved ref. +func (y *Syncer) fetchPack(ctx context.Context, src Source, ref string) (*Pack, string, error) { + tree, sourceRef, err := src.Fetch(ctx, ref) + if err != nil { + return nil, "", err + } + pack, err := LoadPack(tree) + if err != nil { + return nil, "", err + } + if err := y.Store.Put(ctx, pack.Digest, pack.Tree); err != nil { + return nil, "", err + } + return pack, sourceRef, nil +} + +// Subscribe fetches a pack from src at trackRef, caches it, and persists a new +// Subscription pinned to that exact content, attributed to by. It rejects a +// second subscription to the same pack name. +func (y *Syncer) Subscribe(ctx context.Context, src Source, trackRef, by string) (*Subscription, error) { + pack, sourceRef, err := y.fetchPack(ctx, src, trackRef) + if err != nil { + return nil, err + } + if existing, err := y.Subs.GetByName(ctx, pack.Manifest.Name); err == nil { + return nil, fmt.Errorf("skillpack: already subscribed to %q (id %s)", pack.Manifest.Name, existing.ID) + } + + sub := &Subscription{ + ID: y.newID(), + SourceKind: src.Kind(), + SourceURL: src.String(), + TrackRef: trackRef, + Enabled: true, + } + // Store the raw URL/subpath (String() may combine them for display). + if gs, ok := src.(GitSource); ok { + sub.SourceURL = gs.URL + sub.Subpath = gs.Subpath + } + sub.pinTo(pack, sourceRef, by, y.now()) + if err := y.Subs.Save(ctx, sub); err != nil { + return nil, err + } + return sub, nil +} + +// Check fetches the subscription's tracked ref and, if the content digest +// differs from the current pin, caches the new tree and records it as PENDING — +// it never moves the pin. If the tracked ref matches the pin, any stale pending +// state is cleared. The updated subscription is saved and returned. +func (y *Syncer) Check(ctx context.Context, id string) (*Subscription, error) { + sub, err := y.Subs.Get(ctx, id) + if err != nil { + return nil, err + } + src, err := y.sourceFor(sub) + if err != nil { + return nil, err + } + pack, sourceRef, err := y.fetchPack(ctx, src, sub.TrackRef) + if err != nil { + return nil, err + } + if pack.Digest == sub.PinnedDigest { + // No change upstream; drop any previously-recorded pending update. + sub.PendingDigest, sub.PendingSourceRef, sub.PendingAt = "", "", time.Time{} + } else { + sub.PendingDigest = pack.Digest + sub.PendingSourceRef = sourceRef + sub.PendingAt = y.now() + } + if err := y.Subs.Save(ctx, sub); err != nil { + return nil, err + } + return sub, nil +} + +// CheckAll runs Check on every subscription and returns the ones that now have a +// pending update. Errors on individual subscriptions are collected, not fatal — +// one unreachable source shouldn't stop the sweep. A host calls this on its own +// ticker (skillpack has no cron opinion; the update is never auto-applied so the +// cadence only affects how fresh the "pending" signal is). +func (y *Syncer) CheckAll(ctx context.Context) (pending []Subscription, errs []error) { + subs, err := y.Subs.List(ctx) + if err != nil { + return nil, []error{err} + } + for i := range subs { + updated, err := y.Check(ctx, subs[i].ID) + if err != nil { + errs = append(errs, fmt.Errorf("skillpack: check %q: %w", subs[i].Name, err)) + continue + } + if updated.HasPending() { + pending = append(pending, *updated) + } + } + return pending, errs +} + +// Apply promotes a subscription's pending update to the active pin, attributed +// to by. This is the ONLY call that changes what an agent runs. It errors if +// there is no pending update or the pending tree is missing from the cache. +func (y *Syncer) Apply(ctx context.Context, id, by string) (*Subscription, error) { + sub, err := y.Subs.Get(ctx, id) + if err != nil { + return nil, err + } + if !sub.HasPending() { + return nil, fmt.Errorf("skillpack: %q has no pending update to apply", sub.Name) + } + tree, err := y.Store.Get(ctx, sub.PendingDigest) + if err != nil { + return nil, fmt.Errorf("skillpack: pending tree for %q missing from cache: %w", sub.Name, err) + } + pack, err := LoadPack(tree) + if err != nil { + return nil, err + } + sub.pinTo(pack, sub.PendingSourceRef, by, y.now()) + if err := y.Subs.Save(ctx, sub); err != nil { + return nil, err + } + return sub, nil +} diff --git a/skillpack/sync_test.go b/skillpack/sync_test.go new file mode 100644 index 0000000..ce74133 --- /dev/null +++ b/skillpack/sync_test.go @@ -0,0 +1,176 @@ +package skillpack + +import ( + "context" + "testing" + "time" +) + +// fakeSource returns a caller-controlled tree, so sync behavior is tested with +// no filesystem or git. +type fakeSource struct { + tree Tree + ref string + err error +} + +func (f *fakeSource) Fetch(context.Context, string) (Tree, string, error) { + return f.tree, f.ref, f.err +} +func (f *fakeSource) Kind() string { return "fake" } +func (f *fakeSource) String() string { return "fake://pack" } + +func packTree(name, body string) Tree { + return Tree{ManifestName: []byte("---\nname: " + name + "\ndescription: does " + name + "\n---\n" + body + "\n")} +} + +func newTestSyncer(src *fakeSource) *Syncer { + n := 0 + return &Syncer{ + Store: NewMemoryPackCache(), + Subs: NewMemory(), + Now: func() time.Time { return time.Unix(1000, 0) }, + NewID: func() string { n++; return "id-1" }, + SourceFor: func(*Subscription) (Source, error) { return src, nil }, + } +} + +func TestSubscribeAndPin(t *testing.T) { + ctx := context.Background() + src := &fakeSource{tree: packTree("alpha", "v1"), ref: "sha-v1"} + y := newTestSyncer(src) + + sub, err := y.Subscribe(ctx, src, "main", "steve") + if err != nil { + t.Fatal(err) + } + if sub.Name != "alpha" || sub.PinnedSourceRef != "sha-v1" || sub.PinnedBy != "steve" { + t.Fatalf("bad pin: %+v", sub) + } + if sub.HasPending() { + t.Fatal("fresh subscription should have no pending update") + } + // pinned tree is cached under its digest + if _, err := y.Store.Get(ctx, sub.PinnedDigest); err != nil { + t.Fatalf("pinned tree not cached: %v", err) + } +} + +func TestSubscribe_DuplicateName(t *testing.T) { + ctx := context.Background() + src := &fakeSource{tree: packTree("alpha", "v1"), ref: "r"} + y := newTestSyncer(src) + if _, err := y.Subscribe(ctx, src, "", "s"); err != nil { + t.Fatal(err) + } + if _, err := y.Subscribe(ctx, src, "", "s"); err == nil { + t.Fatal("expected duplicate-name error") + } +} + +func TestCheck_RecordsPendingButDoesNotMovePin(t *testing.T) { + ctx := context.Background() + src := &fakeSource{tree: packTree("alpha", "v1"), ref: "sha-v1"} + y := newTestSyncer(src) + sub, _ := y.Subscribe(ctx, src, "main", "s") + pinnedBefore := sub.PinnedDigest + + // upstream changes + src.tree = packTree("alpha", "v2-new-instructions") + src.ref = "sha-v2" + + updated, err := y.Check(ctx, sub.ID) + if err != nil { + t.Fatal(err) + } + if !updated.HasPending() { + t.Fatal("expected a pending update after upstream change") + } + if updated.PinnedDigest != pinnedBefore { + t.Fatal("Check must NOT move the pin — that is the supply-chain guard") + } + if updated.PendingSourceRef != "sha-v2" { + t.Errorf("pending ref = %q", updated.PendingSourceRef) + } + // the pending tree is cached, ready for Apply + if _, err := y.Store.Get(ctx, updated.PendingDigest); err != nil { + t.Fatalf("pending tree not cached: %v", err) + } +} + +func TestCheck_ClearsStalePendingWhenUpstreamMatches(t *testing.T) { + ctx := context.Background() + src := &fakeSource{tree: packTree("alpha", "v1"), ref: "r1"} + y := newTestSyncer(src) + sub, _ := y.Subscribe(ctx, src, "main", "s") + + src.tree = packTree("alpha", "v2") + src.ref = "r2" + sub, _ = y.Check(ctx, sub.ID) // records pending + if !sub.HasPending() { + t.Fatal("precondition: pending expected") + } + // upstream reverts to the pinned content + src.tree = packTree("alpha", "v1") + src.ref = "r1" + sub, _ = y.Check(ctx, sub.ID) + if sub.HasPending() { + t.Fatal("pending should be cleared once upstream matches the pin again") + } +} + +func TestApply_MovesPinAndClearsPending(t *testing.T) { + ctx := context.Background() + src := &fakeSource{tree: packTree("alpha", "v1"), ref: "sha-v1"} + y := newTestSyncer(src) + sub, _ := y.Subscribe(ctx, src, "main", "s") + + src.tree = packTree("alpha", "v2") + src.ref = "sha-v2" + sub, _ = y.Check(ctx, sub.ID) + pendingDigest := sub.PendingDigest + + applied, err := y.Apply(ctx, sub.ID, "admin") + if err != nil { + t.Fatal(err) + } + if applied.PinnedDigest != pendingDigest { + t.Fatal("Apply must move the pin to the pending digest") + } + if applied.PinnedSourceRef != "sha-v2" || applied.PinnedBy != "admin" { + t.Errorf("bad post-apply pin: %+v", applied) + } + if applied.HasPending() { + t.Fatal("Apply must clear the pending update") + } +} + +func TestApply_NoPending(t *testing.T) { + ctx := context.Background() + src := &fakeSource{tree: packTree("alpha", "v1"), ref: "r"} + y := newTestSyncer(src) + sub, _ := y.Subscribe(ctx, src, "", "s") + if _, err := y.Apply(ctx, sub.ID, "admin"); err == nil { + t.Fatal("expected error applying with no pending update") + } +} + +func TestCheckAll(t *testing.T) { + ctx := context.Background() + src := &fakeSource{tree: packTree("alpha", "v1"), ref: "r1"} + y := newTestSyncer(src) + sub, _ := y.Subscribe(ctx, src, "main", "s") + + if pend, errs := y.CheckAll(ctx); len(pend) != 0 || len(errs) != 0 { + t.Fatalf("no change: pend=%v errs=%v", pend, errs) + } + src.tree = packTree("alpha", "v2") + src.ref = "r2" + pend, errs := y.CheckAll(ctx) + if len(errs) != 0 { + t.Fatalf("errs: %v", errs) + } + if len(pend) != 1 || pend[0].ID != sub.ID { + t.Fatalf("expected 1 pending, got %v", pend) + } +}