feat(skills): own azure.ai.skill service blocks in azure.yaml#9116
feat(skills): own azure.ai.skill service blocks in azure.yaml#9116glharper wants to merge 2 commits into
Conversation
Add an opt-in --save-to-azure-yaml flow to skill create/update, preserving existing uses and unrelated service fields during idempotent updates. Reconcile inline content or project-relative ZIP/directory archive references through the existing service target, and recognize azure.ai.skill in the agents adoption scan. Fixes #9088 Co-authored-by: Copilot <[email protected]> Copilot-Session: 3aee4aec-260c-4b09-8748-212aa891d664
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
There was a problem hiding this comment.
Pull request overview
Adds azure.ai.skill service ownership to the skills extension, including manifest authoring and deployment reconciliation.
Changes:
- Adds opt-in
azure.yamlpersistence for create/update commands. - Supports inline and archive-based skill reconciliation.
- Recognizes skill services during agents adoption.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
schemas/azure.ai.skill.json |
Defines archive configuration. |
README.md |
Documents skill composition. |
internal/exterrors/codes.go |
Adds manifest/reconcile error codes. |
internal/cmd/skill_update.go |
Adds manifest persistence to updates. |
internal/cmd/skill_update_test.go |
Tests update flag validation. |
internal/cmd/skill_service_config.go |
Implements service upsert logic. |
internal/cmd/skill_service_config_test.go |
Tests service authoring. |
internal/cmd/skill_create.go |
Adds manifest persistence to creation. |
internal/cmd/skill_create_test.go |
Tests create flag registration. |
internal/cmd/service_target.go |
Reconciles archive declarations. |
internal/cmd/service_target_test.go |
Tests archive parsing/preparation. |
azure.ai.agents/internal/cmd/init_adopt.go |
Recognizes skill hosts. |
azure.ai.agents/internal/cmd/init_adopt_test.go |
Tests skill-only adoption detection. |
|
|
||
| cfg := declaration.Config | ||
| if declaration.ArchiveSource != "" { | ||
| archive, err := portableSkillArchiveReference(project.GetPath(), declaration.ArchiveSource) |
| baseDir := svc.GetRelativePath() | ||
| if baseDir == "" { | ||
| baseDir = "." | ||
| } | ||
| return filepath.Join(baseDir, path), nil |
| if err := a.saveService(ctx, skillServiceConfig{ | ||
| Description: a.flags.description, | ||
| Instructions: a.flags.instructions, | ||
| }, ""); err != nil { | ||
| return err |
| if err := a.saveService(ctx, skillServiceConfig{ | ||
| Description: content.Description, | ||
| Instructions: content.Instructions, | ||
| Tools: content.AllowedTools, | ||
| }, ""); err != nil { | ||
| return err |
jongio
left a comment
There was a problem hiding this comment.
Traced the archive vs inline validation, the field-preserving upsert merge (drops stale archive on an inline switch while keeping uses: and unknown fields), and the path handling: portableSkillArchiveReference rejects out-of-project sources and resolveSkillArchivePath/hasParentTraversal block .. escapes. The save-time project-root-relative vs deploy-time service-relative resolution lines up with the existing resolveSkillInstructions convention and the schema doc. The isInstructionFilePath multiline guard is right and covered by a regression test. Tests cover add/update, host conflict, missing project, traversal, outside-project, zip/dir prep, and the --set-default-version conflict.
Co-authored-by: glharper <[email protected]>
jongio
left a comment
There was a problem hiding this comment.
Incremental review of the preflight-persistence commit. Splitting the azure.yaml save into prepare + save runs validation before the Azure delete/create, so a manifest conflict no longer leaves the service mutated with azure.yaml unwritten. Force-delete relocation covers all four create modes, and anchoring archive resolution to the project path (with the traversal guard retained) fixes the nested-working-directory case. Tests, vet, and go fix are clean.
trangevi
left a comment
There was a problem hiding this comment.
I'd like to wait on the "authoring" piece of the corresponding issue until John is back from vacation; there needs to be further discussion about how we introduce those pieces. For now, if we want to make progress on this work item and the others like it, we should focus on provision and deploy surface areas, and leave any authoring pieces that may exist within the agents extension for the time being.
| cmd.Flags().StringVar(&flags.file, "file", "", | ||
| "Path to SKILL.md (.md), a ZIP package (.zip), or a directory containing SKILL.md at its root") | ||
| cmd.Flags().BoolVar(&flags.force, "force", false, "Delete an existing skill of the same name before creating") | ||
| cmd.Flags().BoolVar( |
There was a problem hiding this comment.
I'd like John to weigh in on a parameter like this when he is back from vacation
Summary
Gives the
azure.ai.skillsextension ownership of itshost: azure.ai.skillservice blocks inazure.yaml: command-driven authoring, idempotent updates, reading, and deploy-time reconciliation.Developers can now opt a skill into the current azd project directly from the owning extension:
This has no dependency on the agents extension. An agent that consumes the skill declares the dependency using the existing
uses:field.Fixes #9088
What changed
azure.ai.skillsowns authoring and updates--save-to-azure-yamltoskill createandskill update(opt-in; existing remote-only behavior remains the default).Project.Getto locate/read the current project and existing service.AddServiceto create a newhost: azure.ai.skillblock.GetServiceConfigSection+ read/merge/SetServiceConfigSectionto update an existing block while preservinguses:,project, and unrelated/unknown fields.--set-default-versionis rejected with--save-to-azure-yamlbecause a default-version pointer is not a declarative content source thatazd deploycan reconcile.Inline and archive declarations
Inline flags and parsed
SKILL.mdinputs are saved as service-level content:ZIP and directory inputs are saved as an archive reference:
The existing skill service target now reads and reconciles both shapes. Archive reconciliation:
.zipfile or a directory containing root-levelSKILL.md.ArchiveDirectorysafety limits for directories (no symlinks/non-regular entries; entry/size limits).The runtime validation keeps the pre-split legacy
config:fallback compatible while enforcing that archive and inline fields are not combined.Composition/adoption
azure.ai.skillto the agents extension's generic Foundry host recognition, so a skill-only or composed unifiedazure.yamlround-trips through the adoption flow.uses:list.Tests
uses:and unknown fields.--save-to-azure-yamlflag registration and--set-default-versionconflict..md/.txt(must not be misclassified as a file path).Validation run:
go build ./...andgo test ./...inazure.ai.skillsgolangci-lint run ./...and extension cspell inazure.ai.skillsinternal/cmdtest/lint and cspell for the adoption changegit diff --check