Skip to content
Merged
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
27 changes: 24 additions & 3 deletions cmd/ax/internal/cliutil/cliutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ 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/geminienterprise"
)

// Controller is the active controller type for this build.
Expand Down Expand Up @@ -64,6 +65,22 @@ 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
// 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.
//
// 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.
var skillsPointer string
if !substrateMode {
skillsPointer = antigravityinteractions.SkillsSystemInstruction(geminienterprise.Materialize(ctx, cfg.Skills))
}

// Built-in Antigravity harness.
var antigravityHarness harness.Harness
if !substrateMode {
Expand Down Expand Up @@ -98,7 +115,8 @@ func NewControllerFromConfig(ctx context.Context, cfg *Config) (*controller.Cont
// Built-in Antigravity Interactions harness.
var antigravityInteractionsHarness harness.Harness
if !substrateMode {
agent := cfg.Harnesses.AntigravityInteractions.Agent
aiCfg := cfg.Harnesses.AntigravityInteractions
agent := aiCfg.Agent
if agent == "" {
agent = antigravityinteractions.DefaultAgent
}
Expand All @@ -108,9 +126,12 @@ func NewControllerFromConfig(ctx context.Context, cfg *Config) (*controller.Cont
if sErr != nil {
return nil, fmt.Errorf("antigravity-interactions harness: %w", sErr)
}
// skillsPointer was built once, up front, from the top-level skills
// config (see above). Append it to any configured system instruction.
antigravityInteractionsHarness, err = antigravityinteractions.New(antigravityinteractions.AntigravityInteractionsConfig{
Agent: agent,
StateDir: stateDir,
Agent: agent,
SystemInstruction: antigravityinteractions.JoinSystemInstruction(aiCfg.SystemInstruction, skillsPointer),
StateDir: stateDir,
})
} else {
antigravityInteractionsHarness, err = substrate.New(config.AntigravityInteractionsHarnessID, "", "", config.AntigravityInteractionsTemplate, 80)
Expand Down
104 changes: 104 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ 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 SkillsConfig `yaml:"skills,omitempty"`
Telemetry TelemetryConfig `yaml:"telemetry,omitempty"`
}

Expand Down Expand Up @@ -105,6 +110,101 @@ type AntigravityHarnessConfig struct {
type AntigravityInteractionsHarnessConfig struct {
Default bool `yaml:"default,omitempty"` // Default harness or not
Agent string `yaml:"agent,omitempty"` // Interactions API agent (default: antigravityinteractions.DefaultAgent)
// SystemInstruction is a free-form system prompt sent on every turn.
SystemInstruction string `yaml:"system_instruction,omitempty"`
}

// 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.
type SkillsConfig struct {
Registries []SkillsRegistryConfig `yaml:"registries,omitempty"`
}

// Validate checks the (top-level) skills config.
func (s SkillsConfig) Validate() error {
for i := range s.Registries {
if err := s.Registries[i].validate(i); err != nil {
return err
}
}
return nil
}

// validate enforces that, when this registry source is enabled, exactly one
// selection mode is set (skills, query, or all) — a config-level "oneof". idx is
// the registry's index within the registries list, for error context.
func (rc SkillsRegistryConfig) validate(idx int) error {
if !rc.Enabled {
return nil
}
if rc.TargetDir == "" {
return fmt.Errorf("skills.registries[%d] requires target_dir (skills materialize to <target_dir>/<skill-id>/)", idx)
}
modes := 0
if len(rc.Skills) > 0 {
modes++
}
if rc.Query != nil {
modes++
}
if rc.All {
modes++
}
switch {
case modes == 0:
return fmt.Errorf("skills.registries[%d] requires exactly one selection mode (set one of skills, query, or all)", idx)
case modes > 1:
return fmt.Errorf("skills.registries[%d] sets multiple selection modes; set exactly one of skills, query, or all", idx)
}
if rc.Query != nil && rc.Query.Text == "" {
return fmt.Errorf("skills.registries[%d].query requires a non-empty text", idx)
}
return nil
}

// SkillsRegistryConfig sources agentskills.io skills from the Gemini Skill
// Registry. When Enabled, exactly one selection mode should be set (Skills,
// Query, or All); if none is set, all skills are materialized.
type SkillsRegistryConfig struct {
Enabled bool `yaml:"enabled,omitempty"`
// Project owns the skills (projects/{Project}/locations/{Location}/skills).
// Empty falls back to the GOOGLE_CLOUD_PROJECT environment variable.
Project string `yaml:"project,omitempty"`
// Location is the registry region, e.g. "us-central1". Empty falls back to
// GOOGLE_CLOUD_LOCATION, then a built-in default.
Location string `yaml:"location,omitempty"`

// --- selection (choose one) ---

// Skills is an explicit allowlist of skills, each optionally pinned to a
// revision. Takes precedence over Query and All.
Skills []SkillRefConfig `yaml:"skills,omitempty"`
// Query is a semantic search selection; its top matches are materialized.
// TopK lives inside it because it only has meaning for a query.
Query *SkillsQueryConfig `yaml:"query,omitempty"`
// All materializes every skill in the project/location (used when neither
// Skills nor Query is set; can also be set explicitly).
All bool `yaml:"all,omitempty"`

// TargetDir is the base directory skills are materialized into: each skill
// is written to <TargetDir>/<skill-id>/. Required when Enabled.
TargetDir string `yaml:"target_dir,omitempty"`
}

// SkillsQueryConfig selects skills by semantic search.
type SkillsQueryConfig struct {
// Text is the semantic search string (required for query selection).
Text string `yaml:"text"`
// TopK bounds the number of matches (<=0 uses the server default).
TopK int `yaml:"top_k,omitempty"`
Comment thread
zbl94 marked this conversation as resolved.
}

// SkillRefConfig identifies a skill to materialize, optionally pinned.
type SkillRefConfig struct {
ID string `yaml:"id"`
Revision string `yaml:"revision,omitempty"`
}

// SubstrateHarnessConfig registers a custom harness deployed on substrate
Expand Down Expand Up @@ -216,6 +316,10 @@ func (c *Config) Validate() error {
return fmt.Errorf("multiple harnesses marked as default")
}

if err := c.Skills.Validate(); err != nil {
return err
}

return nil
}

Expand Down
177 changes: 177 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,180 @@ func TestLoadFromBytes_Invalid(t *testing.T) {
t.Fatal("LoadFromBytes(invalid): got nil error, want error")
}
}

func TestParse_SkillsByID(t *testing.T) {
data := `
skills:
registries:
- enabled: true
project: my-proj
location: us-central1
target_dir: /tmp/ax-skills
skills:
- id: emoji
- id: lowercase
revision: rev-3
`
var cfg Config
if err := yaml.Unmarshal([]byte(data), &cfg); err != nil {
t.Fatalf("Unmarshal failed: %v", err)
}
if len(cfg.Skills.Registries) != 1 {
t.Fatalf("registries = %d, want 1", len(cfg.Skills.Registries))
}
reg := cfg.Skills.Registries[0]
if !reg.Enabled || reg.Project != "my-proj" || reg.Location != "us-central1" || reg.TargetDir != "/tmp/ax-skills" {
t.Errorf("registry = %+v, want enabled my-proj/us-central1 /tmp/ax-skills", reg)
}
if len(reg.Skills) != 2 {
t.Fatalf("skills = %d, want 2", len(reg.Skills))
}
if reg.Skills[0].ID != "emoji" || reg.Skills[0].Revision != "" {
t.Errorf("skills[0] = %+v, want {emoji }", reg.Skills[0])
}
if reg.Skills[1].ID != "lowercase" || reg.Skills[1].Revision != "rev-3" {
t.Errorf("skills[1] = %+v, want {lowercase rev-3}", reg.Skills[1])
}
if reg.Query != nil {
t.Errorf("Query = %+v, want nil in by-id mode", reg.Query)
}
}

func TestParse_SkillsByQuery(t *testing.T) {
data := `
skills:
registries:
- enabled: true
project: my-proj
target_dir: /tmp/ax-skills
query:
text: "find gcp skills"
top_k: 5
`
var cfg Config
if err := yaml.Unmarshal([]byte(data), &cfg); err != nil {
t.Fatalf("Unmarshal failed: %v", err)
}
regs := cfg.Skills.Registries
if len(regs) != 1 || regs[0].Query == nil {
t.Fatalf("registries = %+v, want one with a query block", regs)
}
if regs[0].Query.Text != "find gcp skills" || regs[0].Query.TopK != 5 {
t.Errorf("Query = %+v, want {find gcp skills 5}", *regs[0].Query)
}
}

func TestParse_MultipleRegistries(t *testing.T) {
data := `
skills:
registries:
- enabled: true
project: org-proj
target_dir: /tmp/org
all: true
- enabled: true
project: team-proj
target_dir: /tmp/team
skills:
- id: teamskill
`
var cfg Config
if err := yaml.Unmarshal([]byte(data), &cfg); err != nil {
t.Fatalf("Unmarshal failed: %v", err)
}
regs := cfg.Skills.Registries
if len(regs) != 2 {
t.Fatalf("registries = %d, want 2", len(regs))
}
if regs[0].Project != "org-proj" || regs[0].TargetDir != "/tmp/org" || !regs[0].All {
t.Errorf("registries[0] = %+v, want org-proj /tmp/org all", regs[0])
}
if regs[1].Project != "team-proj" || regs[1].TargetDir != "/tmp/team" || len(regs[1].Skills) != 1 {
t.Errorf("registries[1] = %+v, want team-proj /tmp/team [teamskill]", regs[1])
}
// Each registry has exactly one selection mode + target_dir, so validation passes.
if err := cfg.Skills.Validate(); err != nil {
t.Errorf("Skills.Validate() = %v, want nil", err)
}
}

func TestValidate_SkillsSelectionOneof(t *testing.T) {
// withRegistry returns a valid config carrying a single top-level registry.
// It fills TargetDir (a required field) unless the caller already set one, so
// selection-mode assertions aren't masked by the target_dir check.
withRegistry := func(rc SkillsRegistryConfig) *Config {
if rc.Enabled && rc.TargetDir == "" {
rc.TargetDir = "/tmp/skills"
}
c := validConfig()
c.Skills.Registries = []SkillsRegistryConfig{rc}
return c
}

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

t.Run("enabled without target_dir is an error", func(t *testing.T) {
c := validConfig()
c.Skills.Registries = []SkillsRegistryConfig{
{Enabled: true, Project: "p", All: true}, // valid mode, but no target_dir
}
err := c.Validate()
if err == nil || !strings.Contains(err.Error(), "target_dir") {
t.Fatalf("Validate() = %v, want target_dir error", err)
}
})

t.Run("zero selection modes is an error", func(t *testing.T) {
err := withRegistry(SkillsRegistryConfig{Enabled: true, Project: "p"}).Validate()
if err == nil || !strings.Contains(err.Error(), "exactly one selection mode") {
t.Fatalf("Validate() = %v, want exactly-one error", err)
}
})

t.Run("multiple selection modes is an error", func(t *testing.T) {
err := withRegistry(SkillsRegistryConfig{
Enabled: true, Project: "p",
Skills: []SkillRefConfig{{ID: "emoji"}},
All: true,
}).Validate()
if err == nil || !strings.Contains(err.Error(), "multiple selection modes") {
t.Fatalf("Validate() = %v, want multiple-modes error", err)
}
})

t.Run("exactly one is valid", func(t *testing.T) {
err := withRegistry(SkillsRegistryConfig{
Enabled: true, Project: "p",
Skills: []SkillRefConfig{{ID: "emoji"}},
}).Validate()
if err != nil {
t.Fatalf("Validate() = %v, want nil", err)
}
})

t.Run("query with empty text is an error", func(t *testing.T) {
err := withRegistry(SkillsRegistryConfig{
Enabled: true, Project: "p",
Query: &SkillsQueryConfig{Text: ""},
}).Validate()
if err == nil || !strings.Contains(err.Error(), "non-empty text") {
t.Fatalf("Validate() = %v, want empty-text error", err)
}
})

t.Run("second registry invalid is caught", func(t *testing.T) {
c := validConfig()
c.Skills.Registries = []SkillsRegistryConfig{
{Enabled: true, Project: "p", TargetDir: "/tmp/a", All: true},
{Enabled: true, Project: "p", TargetDir: "/tmp/b"}, // no selection mode
}
err := c.Validate()
if err == nil || !strings.Contains(err.Error(), "registries[1]") {
t.Fatalf("Validate() = %v, want error citing registries[1]", err)
}
})
}
Loading
Loading