diff --git a/cli/azd/extensions/azure.ai.skills/README.md b/cli/azd/extensions/azure.ai.skills/README.md index e400da6b0ab..dd1d14144d2 100644 --- a/cli/azd/extensions/azure.ai.skills/README.md +++ b/cli/azd/extensions/azure.ai.skills/README.md @@ -7,6 +7,11 @@ terminal. ## Commands ```bash +azd ai skill add [--description "..." --instructions "..."] +azd ai skill add --file ./SKILL.md +azd ai skill add --file ./skill.zip +azd ai skill add --file ./skill-src/ + azd ai skill create [--description "..." --instructions "..."] azd ai skill create --file ./SKILL.md azd ai skill create --file ./skill.zip @@ -28,6 +33,12 @@ version; `update` uploads a new default version (or, with version). Names follow the agentskills.io spec (`^[a-z0-9]([a-z0-9\-]*[a-z0-9])?$`, max 64 chars). +`add` is declarative. It adds or updates a +`host: azure.ai.skill` service in the current project's `azure.yaml` without +mutating the remote skill. Run `azd deploy ` or `azd up` afterward to +reconcile it. Existing `uses:`, `project:`, and unowned service fields are +preserved. + `create` accepts inline content (`--description` / `--instructions`), a single `SKILL.md` file, a `.zip` package, or a directory whose root contains a `SKILL.md`. Directory mode is the round-trip inverse of @@ -49,8 +60,14 @@ All commands accept the standard cross-cutting flags: `-p` / `--project-endpoint ## Composing skills in `azure.yaml` -Declare a skill as its own service to reconcile it with `azd deploy` or -`azd up`: +Use the owning extension to add a skill service, then declare the dependency +from each consuming agent: + +```bash +azd ai skill add triage-rules \ + --description "Rules for triaging incoming issues" \ + --instructions "Classify the issue, identify its owner, and recommend next steps." +``` ```yaml services: @@ -67,11 +84,18 @@ services: support-agent: host: azure.ai.agent + kind: hosted + name: support-agent + project: ./agents/support-agent + image: ghcr.io/example/support-agent:latest uses: - triage-rules - skill: triage-rules ``` +The skill command does not infer which agents consume the skill. Add the skill +service name to each consuming agent's `uses:` list to declare deployment +ordering explicitly. + `instructions` can also reference a `.md` or `.txt` file. To preserve a complete skill package, use `archive` instead of the inline fields: @@ -87,6 +111,10 @@ Relative instruction and archive paths resolve from the service's `project` path when set, otherwise from the directory containing `azure.yaml`. Parent traversal (`..`) is rejected. +When `add` receives a ZIP or directory, the source must be inside that service +directory. The command stores a portable forward-slash relative reference and +rejects host-name collisions instead of overwriting another service type. + Deploying the skill creates a new immutable default version and publishes readiness markers for dependent agent services. A consuming agent must list the skill service in `uses:` so azd deploys it first. Removing the service from diff --git a/cli/azd/extensions/azure.ai.skills/extension.yaml b/cli/azd/extensions/azure.ai.skills/extension.yaml index b8fac06b9e6..3c1013937a5 100644 --- a/cli/azd/extensions/azure.ai.skills/extension.yaml +++ b/cli/azd/extensions/azure.ai.skills/extension.yaml @@ -20,6 +20,9 @@ tags: - ai - skill examples: + - name: add + description: Add a skill service to the current azure.yaml. + usage: azd ai skill add my-skill --file ./skills/my-skill - name: list description: List skills in the current Foundry project. usage: azd ai skill list diff --git a/cli/azd/extensions/azure.ai.skills/internal/cmd/root.go b/cli/azd/extensions/azure.ai.skills/internal/cmd/root.go index a5f2ddd749e..d37c05b5a79 100644 --- a/cli/azd/extensions/azure.ai.skills/internal/cmd/root.go +++ b/cli/azd/extensions/azure.ai.skills/internal/cmd/root.go @@ -21,8 +21,8 @@ at runtime — from your terminal. Skills carry either inline JSON (description + Markdown instructions) or a packaged ZIP archive bundling SKILL.md plus any sibling assets. Use this -command group to create, update, show, list, download, and delete skills in -a Foundry project.`, +command group to compose skills into azure.yaml or create, update, show, list, +download, and delete skills in a Foundry project.`, }) rootCmd.SilenceUsage = true rootCmd.SilenceErrors = true @@ -49,6 +49,7 @@ a Foundry project.`, rootCmd.AddCommand(newMetadataCommand(rootCmd)) rootCmd.AddCommand(newContextCommand()) + rootCmd.AddCommand(newAddCommand(extCtx)) rootCmd.AddCommand(newCreateCommand(extCtx)) rootCmd.AddCommand(newUpdateCommand(extCtx)) rootCmd.AddCommand(newShowCommand(extCtx)) diff --git a/cli/azd/extensions/azure.ai.skills/internal/cmd/service_target.go b/cli/azd/extensions/azure.ai.skills/internal/cmd/service_target.go index 49db510d20b..030a2c0937e 100644 --- a/cli/azd/extensions/azure.ai.skills/internal/cmd/service_target.go +++ b/cli/azd/extensions/azure.ai.skills/internal/cmd/service_target.go @@ -438,6 +438,9 @@ func prepareSkillArchive(path string) (*preparedSkillArchive, error) { if err != nil { return nil, classifyArchiveDirectoryError(err, path) } + if err := validateSkillArchiveUploadSize(path, int64(len(data))); err != nil { + return nil, err + } return &preparedSkillArchive{ Name: filepath.Base(filepath.Clean(path)) + ".zip", Reader: io.NopCloser(bytes.NewReader(data)), @@ -459,12 +462,8 @@ func prepareSkillArchive(path string) (*preparedSkillArchive, error) { "set archive to a .zip file or a directory containing SKILL.md", ) } - if info.Size() > skill_api.MaxUploadBytes { - return nil, exterrors.Validation( - exterrors.CodeInvalidSkillFile, - fmt.Sprintf("skill archive %s exceeds the 25 MB upload size limit", path), - "reduce the archive size to 25 MB or less", - ) + if err := validateSkillArchiveUploadSize(path, info.Size()); err != nil { + return nil, err } file, err := os.Open(path) //nolint:gosec // user-authored azure.yaml path opened on user's behalf if err != nil { @@ -480,6 +479,17 @@ func prepareSkillArchive(path string) (*preparedSkillArchive, error) { }, nil } +func validateSkillArchiveUploadSize(path string, size int64) error { + if size <= skill_api.MaxUploadBytes { + return nil + } + return exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("skill archive %s exceeds the 25 MB upload size limit", path), + "reduce the archive size to 25 MB or less", + ) +} + // hasParentTraversal reports whether a relative path contains a ".." segment // that could escape its base directory, treating both '/' and '\' as separators. func hasParentTraversal(p string) bool { @@ -493,7 +503,7 @@ func hasParentTraversal(p string) bool { func isInstructionFilePath(instructions string) bool { value := strings.TrimSpace(instructions) - if strings.ContainsAny(value, "\r\n") { + if strings.ContainsAny(value, " \t\r\n") { return false } switch strings.ToLower(filepath.Ext(value)) { diff --git a/cli/azd/extensions/azure.ai.skills/internal/cmd/service_target_test.go b/cli/azd/extensions/azure.ai.skills/internal/cmd/service_target_test.go index a84d25ca8ad..defd7959e8f 100644 --- a/cli/azd/extensions/azure.ai.skills/internal/cmd/service_target_test.go +++ b/cli/azd/extensions/azure.ai.skills/internal/cmd/service_target_test.go @@ -228,6 +228,15 @@ func TestResolveSkillInstructions_MultilineBodyEndingInFileExtensionIsInline(t * assert.Equal(t, instructions, got) } +func TestResolveSkillInstructions_SingleLineBodyEndingInFileExtensionIsInline(t *testing.T) { + t.Parallel() + + instructions := "Follow README.md" + got, err := resolveSkillInstructions("", &azdext.ServiceConfig{Name: "inline"}, instructions) + require.NoError(t, err) + assert.Equal(t, instructions, got) +} + func TestResolveSkillInstructions_FilePath(t *testing.T) { t.Parallel() @@ -375,6 +384,17 @@ func TestPrepareSkillArchive_RejectsOversizedZip(t *testing.T) { assert.Nil(t, archive) } +func TestValidateSkillArchiveUploadSize_RejectsOversizedArchive(t *testing.T) { + t.Parallel() + + require.NoError(t, validateSkillArchiveUploadSize("skill.zip", skill_api.MaxUploadBytes)) + require.ErrorContains( + t, + validateSkillArchiveUploadSize("skill.zip", skill_api.MaxUploadBytes+1), + "exceeds the 25 MB upload size limit", + ) +} + func TestPrepareSkillArchive_RejectsNonRegularZip(t *testing.T) { t.Parallel() if runtime.GOOS == "windows" { diff --git a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_add.go b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_add.go new file mode 100644 index 00000000000..3fa30a1da6e --- /dev/null +++ b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_add.go @@ -0,0 +1,211 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + + "azureaiskills/internal/exterrors" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/spf13/cobra" +) + +type addFlags struct { + name string + description string + instructions string + file string + output string + noPrompt bool + + descriptionSet bool + instructionsSet bool +} + +type addAction struct { + flags *addFlags + upsert func(context.Context, skillServiceDeclaration) (*skillServiceUpsertResult, error) + writer io.Writer + errorWriter io.Writer +} + +func (a *addAction) Run(ctx context.Context) error { + if err := validateSkillName(a.flags.name); err != nil { + return err + } + + declaration, err := a.buildDeclaration() + if err != nil { + return err + } + if a.upsert == nil { + return fmt.Errorf("skill service upsert is not configured") + } + result, err := a.upsert(ctx, declaration) + if err != nil { + return err + } + if result == nil { + return fmt.Errorf("skill service upsert returned no result") + } + writer := a.writer + if writer == nil { + writer = io.Discard + } + return writeSkillServiceUpsertResult(writer, result, a.flags.output) +} + +func (a *addAction) buildDeclaration() (skillServiceDeclaration, error) { + mode, err := selectCreateMode(&createFlags{ + description: a.flags.description, + instructions: a.flags.instructions, + file: a.flags.file, + descriptionSet: a.flags.descriptionSet, + instructionsSet: a.flags.instructionsSet, + }) + if err != nil { + return skillServiceDeclaration{}, err + } + + declaration := skillServiceDeclaration{Name: a.flags.name} + switch mode { + case modeInline: + declaration.Config = skillServiceConfig{ + Description: a.flags.description, + Instructions: a.flags.instructions, + } + case modeFileMd: + parsed, err := loadSkillMd(a.flags.file) + if err != nil { + return skillServiceDeclaration{}, err + } + if parsed.Name != "" && + parsed.Name != a.flags.name && + !shouldSuppressWarning(a.flags.noPrompt, a.flags.output) { + errorWriter := a.errorWriter + if errorWriter == nil { + errorWriter = io.Discard + } + fmt.Fprintf( + errorWriter, + "Warning: SKILL.md front matter `name: %q` does not match positional argument %q; using %q\n", + parsed.Name, + a.flags.name, + a.flags.name, + ) + } + declaration.Config = skillServiceConfig{ + Description: parsed.Description, + Instructions: parsed.Instructions, + License: parsed.License, + Compatibility: parsed.Compatibility, + Metadata: parsed.Metadata, + Tools: parsed.AllowedTools, + } + case modeFilePackage, modeFileDirectory: + declaration.ArchiveSource = a.flags.file + case modeNone: + return skillServiceDeclaration{}, exterrors.Validation( + exterrors.CodeMissingRequiredField, + "no content supplied to skill add", + "pass --description and --instructions, or --file ", + ) + default: + return skillServiceDeclaration{}, exterrors.Validation( + exterrors.CodeInvalidParameter, + "unsupported skill add mode", + "this is a bug; please file an issue", + ) + } + + if declaration.ArchiveSource == "" { + if err := validateSkillServiceConfig(declaration.Name, &declaration.Config); err != nil { + return skillServiceDeclaration{}, err + } + } + return declaration, nil +} + +func writeSkillServiceUpsertResult( + writer io.Writer, + result *skillServiceUpsertResult, + format string, +) error { + if format == outputJSON { + encoder := json.NewEncoder(writer) + encoder.SetIndent("", " ") + encoder.SetEscapeHTML(false) + return encoder.Encode(result) + } + + action := "updated" + if result.Created { + action = "added" + } + _, err := fmt.Fprintf(writer, "Skill service %q %s in azure.yaml.\n", result.Name, action) + return err +} + +func newAddCommand(extCtx *azdext.ExtensionContext) *cobra.Command { + flags := &addFlags{} + action := &addAction{ + flags: flags, + upsert: upsertSkillServiceToProject, + writer: os.Stdout, + errorWriter: os.Stderr, + } + + cmd := &cobra.Command{ + Use: "add ", + Short: "Add or update a Foundry skill service in azure.yaml.", + Long: `Add or update a host: azure.ai.skill service in the current azd +project's azure.yaml. + +This command is declarative: it only updates azure.yaml and does not create or +modify the remote Foundry skill. Run azd deploy or azd up to reconcile +the service after adding it. + +Accepted content shapes: + + 1. Inline: --description "..." --instructions "..." + 2. SKILL.md: --file ./SKILL.md + 3. Package: --file ./skill.zip + 4. Directory: --file ./skill-src + +Inline and SKILL.md inputs are stored as service properties. ZIP and directory +inputs are stored as portable archive references. Updating an existing skill +service preserves uses:, project:, and fields owned by other extensions.`, + Example: ` azd ai skill add triage-rules --description "Triage issues" --instructions "Classify each issue." + azd ai skill add triage-rules --file ./SKILL.md + azd ai skill add triage-rules --file ./skills/triage-rules + azd deploy triage-rules`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + flags.name = args[0] + flags.output = extCtx.OutputFormat + flags.noPrompt = extCtx.NoPrompt + flags.descriptionSet = cmd.Flags().Changed("description") + flags.instructionsSet = cmd.Flags().Changed("instructions") + return action.Run(azdext.WithAccessToken(cmd.Context())) + }, + } + + cmd.Flags().StringVar(&flags.description, "description", "", "Inline mode: human-readable summary of the skill") + cmd.Flags().StringVar(&flags.instructions, "instructions", "", "Inline mode: Markdown body defining skill behavior") + cmd.Flags().StringVar( + &flags.file, + "file", + "", + "Path to SKILL.md, a .zip package, or a directory containing SKILL.md at its root", + ) + azdext.RegisterFlagOptions(cmd, azdext.FlagOptions{ + Name: "output", AllowedValues: []string{outputJSON, outputTable}, Default: outputJSON, + }) + return cmd +} diff --git a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_add_test.go b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_add_test.go new file mode 100644 index 00000000000..6ce44dd83a2 --- /dev/null +++ b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_add_test.go @@ -0,0 +1,164 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "bytes" + "context" + "os" + "path/filepath" + "testing" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAddAction_RunWritesServiceOnly(t *testing.T) { + t.Parallel() + + var captured skillServiceDeclaration + var output bytes.Buffer + action := &addAction{ + flags: &addFlags{ + name: "triage-rules", + description: "Triage issues", + instructions: "Classify each issue.", + descriptionSet: true, + instructionsSet: true, + output: outputTable, + }, + upsert: func( + _ context.Context, + declaration skillServiceDeclaration, + ) (*skillServiceUpsertResult, error) { + captured = declaration + return &skillServiceUpsertResult{ + Name: declaration.Name, + Host: aiSkillHost, + Created: true, + }, nil + }, + writer: &output, + errorWriter: &bytes.Buffer{}, + } + + require.NoError(t, action.Run(t.Context())) + assert.Equal(t, "triage-rules", captured.Name) + assert.Equal(t, "Triage issues", captured.Config.Description) + assert.Equal(t, "Classify each issue.", captured.Config.Instructions) + assert.Empty(t, captured.ArchiveSource) + assert.Equal(t, "Skill service \"triage-rules\" added in azure.yaml.\n", output.String()) +} + +func TestAddAction_BuildsCompleteSkillMdDeclaration(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "SKILL.md") + require.NoError(t, os.WriteFile(path, []byte(`--- +name: source-name +description: Review code +license: MIT +compatibility: gpt-5 +metadata: + owner: platform +allowed_tools: + - code_interpreter +--- +Review code for correctness. +`), 0600)) + + var warnings bytes.Buffer + action := &addAction{ + flags: &addFlags{ + name: "code-review", + file: path, + output: outputTable, + noPrompt: false, + }, + errorWriter: &warnings, + } + declaration, err := action.buildDeclaration() + require.NoError(t, err) + + assert.Equal(t, "code-review", declaration.Name) + assert.Equal(t, "Review code", declaration.Config.Description) + assert.Equal(t, "Review code for correctness.\n", declaration.Config.Instructions) + assert.Equal(t, "MIT", declaration.Config.License) + assert.Equal(t, "gpt-5", declaration.Config.Compatibility) + assert.Equal(t, map[string]string{"owner": "platform"}, declaration.Config.Metadata) + assert.Equal(t, []string{"code_interpreter"}, declaration.Config.Tools) + assert.Contains(t, warnings.String(), "does not match positional argument") +} + +func TestAddAction_BuildsArchiveDeclaration(t *testing.T) { + t.Parallel() + + for _, path := range []string{"skill.zip", "skill-dir"} { + t.Run(path, func(t *testing.T) { + t.Parallel() + + fullPath := filepath.Join(t.TempDir(), path) + if filepath.Ext(fullPath) == ".zip" { + require.NoError(t, os.WriteFile(fullPath, []byte("zip"), 0600)) + } else { + require.NoError(t, os.MkdirAll(fullPath, 0750)) + } + action := &addAction{ + flags: &addFlags{name: "triage-rules", file: fullPath}, + errorWriter: &bytes.Buffer{}, + } + + declaration, err := action.buildDeclaration() + require.NoError(t, err) + assert.Equal(t, fullPath, declaration.ArchiveSource) + assert.Empty(t, declaration.Config) + }) + } +} + +func TestAddAction_RequiresContent(t *testing.T) { + t.Parallel() + + action := &addAction{ + flags: &addFlags{name: "triage-rules"}, + errorWriter: &bytes.Buffer{}, + } + _, err := action.buildDeclaration() + require.ErrorContains(t, err, "no content supplied") +} + +func TestWriteSkillServiceUpsertResult_JSON(t *testing.T) { + t.Parallel() + + var output bytes.Buffer + require.NoError(t, writeSkillServiceUpsertResult(&output, &skillServiceUpsertResult{ + Name: "triage-rules", + Host: aiSkillHost, + ProjectPath: "C:/src/app", + Created: true, + }, outputJSON)) + assert.JSONEq(t, `{ + "name": "triage-rules", + "host": "azure.ai.skill", + "projectPath": "C:/src/app", + "created": true + }`, output.String()) +} + +func TestNewAddCommand_RegistersFlags(t *testing.T) { + t.Parallel() + + cmd := newAddCommand(&azdext.ExtensionContext{}) + assert.NotNil(t, cmd.Flags().Lookup("description")) + assert.NotNil(t, cmd.Flags().Lookup("instructions")) + assert.NotNil(t, cmd.Flags().Lookup("file")) + assert.Contains(t, cmd.Long, "does not create or") +} + +func TestRootCommand_RegistersAdd(t *testing.T) { + command, _, err := NewRootCommand().Find([]string{"add"}) + require.NoError(t, err) + assert.Equal(t, "add", command.Name()) +} diff --git a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_create.go b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_create.go index af870740079..140432b0c79 100644 --- a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_create.go +++ b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_create.go @@ -128,18 +128,10 @@ func (a *createAction) runInline(ctx context.Context, client *skill_api.Client) } func (a *createAction) runFileMd(ctx context.Context, client *skill_api.Client) error { - data, err := readFileWithLimit(a.flags.file) + parsed, err := loadSkillMd(a.flags.file) if err != nil { return err } - parsed, parseErr := skill_api.ParseSkillMd(data) - if parseErr != nil { - return exterrors.Validation( - exterrors.CodeInvalidSkillFile, - fmt.Sprintf("failed to parse %s: %s", a.flags.file, parseErr), - "ensure the file begins with a YAML front matter block delimited by '---'", - ) - } if parsed.Name != "" && parsed.Name != a.flags.name && !shouldSuppressWarning(a.flags.noPrompt, a.flags.output) { fmt.Fprintf(os.Stderr, @@ -325,18 +317,10 @@ func verifyDirectoryNameMatches(dirPath, positionalName string) error { // verifyMdNameMatches reads the SKILL.md front matter and refuses --force on // a `name` mismatch. Returns nil when SKILL.md omits `name` or the names agree. func verifyMdNameMatches(filePath, positionalName string) error { - data, err := readFileWithLimit(filePath) + parsed, err := loadSkillMd(filePath) if err != nil { return err } - parsed, parseErr := skill_api.ParseSkillMd(data) - if parseErr != nil { - return exterrors.Validation( - exterrors.CodeInvalidSkillFile, - fmt.Sprintf("failed to parse %s: %s", filePath, parseErr), - "ensure the file begins with a YAML front matter block delimited by '---'", - ) - } if parsed.Name == "" || parsed.Name == positionalName { return nil } diff --git a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_input.go b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_input.go new file mode 100644 index 00000000000..b92dfc68c51 --- /dev/null +++ b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_input.go @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "fmt" + + "azureaiskills/internal/exterrors" + "azureaiskills/internal/pkg/skill_api" +) + +func loadSkillMd(path string) (*skill_api.SkillMd, error) { + data, err := readFileWithLimit(path) + if err != nil { + return nil, err + } + parsed, err := skill_api.ParseSkillMd(data) + if err != nil { + return nil, exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("failed to parse %s: %s", path, err), + "ensure the file begins with a YAML front matter block delimited by '---'", + ) + } + return parsed, nil +} diff --git a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_service_config.go b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_service_config.go new file mode 100644 index 00000000000..aa7b1030999 --- /dev/null +++ b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_service_config.go @@ -0,0 +1,289 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "context" + "encoding/json" + "fmt" + "maps" + "os" + "path/filepath" + "strings" + + "azureaiskills/internal/exterrors" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "google.golang.org/grpc" + "google.golang.org/protobuf/types/known/structpb" +) + +var skillServiceOwnedFields = []string{ + "archive", + "compatibility", + "description", + "instructions", + "license", + "metadata", + "tools", +} + +type skillServiceDeclaration struct { + Name string + Config skillServiceConfig + ArchiveSource string +} + +type skillServiceUpsertResult struct { + Name string `json:"name"` + Host string `json:"host"` + ProjectPath string `json:"projectPath"` + Created bool `json:"created"` +} + +type skillProjectClient interface { + Get( + ctx context.Context, + in *azdext.EmptyRequest, + opts ...grpc.CallOption, + ) (*azdext.GetProjectResponse, error) + AddService( + ctx context.Context, + in *azdext.AddServiceRequest, + opts ...grpc.CallOption, + ) (*azdext.EmptyResponse, error) + GetServiceConfigSection( + ctx context.Context, + in *azdext.GetServiceConfigSectionRequest, + opts ...grpc.CallOption, + ) (*azdext.GetServiceConfigSectionResponse, error) + SetServiceConfigSection( + ctx context.Context, + in *azdext.SetServiceConfigSectionRequest, + opts ...grpc.CallOption, + ) (*azdext.EmptyResponse, error) +} + +func upsertSkillServiceToProject( + ctx context.Context, + declaration skillServiceDeclaration, +) (*skillServiceUpsertResult, error) { + azdClient, err := azdext.NewAzdClient() + if err != nil { + return nil, fmt.Errorf("create azd client to update azure.yaml: %w", err) + } + defer azdClient.Close() + + return upsertSkillService(ctx, azdClient.Project(), declaration) +} + +func upsertSkillService( + ctx context.Context, + projectClient skillProjectClient, + declaration skillServiceDeclaration, +) (*skillServiceUpsertResult, error) { + projectResponse, err := projectClient.Get(ctx, &azdext.EmptyRequest{}) + if err != nil { + return nil, exterrors.Dependency( + exterrors.CodeProjectManifestNotFound, + fmt.Sprintf("cannot add skill %q to azure.yaml: %s", declaration.Name, err), + "run this command from an azd project containing azure.yaml", + ) + } + project := projectResponse.GetProject() + if project == nil || strings.TrimSpace(project.GetPath()) == "" { + return nil, exterrors.Dependency( + exterrors.CodeProjectManifestNotFound, + fmt.Sprintf("cannot add skill %q to azure.yaml: no azd project is loaded", declaration.Name), + "run this command from an azd project containing azure.yaml", + ) + } + + existing, found := project.GetServices()[declaration.Name] + if found && existing.GetHost() != aiSkillHost { + return nil, exterrors.Validation( + exterrors.CodeSkillServiceConflict, + fmt.Sprintf( + "cannot add skill %q to azure.yaml: service %q already uses host %q", + declaration.Name, + declaration.Name, + existing.GetHost(), + ), + "choose a different skill name or rename the existing azure.yaml service", + ) + } + + cfg := declaration.Config + if declaration.ArchiveSource != "" { + serviceRoot := project.GetPath() + if found { + serviceRoot = skillServiceRoot(project.GetPath(), existing) + } + archiveReference, err := portableSkillArchiveReference(serviceRoot, declaration.ArchiveSource) + if err != nil { + return nil, err + } + + archive, err := prepareSkillArchive(declaration.ArchiveSource) + if err != nil { + return nil, err + } + if err := archive.Reader.Close(); err != nil { + return nil, fmt.Errorf("close prepared skill archive: %w", err) + } + cfg.Archive = archiveReference + } + if err := validateSkillServiceConfig(declaration.Name, &cfg); err != nil { + return nil, err + } + + cfgMap, err := skillServiceConfigMap(cfg) + if err != nil { + return nil, fmt.Errorf("encode skill service %q: %w", declaration.Name, err) + } + cfgStruct, err := structpb.NewStruct(cfgMap) + if err != nil { + return nil, fmt.Errorf("encode skill service %q: %w", declaration.Name, err) + } + + if !found { + if _, err := projectClient.AddService(ctx, &azdext.AddServiceRequest{ + Service: &azdext.ServiceConfig{ + Name: declaration.Name, + Host: aiSkillHost, + AdditionalProperties: cfgStruct, + }, + }); err != nil { + return nil, fmt.Errorf("add azure.ai.skill service %q: %w", declaration.Name, err) + } + return &skillServiceUpsertResult{ + Name: declaration.Name, + Host: aiSkillHost, + ProjectPath: project.GetPath(), + Created: true, + }, nil + } + + sectionResponse, err := projectClient.GetServiceConfigSection( + ctx, + &azdext.GetServiceConfigSectionRequest{ServiceName: declaration.Name}, + ) + if err != nil { + return nil, fmt.Errorf("read azure.ai.skill service %q from azure.yaml: %w", declaration.Name, err) + } + if !sectionResponse.GetFound() || sectionResponse.GetSection() == nil { + return nil, fmt.Errorf("azure.ai.skill service %q disappeared from azure.yaml", declaration.Name) + } + + merged := sectionResponse.GetSection().AsMap() + for _, field := range skillServiceOwnedFields { + delete(merged, field) + } + maps.Copy(merged, cfgMap) + merged["host"] = aiSkillHost + + section, err := structpb.NewStruct(merged) + if err != nil { + return nil, fmt.Errorf("encode updated skill service %q: %w", declaration.Name, err) + } + if _, err := projectClient.SetServiceConfigSection(ctx, &azdext.SetServiceConfigSectionRequest{ + ServiceName: declaration.Name, + Section: section, + }); err != nil { + return nil, fmt.Errorf("update azure.ai.skill service %q in azure.yaml: %w", declaration.Name, err) + } + + return &skillServiceUpsertResult{ + Name: declaration.Name, + Host: aiSkillHost, + ProjectPath: project.GetPath(), + }, nil +} + +func skillServiceConfigMap(cfg skillServiceConfig) (map[string]any, error) { + data, err := json.Marshal(cfg) + if err != nil { + return nil, err + } + var values map[string]any + if err := json.Unmarshal(data, &values); err != nil { + return nil, err + } + return values, nil +} + +func portableSkillArchiveReference(serviceRoot, source string) (string, error) { + if strings.TrimSpace(serviceRoot) == "" { + return "", fmt.Errorf("service directory is empty") + } + + rootAbs, err := filepath.Abs(serviceRoot) + if err != nil { + return "", fmt.Errorf("resolve service directory %q: %w", serviceRoot, err) + } + sourceAbs, err := filepath.Abs(source) + if err != nil { + return "", fmt.Errorf("resolve skill archive path %q: %w", source, err) + } + + rootReal, err := filepath.EvalSymlinks(rootAbs) + if err != nil { + return "", exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("cannot resolve skill service directory %q: %s", serviceRoot, err), + "verify the service project path exists and is readable", + ) + } + sourceReal, err := filepath.EvalSymlinks(sourceAbs) + if err != nil { + return "", exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("cannot resolve skill archive path %q: %s", source, err), + "verify the archive or directory exists and is readable", + ) + } + + relative, err := filepath.Rel(rootAbs, sourceAbs) + if err != nil { + return "", exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("cannot make skill archive path %q portable: %s", source, err), + "move the skill archive or directory onto the same volume as the skill service directory", + ) + } + resolvedRelative, err := filepath.Rel(rootReal, sourceReal) + if err != nil { + return "", exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("cannot verify skill archive path %q: %s", source, err), + "move the skill archive or directory onto the same volume as the skill service directory", + ) + } + if pathEscapesBase(relative) || pathEscapesBase(resolvedRelative) { + return "", exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf( + "cannot add archive %q to azure.yaml because it is outside the skill service directory at %q", + source, + serviceRoot, + ), + "move the skill archive or directory inside the skill service directory and retry", + ) + } + if relative == "." || relative == "" { + return "", exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("cannot use the skill service directory %q itself as the archive source", serviceRoot), + "place the skill in a child directory or .zip file and retry", + ) + } + + return filepath.ToSlash(resolvedRelative), nil +} + +func pathEscapesBase(relative string) bool { + return filepath.IsAbs(relative) || + relative == ".." || + strings.HasPrefix(relative, ".."+string(os.PathSeparator)) +} diff --git a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_service_config_test.go b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_service_config_test.go new file mode 100644 index 00000000000..cbd6de21f8d --- /dev/null +++ b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_service_config_test.go @@ -0,0 +1,383 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/protobuf/types/known/structpb" +) + +type recordingSkillProjectClient struct { + project *azdext.ProjectConfig + section map[string]any + getErr error + addRequest *azdext.AddServiceRequest + setRequest *azdext.SetServiceConfigSectionRequest +} + +func (c *recordingSkillProjectClient) Get( + context.Context, + *azdext.EmptyRequest, + ...grpc.CallOption, +) (*azdext.GetProjectResponse, error) { + if c.getErr != nil { + return nil, c.getErr + } + return &azdext.GetProjectResponse{Project: c.project}, nil +} + +func (c *recordingSkillProjectClient) AddService( + _ context.Context, + request *azdext.AddServiceRequest, + _ ...grpc.CallOption, +) (*azdext.EmptyResponse, error) { + c.addRequest = request + return &azdext.EmptyResponse{}, nil +} + +func (c *recordingSkillProjectClient) GetServiceConfigSection( + context.Context, + *azdext.GetServiceConfigSectionRequest, + ...grpc.CallOption, +) (*azdext.GetServiceConfigSectionResponse, error) { + if c.section == nil { + return &azdext.GetServiceConfigSectionResponse{}, nil + } + section, err := structpb.NewStruct(c.section) + if err != nil { + return nil, err + } + return &azdext.GetServiceConfigSectionResponse{ + Found: true, + Section: section, + }, nil +} + +func (c *recordingSkillProjectClient) SetServiceConfigSection( + _ context.Context, + request *azdext.SetServiceConfigSectionRequest, + _ ...grpc.CallOption, +) (*azdext.EmptyResponse, error) { + c.setRequest = request + return &azdext.EmptyResponse{}, nil +} + +func TestUpsertSkillService_AddsInlineService(t *testing.T) { + t.Parallel() + + client := &recordingSkillProjectClient{ + project: &azdext.ProjectConfig{ + Path: t.TempDir(), + Services: map[string]*azdext.ServiceConfig{}, + }, + } + result, err := upsertSkillService(t.Context(), client, skillServiceDeclaration{ + Name: "code-review", + Config: skillServiceConfig{ + Description: "Review code", + Instructions: "Review for correctness.", + License: "MIT", + Compatibility: "gpt-5", + Metadata: map[string]string{"owner": "platform"}, + Tools: []string{"code_interpreter"}, + }, + }) + require.NoError(t, err) + require.NotNil(t, client.addRequest) + assert.Nil(t, client.setRequest) + assert.True(t, result.Created) + assert.Equal(t, "code-review", result.Name) + + service := client.addRequest.GetService() + assert.Equal(t, aiSkillHost, service.GetHost()) + assert.Equal(t, map[string]any{ + "compatibility": "gpt-5", + "description": "Review code", + "instructions": "Review for correctness.", + "license": "MIT", + "metadata": map[string]any{"owner": "platform"}, + "tools": []any{"code_interpreter"}, + }, service.GetAdditionalProperties().AsMap()) +} + +func TestUpsertSkillService_UpdatesOwnedFieldsAndPreservesOthers(t *testing.T) { + t.Parallel() + + client := &recordingSkillProjectClient{ + project: &azdext.ProjectConfig{ + Path: t.TempDir(), + Services: map[string]*azdext.ServiceConfig{ + "code-review": { + Name: "code-review", + Host: aiSkillHost, + RelativePath: "skills", + Uses: []string{"review-tools"}, + }, + }, + }, + section: map[string]any{ + "host": aiSkillHost, + "project": "skills", + "uses": []any{"review-tools"}, + "archive": "old.zip", + "custom": "preserve-me", + }, + } + result, err := upsertSkillService(t.Context(), client, skillServiceDeclaration{ + Name: "code-review", + Config: skillServiceConfig{ + Description: "Updated review", + Instructions: "Review new code.", + }, + }) + require.NoError(t, err) + assert.Nil(t, client.addRequest) + require.NotNil(t, client.setRequest) + assert.False(t, result.Created) + + updated := client.setRequest.GetSection().AsMap() + assert.Equal(t, aiSkillHost, updated["host"]) + assert.Equal(t, "skills", updated["project"]) + assert.Equal(t, []any{"review-tools"}, updated["uses"]) + assert.Equal(t, "preserve-me", updated["custom"]) + assert.Equal(t, "Updated review", updated["description"]) + assert.Equal(t, "Review new code.", updated["instructions"]) + assert.NotContains(t, updated, "archive") +} + +func TestUpsertSkillService_SavesPortableArchiveReference(t *testing.T) { + t.Parallel() + + projectRoot := t.TempDir() + source := filepath.Join(projectRoot, "skills", "code-review") + writeSkillFiles(t, source) + client := &recordingSkillProjectClient{ + project: &azdext.ProjectConfig{ + Path: projectRoot, + Services: map[string]*azdext.ServiceConfig{}, + }, + } + + _, err := upsertSkillService(t.Context(), client, skillServiceDeclaration{ + Name: "code-review", + ArchiveSource: source, + }) + require.NoError(t, err) + assert.Equal( + t, + "skills/code-review", + client.addRequest.GetService().GetAdditionalProperties().AsMap()["archive"], + ) +} + +func TestUpsertSkillService_SavesArchiveRelativeToServicePath(t *testing.T) { + t.Parallel() + + projectRoot := t.TempDir() + source := filepath.Join(projectRoot, "skills", "code-review") + writeSkillFiles(t, source) + client := &recordingSkillProjectClient{ + project: &azdext.ProjectConfig{ + Path: projectRoot, + Services: map[string]*azdext.ServiceConfig{ + "code-review": { + Name: "code-review", + Host: aiSkillHost, + RelativePath: "skills", + }, + }, + }, + section: map[string]any{ + "host": aiSkillHost, + "project": "skills", + }, + } + + _, err := upsertSkillService(t.Context(), client, skillServiceDeclaration{ + Name: "code-review", + ArchiveSource: source, + }) + require.NoError(t, err) + updated := client.setRequest.GetSection().AsMap() + assert.Equal(t, "code-review", updated["archive"]) + assert.Equal(t, "skills", updated["project"]) +} + +func TestUpsertSkillService_SwitchesInlineServiceToArchive(t *testing.T) { + t.Parallel() + + projectRoot := t.TempDir() + source := filepath.Join(projectRoot, "skills", "code-review") + writeSkillFiles(t, source) + client := &recordingSkillProjectClient{ + project: &azdext.ProjectConfig{ + Path: projectRoot, + Services: map[string]*azdext.ServiceConfig{ + "code-review": { + Name: "code-review", + Host: aiSkillHost, + }, + }, + }, + section: map[string]any{ + "host": aiSkillHost, + "description": "Old description", + "instructions": "Old instructions", + "tools": []any{"code_interpreter"}, + "custom": "preserve-me", + }, + } + + _, err := upsertSkillService(t.Context(), client, skillServiceDeclaration{ + Name: "code-review", + ArchiveSource: source, + }) + require.NoError(t, err) + updated := client.setRequest.GetSection().AsMap() + assert.Equal(t, "skills/code-review", updated["archive"]) + assert.Equal(t, "preserve-me", updated["custom"]) + assert.NotContains(t, updated, "description") + assert.NotContains(t, updated, "instructions") + assert.NotContains(t, updated, "tools") +} + +func TestUpsertSkillService_RejectsArchiveOutsideServicePath(t *testing.T) { + t.Parallel() + + projectRoot := t.TempDir() + outside := filepath.Join(t.TempDir(), "code-review") + writeSkillFiles(t, outside) + client := &recordingSkillProjectClient{ + project: &azdext.ProjectConfig{ + Path: projectRoot, + Services: map[string]*azdext.ServiceConfig{}, + }, + } + + _, err := upsertSkillService(t.Context(), client, skillServiceDeclaration{ + Name: "code-review", + ArchiveSource: outside, + }) + require.ErrorContains(t, err, "outside the skill service directory") + assert.Nil(t, client.addRequest) +} + +func TestUpsertSkillService_RejectsServiceDirectoryAsArchive(t *testing.T) { + t.Parallel() + + projectRoot := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(projectRoot, "SKILL.md"), + []byte("---\nname: code-review\ndescription: Review code\n---\nReview."), + 0600, + )) + client := &recordingSkillProjectClient{ + project: &azdext.ProjectConfig{ + Path: projectRoot, + Services: map[string]*azdext.ServiceConfig{}, + }, + } + + _, err := upsertSkillService(t.Context(), client, skillServiceDeclaration{ + Name: "code-review", + ArchiveSource: projectRoot, + }) + require.ErrorContains(t, err, "service directory") + assert.Nil(t, client.addRequest) +} + +func TestUpsertSkillService_RejectsHostConflict(t *testing.T) { + t.Parallel() + + client := &recordingSkillProjectClient{ + project: &azdext.ProjectConfig{ + Path: t.TempDir(), + Services: map[string]*azdext.ServiceConfig{ + "code-review": { + Name: "code-review", + Host: "containerapp", + }, + }, + }, + } + _, err := upsertSkillService(t.Context(), client, skillServiceDeclaration{ + Name: "code-review", + Config: skillServiceConfig{Instructions: "Review code."}, + }) + require.ErrorContains(t, err, "already uses host") + assert.Nil(t, client.addRequest) + assert.Nil(t, client.setRequest) +} + +func TestUpsertSkillService_DoesNotReplaceMissingExistingSection(t *testing.T) { + t.Parallel() + + client := &recordingSkillProjectClient{ + project: &azdext.ProjectConfig{ + Path: t.TempDir(), + Services: map[string]*azdext.ServiceConfig{ + "code-review": { + Name: "code-review", + Host: aiSkillHost, + }, + }, + }, + } + + _, err := upsertSkillService(t.Context(), client, skillServiceDeclaration{ + Name: "code-review", + Config: skillServiceConfig{Instructions: "Review code."}, + }) + require.ErrorContains(t, err, "disappeared from azure.yaml") + assert.Nil(t, client.setRequest) +} + +func TestUpsertSkillService_RequiresProject(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + client *recordingSkillProjectClient + }{ + { + name: "project lookup fails", + client: &recordingSkillProjectClient{getErr: errors.New("not found")}, + }, + { + name: "project missing", + client: &recordingSkillProjectClient{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + _, err := upsertSkillService(t.Context(), tt.client, skillServiceDeclaration{ + Name: "code-review", + Config: skillServiceConfig{Instructions: "Review code."}, + }) + require.ErrorContains(t, err, "cannot add skill") + }) + } +} + +func writeSkillFiles(t *testing.T, dir string) { + t.Helper() + require.NoError(t, os.MkdirAll(dir, 0750)) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "SKILL.md"), + []byte("---\nname: code-review\ndescription: Review code\n---\nReview."), + 0600, + )) +} diff --git a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_update.go b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_update.go index 04762c791a0..1a82acde370 100644 --- a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_update.go +++ b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_update.go @@ -216,17 +216,9 @@ func (a *updateAction) buildInlineContent() (*skill_api.SkillInlineContent, erro } if a.flags.file != "" { - data, readErr := readFileWithLimit(a.flags.file) - if readErr != nil { - return nil, readErr - } - parsed, parseErr := skill_api.ParseSkillMd(data) - if parseErr != nil { - return nil, exterrors.Validation( - exterrors.CodeInvalidSkillFile, - fmt.Sprintf("failed to parse %s: %s", a.flags.file, parseErr), - "ensure the file begins with a YAML front matter block delimited by '---'", - ) + parsed, err := loadSkillMd(a.flags.file) + if err != nil { + return nil, err } content.Description = parsed.Description content.Instructions = parsed.Instructions diff --git a/cli/azd/extensions/azure.ai.skills/internal/exterrors/codes.go b/cli/azd/extensions/azure.ai.skills/internal/exterrors/codes.go index 393582848b0..c44ac6a4239 100644 --- a/cli/azd/extensions/azure.ai.skills/internal/exterrors/codes.go +++ b/cli/azd/extensions/azure.ai.skills/internal/exterrors/codes.go @@ -13,11 +13,13 @@ const ( // Codes shared across the extension surface. const ( - CodeConflictingArguments = "conflicting_arguments" - CodeInvalidParameter = "invalid_parameter" - CodeMissingProjectEndpoint = "missing_project_endpoint" - CodeMissingRequiredField = "missing_required_field" - CodeMissingForceFlag = "missing_force_flag" + CodeConflictingArguments = "conflicting_arguments" + CodeInvalidParameter = "invalid_parameter" + CodeMissingProjectEndpoint = "missing_project_endpoint" + CodeMissingRequiredField = "missing_required_field" + CodeMissingForceFlag = "missing_force_flag" + CodeProjectManifestNotFound = "project_manifest_not_found" + CodeSkillServiceConflict = "skill_service_conflict" ) const ( diff --git a/schemas/alpha/azure.yaml.json b/schemas/alpha/azure.yaml.json index d103b88fb2a..227e8f010a2 100644 --- a/schemas/alpha/azure.yaml.json +++ b/schemas/alpha/azure.yaml.json @@ -518,7 +518,7 @@ } }, { - "comment": "Azure AI Foundry skill host - code-less resource service; the service key is the skill name", + "comment": "Azure AI Foundry skill host - resource service; project optionally scopes file and archive paths", "if": { "properties": { "host": { "const": "azure.ai.skill" } @@ -529,7 +529,6 @@ { "$ref": "https://raw.githubusercontent.com/Azure/azure-dev/main/cli/azd/extensions/azure.ai.skills/schemas/azure.ai.skill.json" } ], "properties": { - "project": false, "runtime": false, "docker": false, "image": false, diff --git a/schemas/v1.0/azure.yaml.json b/schemas/v1.0/azure.yaml.json index f7a7c9b5895..7a8be7e7b1f 100644 --- a/schemas/v1.0/azure.yaml.json +++ b/schemas/v1.0/azure.yaml.json @@ -478,7 +478,7 @@ } }, { - "comment": "Azure AI Foundry skill host - code-less resource service; the service key is the skill name", + "comment": "Azure AI Foundry skill host - resource service; project optionally scopes file and archive paths", "if": { "properties": { "host": { "const": "azure.ai.skill" } @@ -489,7 +489,6 @@ { "$ref": "https://raw.githubusercontent.com/Azure/azure-dev/main/cli/azd/extensions/azure.ai.skills/schemas/azure.ai.skill.json" } ], "properties": { - "project": false, "runtime": false, "docker": false, "image": false,