Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 19 additions & 9 deletions cmd/ax/internal/cliutil/cliutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ import (
"github.com/google/ax/internal/harness/antigravity"
"github.com/google/ax/internal/harness/antigravityinteractions"
"github.com/google/ax/internal/harness/substrate"
"github.com/google/ax/internal/skills"
"github.com/google/ax/internal/skills/geminienterprise"
"github.com/google/ax/internal/skills/local"
)

// Controller is the active controller type for this build.
Expand Down Expand Up @@ -65,20 +67,28 @@ func NewControllerFromConfig(ctx context.Context, cfg *Config) (*controller.Cont
var defaultHarnessID string
var err error

// Materialize registry skills once, up front (skills config is top-level and
// Resolve skills once, up front (skills config is top-level and
// harness-agnostic; each actor runs a single harness that consumes the
// materialized folder). Unconditional when configured. Fail-safe: a registry
// error degrades capability but never blocks harness creation. The
// interactions harness (no SKILLS_DIR concept) is told where the materialized
// skills are via a pointer appended to its system instruction. Only the local
// path materializes; substrate/pod does not yet read ax.yaml.
// resulting folder). Two parallel sources feed the same result:
// - Registries: fetched from the Gemini Enterprise Skill Registry and
// materialized to their target_dir.
// - Local: directories already on disk, used as-is.
// Both are unconditional when configured and fail-safe: a source error
// degrades capability but never blocks harness creation. The interactions
// harness (no SKILLS_DIR concept) is told where the skills are via a pointer
// appended to its system instruction. Only the local process path resolves
// skills; substrate/pod does not yet read ax.yaml.
//
// TODO(joycel): wire the Antigravity SDK harness too. It discovers skills via
// SKILLS_DIR, so its SKILLS_DIR needs to be pointed at the materialized
// target_dir; currently only the interactions harness is fully wired.
// SKILLS_DIR, so its SKILLS_DIR needs to be pointed at the resolved skill
// directories (registry target_dir(s) and/or local path(s)); currently only
// the interactions harness is fully wired.
var skillsPointer string
if !substrateMode {
skillsPointer = antigravityinteractions.SkillsSystemInstruction(geminienterprise.Materialize(ctx, cfg.Skills))
var avail skills.Available
avail.Groups = append(avail.Groups, geminienterprise.Materialize(ctx, cfg.Skills).Groups...)
avail.Groups = append(avail.Groups, local.Discover(cfg.Skills).Groups...)
skillsPointer = antigravityinteractions.SkillsSystemInstruction(avail)
}

// Built-in Antigravity harness.
Expand Down
53 changes: 46 additions & 7 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"

"github.com/google/ax/internal/harness"
"github.com/google/ax/internal/harness/substrate"
Expand Down Expand Up @@ -48,10 +49,10 @@ type Config struct {
Server ServerConfig `yaml:"server"`
EventLog EventLogConfig `yaml:"eventlog"`
Harnesses HarnessesConfig `yaml:"harnesses,omitempty"`
// Skills sources skills from the Gemini Enterprise Skill Registry into on-disk folders
// before the harness starts. It is harness-agnostic: each actor runs exactly
// one harness, which consumes the materialized folder(s). Optional; disabled
// when no registry is enabled.
// Skills makes agent skills available on disk before the harness starts. It
// is harness-agnostic: each actor runs exactly one harness, which consumes
// the resulting folder(s). Optional; disabled when no source is enabled. See
// SkillsConfig for the available sources.
Skills SkillsConfig `yaml:"skills,omitempty"`
Telemetry TelemetryConfig `yaml:"telemetry,omitempty"`
}
Expand Down Expand Up @@ -115,11 +116,19 @@ type AntigravityInteractionsHarnessConfig struct {
}

// SkillsConfig configures optional skill sources (top-level, harness-agnostic).
// Today the only source type is the Gemini Enterprise Skill Registry; it may source from more
// than one registry (e.g. a shared org-wide registry plus a team-specific one),
// each with its own project/location, selection, and target directory.
// There are two parallel source types:
// - Registries source skills from the Gemini Enterprise Skill Registry,
// materializing them into a target directory.
// - Local points at directories that already contain skills on disk (each a
// parent dir holding <skill-id>/SKILL.md subfolders, matching the layout of
// examples/skills and a registry's target_dir). Nothing is fetched; the
// directory is used as-is.
//
// Both may be configured together, and both feed the same downstream consumers
// (e.g. a harness system-instruction pointer).
type SkillsConfig struct {
Registries []SkillsRegistryConfig `yaml:"registries,omitempty"`
Local []LocalSkillsConfig `yaml:"local,omitempty"`
}

// Validate checks the (top-level) skills config.
Expand All @@ -129,6 +138,36 @@ func (s SkillsConfig) Validate() error {
return err
}
}
for i := range s.Local {
if err := s.Local[i].validate(i); err != nil {
return err
}
}
return nil
}

// LocalSkillsConfig sources skills from a directory already present on disk. The
// path is a parent directory containing one subfolder per skill (each with a
// SKILL.md); the subfolder name is the skill id. This mirrors the layout of
// examples/skills and a registry's target_dir, so nothing is downloaded.
type LocalSkillsConfig struct {
Enabled bool `yaml:"enabled,omitempty"`
// Path is the parent directory holding <skill-id>/SKILL.md subfolders.
// Required when Enabled.
Path string `yaml:"path,omitempty"`
}

// validate checks one local skills source. idx is its index within the local
// list, for error context. Existence/readability of the path is intentionally
// not checked here: discovery is fail-safe (a bad path degrades capability
// rather than blocking harness creation), so it is validated at discovery time.
func (lc LocalSkillsConfig) validate(idx int) error {
if !lc.Enabled {
return nil
}
if strings.TrimSpace(lc.Path) == "" {
return fmt.Errorf("skills.local[%d] requires path (a directory containing <skill-id>/SKILL.md subfolders)", idx)
}
return nil
}

Expand Down
48 changes: 48 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,3 +345,51 @@ func TestValidate_SkillsSelectionOneof(t *testing.T) {
}
})
}

func TestValidate_LocalSkills(t *testing.T) {
withLocal := func(lcs ...LocalSkillsConfig) *Config {
c := validConfig()
c.Skills.Local = lcs
return c
}

t.Run("disabled skips validation", func(t *testing.T) {
if err := withLocal(LocalSkillsConfig{Enabled: false}).Validate(); err != nil {
t.Fatalf("Validate() = %v, want nil", err)
}
})

t.Run("enabled without path is an error", func(t *testing.T) {
err := withLocal(LocalSkillsConfig{Enabled: true}).Validate()
if err == nil || !strings.Contains(err.Error(), "path") {
t.Fatalf("Validate() = %v, want path error", err)
}
})

t.Run("enabled with path is valid", func(t *testing.T) {
if err := withLocal(LocalSkillsConfig{Enabled: true, Path: "./examples/skills"}).Validate(); err != nil {
t.Fatalf("Validate() = %v, want nil", err)
}
})

t.Run("second local invalid is caught", func(t *testing.T) {
err := withLocal(
LocalSkillsConfig{Enabled: true, Path: "/tmp/a"},
LocalSkillsConfig{Enabled: true}, // no path
).Validate()
if err == nil || !strings.Contains(err.Error(), "local[1]") {
t.Fatalf("Validate() = %v, want error citing local[1]", err)
}
})

t.Run("registries and local can coexist", func(t *testing.T) {
c := validConfig()
c.Skills.Registries = []SkillsRegistryConfig{
{Enabled: true, Project: "p", TargetDir: "/tmp/a", All: true},
}
c.Skills.Local = []LocalSkillsConfig{{Enabled: true, Path: "/tmp/local"}}
if err := c.Validate(); err != nil {
t.Fatalf("Validate() = %v, want nil", err)
}
})
}
18 changes: 10 additions & 8 deletions internal/harness/antigravityinteractions/skills.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,31 @@ import (
"fmt"
"strings"

"github.com/google/ax/internal/skills/geminienterprise"
"github.com/google/ax/internal/skills"
)

// SkillsSystemInstruction builds a system-instruction pointer telling the agent
// where its materialized skills live and lists them.
// where its skills live and lists them. It is source-agnostic: the skills may
// come from the Gemini Enterprise Skill Registry or from a local directory --
// both produce the same skills.Available shape.
//
// This is discovery logic specific to the Antigravity Interactions harness: it
// has no SKILLS_DIR concept, so it must be told where to find skills via its
// system instruction (its built-in file tools then read that directory).
// Harnesses that auto-discover a skills directory (e.g. the Antigravity SDK
// harness via SKILLS_DIR) do not use this.
func SkillsSystemInstruction(res geminienterprise.Result) string {
if res.Empty() {
func SkillsSystemInstruction(avail skills.Available) string {
if avail.Empty() {
return ""
}
var b strings.Builder
for _, w := range res.Written {
for _, g := range avail.Groups {
if b.Len() > 0 {
b.WriteString("\n")
}
fmt.Fprintf(&b, "Agent skills are available under %s. Available skills:", w.Dir)
for _, s := range w.Skills {
fmt.Fprintf(&b, " %s", s.SkillID)
fmt.Fprintf(&b, "Agent skills are available under %s. Available skills:", g.Dir)
for _, s := range g.Skills {
fmt.Fprintf(&b, " %s", s.ID)
}
b.WriteString(". Read a skill's SKILL.md before using it.")
}
Expand Down
20 changes: 10 additions & 10 deletions internal/harness/antigravityinteractions/skills_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,33 @@ import (
"strings"
"testing"

"github.com/google/ax/internal/skills/geminienterprise"
"github.com/google/ax/internal/skills"
)

func TestSkillsSystemInstruction(t *testing.T) {
t.Run("empty result yields empty pointer", func(t *testing.T) {
if got := SkillsSystemInstruction(geminienterprise.Result{}); got != "" {
if got := SkillsSystemInstruction(skills.Available{}); got != "" {
t.Errorf("empty result pointer = %q, want empty", got)
}
})

t.Run("mentions dir and skill ids", func(t *testing.T) {
res := geminienterprise.Result{Written: []geminienterprise.Written{{
avail := skills.Available{Groups: []skills.Group{{
Dir: "/workspace",
Skills: []geminienterprise.MaterializedSkill{{SkillID: "emoji"}, {SkillID: "lowercase"}},
Skills: []skills.Skill{{ID: "emoji"}, {ID: "lowercase"}},
}}}
got := SkillsSystemInstruction(res)
got := SkillsSystemInstruction(avail)
if !strings.Contains(got, "/workspace") || !strings.Contains(got, "emoji") || !strings.Contains(got, "lowercase") {
t.Errorf("pointer = %q, want it to mention dir and skill ids", got)
}
})

t.Run("multiple registries produce multiple lines", func(t *testing.T) {
res := geminienterprise.Result{Written: []geminienterprise.Written{
{Dir: "/a", Skills: []geminienterprise.MaterializedSkill{{SkillID: "s1"}}},
{Dir: "/b", Skills: []geminienterprise.MaterializedSkill{{SkillID: "s2"}}},
t.Run("multiple groups produce multiple lines", func(t *testing.T) {
avail := skills.Available{Groups: []skills.Group{
{Dir: "/a", Skills: []skills.Skill{{ID: "s1"}}},
{Dir: "/b", Skills: []skills.Skill{{ID: "s2"}}},
}}
got := SkillsSystemInstruction(res)
got := SkillsSystemInstruction(avail)
if !strings.Contains(got, "/a") || !strings.Contains(got, "/b") || !strings.Contains(got, "\n") {
t.Errorf("pointer = %q, want both dirs on separate lines", got)
}
Expand Down
6 changes: 3 additions & 3 deletions internal/skills/geminienterprise/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ type skillRef struct {
// fetchResult reports the outcome of a fetch call. fetch is fail-safe: a skill
// that cannot be fetched or unzipped is recorded in skipped.
type fetchResult struct {
materialized []MaterializedSkill
materialized []materializedSkill
skipped []skippedSkill
}

Expand Down Expand Up @@ -215,7 +215,7 @@ func (c *client) resolveSelection(ctx context.Context, sel selection) ([]skillRe
}

// fetchAndWrite fetches one skill's payload (latest or pinned) and unzips it.
func (c *client) fetchAndWrite(ctx context.Context, ref skillRef, targetDir string) (*MaterializedSkill, error) {
func (c *client) fetchAndWrite(ctx context.Context, ref skillRef, targetDir string) (*materializedSkill, error) {
if ref.SkillID == "" {
return nil, errors.New("skill ref has empty SkillID")
}
Expand All @@ -237,7 +237,7 @@ func (c *client) fetchAndWrite(ctx context.Context, ref skillRef, targetDir stri
_ = os.RemoveAll(skillDir)
return nil, fmt.Errorf("unzipping: %w", err)
}
return &MaterializedSkill{SkillID: ref.SkillID, Revision: revision, Dir: skillDir}, nil
return &materializedSkill{SkillID: ref.SkillID, Revision: revision, Dir: skillDir}, nil
}

// fetchPayload returns the base64 zippedFilesystem and the concrete revision id
Expand Down
42 changes: 17 additions & 25 deletions internal/skills/geminienterprise/materialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
// Skill Registry (a managed, versioned catalog exposed over the Vertex AI
// v1beta1 REST API) and writes each skill to <target_dir>/<skill-id>/.
//
// It is harness-agnostic: it only writes files and reports what it wrote (see
// Result). It knows nothing about specific harnesses, SKILLS_DIR, or discovery
// pointers — callers decide how a given harness is told where its skills are.
// It is harness-agnostic: it only writes files and reports what it wrote (as a
// mode-agnostic skills.Available). It knows nothing about specific harnesses,
// SKILLS_DIR, or discovery pointers — callers decide how a given harness is told
// where its skills are.
//
// Scope: read-only. This package never creates, updates, or deletes registry
// skills (authoring is out of scope).
Expand All @@ -32,6 +33,7 @@ import (
"strings"

"github.com/google/ax/internal/config"
"github.com/google/ax/internal/skills"
)

// Environment fallbacks for project/location (the registry target_dir is a
Expand All @@ -43,24 +45,10 @@ const (
defaultRegistryLocation = "us-central1"
)

// Result reports what Materialize wrote: one Written entry per registry that
// produced skills, grouping the skills with the directory they landed in.
type Result struct {
Written []Written
}

// Written groups the skills materialized from one registry with the directory
// they were written into (each skill at <Dir>/<skill-id>/).
type Written struct {
Dir string
Skills []MaterializedSkill
}

// Empty reports whether nothing was materialized.
func (r Result) Empty() bool { return len(r.Written) == 0 }

// MaterializedSkill records one skill written to disk.
type MaterializedSkill struct {
// materializedSkill records one skill written to disk. It carries the resolved
// revision (registry-specific detail) internally; Materialize converts these to
// the mode-agnostic skills.Available shape at its boundary.
type materializedSkill struct {
SkillID string
Revision string
Dir string // path of the written skill folder (<target_dir>/<skill-id>/)
Expand All @@ -77,8 +65,8 @@ type MaterializedSkill struct {
// registry's selection, or across registries sharing a dir), the FIRST writer
// wins and later duplicates are skipped with a warning. Substrate/pod
// materialization is a separate, later path; this wires the local flow only.
func Materialize(ctx context.Context, sc config.SkillsConfig) Result {
var res Result
func Materialize(ctx context.Context, sc config.SkillsConfig) skills.Available {
var avail skills.Available
// claimed tracks (target_dir, skill-id) pairs already written across all
// registries so the FIRST writer of an id into a dir wins; later duplicates
// (within one registry's selection, or across registries sharing a dir) are
Expand Down Expand Up @@ -118,9 +106,13 @@ func Materialize(ctx context.Context, sc config.SkillsConfig) Result {
}
log.Printf("skills: registries[%d] materialized %d skill(s) into %s (skipped %d)",
i, len(out.materialized), rc.TargetDir, len(out.skipped))
res.Written = append(res.Written, Written{Dir: rc.TargetDir, Skills: out.materialized})
group := skills.Group{Dir: rc.TargetDir}
for _, m := range out.materialized {
group.Skills = append(group.Skills, skills.Skill{ID: m.SkillID, Dir: m.Dir})
}
avail.Groups = append(avail.Groups, group)
}
return res
return avail
}

// claimSet tracks which (dir, skill-id) pairs have already been materialized, so
Expand Down
Loading
Loading