diff --git a/pkg/sandbox/kit/kit.go b/pkg/sandbox/kit/kit.go index 4981f71f7..a1c067158 100644 --- a/pkg/sandbox/kit/kit.go +++ b/pkg/sandbox/kit/kit.go @@ -36,11 +36,13 @@ import ( "path/filepath" "slices" "sort" + "strconv" "strings" "time" "unicode/utf8" "github.com/docker/portcullis" + "github.com/fatih/color" "github.com/docker/docker-agent/pkg/config" latestcfg "github.com/docker/docker-agent/pkg/config/latest" @@ -52,6 +54,17 @@ import ( "github.com/docker/docker-agent/pkg/toolinstall" ) +// Output styles for PrintSummary. fatih/color auto-disables when +// stdout is not a TTY, so unit tests and piped output stay plain. +var ( + styleSection = color.New(color.Bold).SprintFunc() + styleName = color.New(color.Bold).SprintFunc() + styleHostPath = color.New(color.FgCyan).SprintFunc() + styleNote = color.New(color.Faint).SprintFunc() + styleRedacted = color.New(color.FgYellow).SprintFunc() + styleCount = color.New(color.Bold).SprintFunc() +) + // manifestFile is the on-disk name of the kit's table of contents. const manifestFile = "manifest.json" @@ -220,16 +233,16 @@ func Build(ctx context.Context, opts Options) (*Result, error) { } // Load the team config so we know which prompt files / skills the - // agent will request. A failure here is non-fatal: we still want - // to ship local skills since they're discovered from $HOME, not - // from the config. We log and continue with an empty config. + // agent will request. A failure here is non-fatal: we ship an empty + // kit (no prompt files, no skills) so the sandbox still boots, but + // the agent will fall back to its in-sandbox defaults. cfg, err := loadConfig(ctx, opts) if err != nil { - slog.DebugContext(ctx, "kit: agent config unavailable; skipping prompt-file collection", "err", err) + slog.DebugContext(ctx, "kit: agent config unavailable; shipping an empty kit", "err", err) cfg = &latestcfg.Config{} } - skillsEntries, redactions, err := stageSkills(stagingDir) + skillsEntries, redactions, err := stageSkills(stagingDir, cfg) if err != nil { return nil, err } @@ -358,14 +371,26 @@ func loadConfig(ctx context.Context, opts Options) (*latestcfg.Config, error) { return config.Load(ctx, source) } -// stageSkills copies every local skill discovered on the host into +// stageSkills copies every local skill the agent config enables into // /skills//, redacting text files in place. -func stageSkills(kitDir string) ([]Entry, []Redaction, error) { +// +// Only skills the agent will actually load are staged: if no agent in +// cfg enables local skills the kit ships nothing, and if every agent +// that enables local skills also restricts them via an `include` +// filter, only the union of those filters is staged. An agent that +// enables local skills without a filter is the wide case — every local +// skill is staged. +func stageSkills(kitDir string, cfg *latestcfg.Config) ([]Entry, []Redaction, error) { target := filepath.Join(kitDir, skills.KitSkillsSubdir) if err := os.MkdirAll(target, 0o750); err != nil { return nil, nil, fmt.Errorf("creating kit skills dir: %w", err) } + include, ok := localSkillFilter(cfg) + if !ok { + return nil, nil, nil + } + var ( entries []Entry redactions []Redaction @@ -374,6 +399,9 @@ func stageSkills(kitDir string) ([]Entry, []Redaction, error) { if skill.BaseDir == "" { continue } + if include != nil && !include[skill.Name] { + continue + } dst := filepath.Join(target, sanitise(skill.Name)) reds, err := copyTree(kitDir, skill.BaseDir, dst) if err != nil { @@ -388,6 +416,41 @@ func stageSkills(kitDir string) ([]Entry, []Redaction, error) { return entries, redactions, nil } +// localSkillFilter inspects every agent in cfg and reports the local +// skill subset the kit should stage. +// +// Returns: +// - (nil, false) when no agent enables local skills — the kit ships +// nothing. +// - (nil, true) when at least one agent enables local skills without +// an `include` filter — every local skill is staged. +// - (set, true) when every agent that enables local skills also +// restricts them — only skills whose name is in the union of +// `include` lists are staged. +func localSkillFilter(cfg *latestcfg.Config) (map[string]bool, bool) { + if cfg == nil { + return nil, false + } + include := make(map[string]bool) + anyLocal := false + for _, agent := range cfg.Agents { + if !agent.Skills.HasLocal() { + continue + } + anyLocal = true + if len(agent.Skills.Include) == 0 { + return nil, true + } + for _, name := range agent.Skills.Include { + include[name] = true + } + } + if !anyLocal { + return nil, false + } + return include, true +} + // stagePromptFiles walks every agent in cfg, records every // add_prompt_files entry the agent will read, and stages the ones // that aren't already reachable inside the sandbox via the live @@ -695,38 +758,47 @@ func (r *Result) PrintSummary(w io.Writer) { return } - fmt.Fprintf(w, "Preparing docker-agent kit at %s\n", r.HostDir) + fmt.Fprintf(w, "Preparing docker-agent kit at %s\n", styleHostPath(pathx.ShortenHome(r.HostDir))) if len(skillFiles) > 0 { - fmt.Fprintln(w, " skills:") + fmt.Fprintf(w, " %s\n", styleSection("skills:")) for _, group := range skillFiles { fmt.Fprintf(w, " %s\n", displaySkillHeader(group.entry)) for _, file := range group.files { - mark := "" + rel := strings.TrimPrefix(file, group.entry.Target+string(filepath.Separator)) if redacted[file] { - mark = " (redacted)" + fmt.Fprintf(w, " %s %s\n", rel, styleRedacted("(redacted)")) + } else { + fmt.Fprintf(w, " %s\n", rel) } - rel := strings.TrimPrefix(file, group.entry.Target+string(filepath.Separator)) - fmt.Fprintf(w, " %s%s\n", rel, mark) } } } if len(promptEntries) > 0 { - fmt.Fprintln(w, " prompt files:") + fmt.Fprintf(w, " %s\n", styleSection("prompt files:")) for _, e := range promptEntries { name := promptFileName(e) notes := []string{"from " + pathx.ShortenHome(e.Source)} + isRedacted := false if !e.IsStaged() { notes = append(notes, "workspace mount") } else if redacted[e.Target] { notes = append(notes, "redacted") + isRedacted = true + } + joined := strings.Join(notes, ", ") + paren := fmt.Sprintf("(%s)", joined) + if isRedacted { + paren = styleRedacted(paren) + } else { + paren = styleNote(paren) } - fmt.Fprintf(w, " %s (%s)\n", name, strings.Join(notes, ", ")) + fmt.Fprintf(w, " %s %s\n", styleName(name), paren) } } - fmt.Fprintf(w, " summary: %s\n", summaryCounts(len(skillFiles), len(promptEntries), len(r.Manifest.Redactions))) + fmt.Fprintf(w, " %s %s\n", styleSection("summary:"), summaryCounts(len(skillFiles), len(promptEntries), len(r.Manifest.Redactions))) } // promptFileName returns the user-visible name for a prompt-file @@ -780,9 +852,9 @@ func (r *Result) skillFilesGrouped() []skillGroup { func displaySkillHeader(e Entry) string { name := filepath.Base(e.Target) if e.Source == "" { - return name + return styleName(name) } - return fmt.Sprintf("%s (from %s)", name, pathx.ShortenHome(e.Source)) + return fmt.Sprintf("%s %s", styleName(name), styleNote(fmt.Sprintf("(from %s)", pathx.ShortenHome(e.Source)))) } // summaryCounts formats the trailing line of PrintSummary. @@ -790,16 +862,16 @@ func summaryCounts(skillCount, promptCount, redactionCount int) string { parts := []string{plural(skillCount, "skill")} parts = append(parts, plural(promptCount, "prompt file")) if redactionCount > 0 { - parts = append(parts, plural(redactionCount, "secret")+" redacted") + parts = append(parts, styleRedacted(plural(redactionCount, "secret")+" redacted")) } return strings.Join(parts, ", ") } func plural(n int, what string) string { if n == 1 { - return "1 " + what + return styleCount("1") + " " + what } - return fmt.Sprintf("%d %ss", n, what) + return fmt.Sprintf("%s %ss", styleCount(strconv.Itoa(n)), what) } // needsAutoInstall reports whether cfg has at least one toolset that diff --git a/pkg/sandbox/kit/kit_test.go b/pkg/sandbox/kit/kit_test.go index fa50ce4a5..8fde70c08 100644 --- a/pkg/sandbox/kit/kit_test.go +++ b/pkg/sandbox/kit/kit_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "os" "path/filepath" + "sort" "strings" "sync" "testing" @@ -485,7 +486,7 @@ func TestBuild_ConcurrentRunsForSameAgentAreSafe(t *testing.T) { cacheDir := t.TempDir() optsTemplate := Options{ - AgentRef: "shared-ref", + AgentRef: "default", // builtin enables skills, so the kit stages them HostHome: hostHome, HostCwd: t.TempDir(), CacheDir: cacheDir, @@ -574,6 +575,7 @@ agents: model: openai/gpt-5 description: tester instruction: hello + skills: true add_prompt_files: ["AGENTS.md"] models: openai/gpt-5: @@ -693,6 +695,195 @@ func TestPrintSummary_NilReceiver(t *testing.T) { assert.Empty(t, buf.String()) } +// stageSkillsTestSetup creates two local skills ("alpha" and "beta") +// under hostHome and returns the agent YAML path the test will load. +// The agent's `skills:` value is configurable so each scenario can +// exercise a different filter. +func stageSkillsTestSetup(t *testing.T, skillsYAML string) (hostHome, workspace, yamlPath string) { + t.Helper() + isolateEnv(t) + hostHome = t.TempDir() + workspace = t.TempDir() + + for _, name := range []string{"alpha", "beta"} { + dir := filepath.Join(hostHome, ".agents", "skills", name) + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), + []byte("---\nname: "+name+"\ndescription: "+name+"\n---\n"), 0o644)) + } + + agentYAML := []byte(`agents: + root: + model: openai/gpt-5 + description: tester + instruction: hello + skills: ` + skillsYAML + ` +models: + openai/gpt-5: + provider: openai + model: gpt-5 +`) + yamlPath = filepath.Join(workspace, "agent.yaml") + require.NoError(t, os.WriteFile(yamlPath, agentYAML, 0o600)) + + t.Setenv("HOME", hostHome) + t.Chdir(workspace) + return hostHome, workspace, yamlPath +} + +func stagedSkillNames(res *Result) []string { + names := make([]string, 0, len(res.Manifest.Skills)) + for _, e := range res.Manifest.Skills { + names = append(names, filepath.Base(e.Target)) + } + sort.Strings(names) + return names +} + +func TestBuild_SkillsDisabledShipsNothing(t *testing.T) { + hostHome, workspace, yamlPath := stageSkillsTestSetup(t, "false") + + res, err := Build(t.Context(), Options{ + AgentRef: yamlPath, + HostHome: hostHome, + HostCwd: workspace, + Workspace: workspace, + CacheDir: t.TempDir(), + }) + require.NoError(t, err) + assert.Empty(t, res.Manifest.Skills, + "agent with skills: false must not stage any local skill into the kit") +} + +func TestBuild_SkillsTrueShipsAll(t *testing.T) { + hostHome, workspace, yamlPath := stageSkillsTestSetup(t, "true") + + res, err := Build(t.Context(), Options{ + AgentRef: yamlPath, + HostHome: hostHome, + HostCwd: workspace, + Workspace: workspace, + CacheDir: t.TempDir(), + }) + require.NoError(t, err) + assert.Equal(t, []string{"alpha", "beta"}, stagedSkillNames(res)) +} + +func TestBuild_SkillsIncludeFilters(t *testing.T) { + hostHome, workspace, yamlPath := stageSkillsTestSetup(t, `["alpha"]`) + + res, err := Build(t.Context(), Options{ + AgentRef: yamlPath, + HostHome: hostHome, + HostCwd: workspace, + Workspace: workspace, + CacheDir: t.TempDir(), + }) + require.NoError(t, err) + assert.Equal(t, []string{"alpha"}, stagedSkillNames(res), + "only the named skill must be staged when skills is filtered") +} + +func TestBuild_SkillsIncludeUnionAcrossAgents(t *testing.T) { + isolateEnv(t) + hostHome := t.TempDir() + workspace := t.TempDir() + + for _, name := range []string{"alpha", "beta", "gamma"} { + dir := filepath.Join(hostHome, ".agents", "skills", name) + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), + []byte("---\nname: "+name+"\ndescription: x\n---\n"), 0o644)) + } + + // Two agents with disjoint filters and a third that disables + // skills entirely. The kit must stage the union of the first two + // (alpha + beta) and ignore the third. + agentYAML := []byte(`agents: + one: + model: openai/gpt-5 + description: one + instruction: hi + skills: ["alpha"] + two: + model: openai/gpt-5 + description: two + instruction: hi + skills: ["beta"] + three: + model: openai/gpt-5 + description: three + instruction: hi + skills: false +models: + openai/gpt-5: + provider: openai + model: gpt-5 +`) + yamlPath := filepath.Join(workspace, "agent.yaml") + require.NoError(t, os.WriteFile(yamlPath, agentYAML, 0o600)) + + t.Setenv("HOME", hostHome) + t.Chdir(workspace) + + res, err := Build(t.Context(), Options{ + AgentRef: yamlPath, + HostHome: hostHome, + HostCwd: workspace, + Workspace: workspace, + CacheDir: t.TempDir(), + }) + require.NoError(t, err) + assert.Equal(t, []string{"alpha", "beta"}, stagedSkillNames(res)) +} + +func TestBuild_SkillsUnfilteredAgentWidens(t *testing.T) { + isolateEnv(t) + hostHome := t.TempDir() + workspace := t.TempDir() + + for _, name := range []string{"alpha", "beta"} { + dir := filepath.Join(hostHome, ".agents", "skills", name) + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), + []byte("---\nname: "+name+"\ndescription: x\n---\n"), 0o644)) + } + + // One agent restricts skills to "alpha"; another has no filter. + // The unfiltered agent widens the kit to every local skill. + agentYAML := []byte(`agents: + one: + model: openai/gpt-5 + description: one + instruction: hi + skills: ["alpha"] + two: + model: openai/gpt-5 + description: two + instruction: hi + skills: true +models: + openai/gpt-5: + provider: openai + model: gpt-5 +`) + yamlPath := filepath.Join(workspace, "agent.yaml") + require.NoError(t, os.WriteFile(yamlPath, agentYAML, 0o600)) + + t.Setenv("HOME", hostHome) + t.Chdir(workspace) + + res, err := Build(t.Context(), Options{ + AgentRef: yamlPath, + HostHome: hostHome, + HostCwd: workspace, + Workspace: workspace, + CacheDir: t.TempDir(), + }) + require.NoError(t, err) + assert.Equal(t, []string{"alpha", "beta"}, stagedSkillNames(res)) +} + func TestNeedsAutoInstall(t *testing.T) { t.Parallel() diff --git a/pkg/toolinstall/resolver.go b/pkg/toolinstall/resolver.go index 76d73ece6..90ab72fce 100644 --- a/pkg/toolinstall/resolver.go +++ b/pkg/toolinstall/resolver.go @@ -10,6 +10,8 @@ import ( "runtime/debug" "strings" + "github.com/fatih/color" + "github.com/mattn/go-isatty" "golang.org/x/sync/singleflight" ) @@ -100,8 +102,6 @@ func doInstall(ctx context.Context, command, versionRef string) (string, error) return binPath, nil } - slog.InfoContext(ctx, "Auto-installing missing command via aqua registry", "command", command) - registry := SharedRegistry() pkg, version, err := lookupPackage(ctx, registry, command, versionRef) @@ -117,7 +117,9 @@ func doInstall(ctx context.Context, command, versionRef string) (string, error) } pkgName := pkg.RepoOwner + "/" + pkg.RepoName - slog.InfoContext(ctx, "Installing tool", "command", command, "package", pkgName, "version", version) + slog.InfoContext(ctx, "Auto-installing missing command", + "command", command, "package", pkgName, "version", version) + announceInstall(command, pkgName, version) binaryPath, err := registry.Install(ctx, pkg, version) if err != nil { @@ -130,6 +132,49 @@ func doInstall(ctx context.Context, command, versionRef string) (string, error) return binaryPath, nil } +// announceInstall prints a single user-visible line to stderr right +// before downloading a tool, so the user understands what the +// upcoming `go install` / GitHub-release chatter is about. We +// intentionally avoid stdout so this never gets piped into the +// agent's prompt or programmatic output. +// +// fatih/color's global NoColor is computed from stdout, so we cannot +// rely on it: when stderr is redirected (e.g. `agent run ... 2>log`) +// stdout may still be a TTY and emit escapes into the log, and when +// stdout is piped (e.g. `agent run ... | tee log`) but stderr is a +// TTY, NoColor is true and the styled branch silently degrades to +// plain text. Decide based on stderr explicitly and force colour on +// the local *color.Color values; honour NO_COLOR / TERM=dumb. +func announceInstall(command, pkgName, version string) { + if stderrSupportsColor() { + // fatih/color's package-level NoColor is set from stdout's TTY + // state, so when stdout is piped but stderr is still a TTY the + // SprintFunc helpers would silently strip ANSI codes. Force the + // local colours on after we've decided stderr can handle them. + bold := color.New(color.Bold) + bold.EnableColor() + faint := color.New(color.Faint) + faint.EnableColor() + fmt.Fprintf(os.Stderr, "Installing %s %s\n", + bold.Sprint(command), faint.Sprintf("(%s@%s)", pkgName, version)) + return + } + fmt.Fprintf(os.Stderr, "Installing %s (%s@%s)\n", command, pkgName, version) +} + +// stderrSupportsColor reports whether ANSI escapes are safe to write +// to os.Stderr. +func stderrSupportsColor() bool { + if os.Getenv("NO_COLOR") != "" || os.Getenv("TERM") == "dumb" { + return false + } + if os.Stderr == nil { + return false + } + fd := os.Stderr.Fd() + return isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd) +} + // lookupPackage resolves the aqua package for a command. // If versionRef is provided (e.g. "owner/repo@v1.0"), it parses the reference // and looks up by name. Otherwise, it searches by command name.