diff --git a/cli/azd/extensions/azure.ai.skills/AGENTS.md b/cli/azd/extensions/azure.ai.skills/AGENTS.md index b57e9ca0799..f2680e10277 100644 --- a/cli/azd/extensions/azure.ai.skills/AGENTS.md +++ b/cli/azd/extensions/azure.ai.skills/AGENTS.md @@ -65,7 +65,7 @@ Each `--debug` run writes to `azd-ai-skills-.log` in the current working d - `--file` is **not** a manifest. It is read at invocation time only; the CLI does not track or re-read it after the command returns. - `create`: accepts `.md`, `.zip`, or a **directory** whose root contains `SKILL.md`. Mode is inferred from the path: directories take precedence over extension matching so callers can hand the output of `azd ai skill download` straight back. Conflicting modes (inline + `--file`) are rejected. `.md` and inline modes send `inline_content` JSON; `.zip` and directory modes upload `multipart/form-data` with a single `files[]` part. Directory mode packages the directory into an in-memory zip using `skill_api.ArchiveDirectory`, which enforces the same safety caps as `SafeExtract` on the way down (no symlinks / non-regular entries, 10,000-entry / 512 MB total cap). -- `update`: accepts `.md` only. `.zip` and directories are rejected with a structured suggestion to use `create --force` (which deletes the skill and all its versions before re-creating). Pass `--set-default-version ` to repoint `default_version` at an existing immutable version without uploading new content. +- `update`: accepts `.md`, `.zip`, or a **directory** whose root contains `SKILL.md` — the same three shapes as `create`, routed through the same `selectUpdateMode` mode dispatch. `update` is non-destructive: every upload (inline / md / zip / directory) creates a new immutable version via `POST /skills/{name}/versions` and promotes it to `default_version`; prior versions remain reachable. Conflicting modes (inline + `--file`) are rejected. Pass `--set-default-version ` to repoint `default_version` at an existing immutable version without uploading new content. Unlike `create`, `update` has no `--force` flag because there is nothing to delete. - `download`: writes either an extracted directory (default) or the unmodified zip archive (`--raw`). Pass `--version ` to download a non-default version. The server always returns `application/zip` (from `GET /skills/{name}/content` or `GET /skills/{name}/versions/{version}/content`). ## Versioning diff --git a/cli/azd/extensions/azure.ai.skills/README.md b/cli/azd/extensions/azure.ai.skills/README.md index 2e07f9fcf12..cec693efdb3 100644 --- a/cli/azd/extensions/azure.ai.skills/README.md +++ b/cli/azd/extensions/azure.ai.skills/README.md @@ -13,6 +13,8 @@ azd ai skill create --file ./skill.zip azd ai skill create --file ./skill-src/ azd ai skill update [--description "..."] [--instructions "..."] [--file ./SKILL.md] +azd ai skill update --file ./skill.zip +azd ai skill update --file ./skill-src/ azd ai skill update --set-default-version azd ai skill show azd ai skill list [--top N] [--orderby ] @@ -32,9 +34,15 @@ a `SKILL.md`. Directory mode is the round-trip inverse of `azd ai skill download`: the CLI packages the directory as a zip in memory and uploads it as multipart/form-data, identical to the `.zip` path. -`update` is inline-only: a `.zip` or a directory is rejected with a pointer -to `azd ai skill create --force` (a destructive delete-then-create), since -swapping the entire package shape is a `create` concern, not an `update`. +`update` is non-destructive and accepts the same four input shapes as +`create` (inline content, `SKILL.md`, `.zip`, or a directory whose root +contains `SKILL.md`). Every upload creates a new immutable version and +promotes it to `default_version`; prior versions remain reachable via +`--set-default-version ` or `azd ai skill download --version `. +For directories, the CLI packages the folder as a zip in memory and +uploads it as multipart/form-data, so the output of +`azd ai skill download` round-trips back through `update` without a +manual zip step. All commands accept the standard cross-cutting flags: `-p` / `--project-endpoint`, `--output table|json`, `--no-prompt`, and `--debug`. 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 9dc382c6b9e..a58599f85d8 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 @@ -4,6 +4,7 @@ package cmd import ( + "bytes" "context" "fmt" "os" @@ -36,11 +37,28 @@ type updateFlags struct { type updateAction struct{ flags *updateFlags } +// updateMode is the dispatch tag selectUpdateMode returns. It mirrors +// createMode so the two commands route the same shapes through the same +// helpers (md/zip/dir) — the difference is that `update` always appends a +// non-destructive new version via POST /skills/{name}/versions and never +// deletes the existing skill. +type updateMode int + +const ( + updateModeNone updateMode = iota + updateModeSetDefault + updateModeInline + updateModeFileMd + updateModeFilePackage + updateModeFileDirectory +) + func (a *updateAction) Run(ctx context.Context) error { if err := validateSkillName(a.flags.name); err != nil { return err } - if err := a.validateFlags(); err != nil { + mode, err := selectUpdateMode(a.flags) + if err != nil { return err } @@ -49,39 +67,142 @@ func (a *updateAction) Run(ctx context.Context) error { return err } - // --set-default-version is a metadata-only update: POST /skills/{name} - // with { default_version }. No new version is created. - if a.flags.setDefault != "" { - updated, err := skillCtx.client.UpdateSkillDefaultVersion(ctx, a.flags.name, a.flags.setDefault) - if err != nil { - return exterrors.ServiceFromAzure(err, exterrors.OpUpdateSkill) - } - if a.flags.output == outputJSON { - return printJSON(updated) - } - fmt.Printf("Skill %q default_version set to %q.\n", updated.Name, updated.DefaultVersion) - return printSkillDetail(updated, outputTable) + switch mode { + case updateModeSetDefault: + return a.runSetDefault(ctx, skillCtx.client) + case updateModeInline, updateModeFileMd: + return a.runInline(ctx, skillCtx.client) + case updateModeFilePackage: + return a.runFilePackage(ctx, skillCtx.client) + case updateModeFileDirectory: + return a.runFileDirectory(ctx, skillCtx.client) } + return exterrors.Validation( + exterrors.CodeInvalidParameter, + "unsupported update mode", + "this is a bug; please file an issue", + ) +} +// runSetDefault is a metadata-only update: POST /skills/{name} with +// { default_version }. No new version is created. +func (a *updateAction) runSetDefault(ctx context.Context, client *skill_api.Client) error { + updated, err := client.UpdateSkillDefaultVersion(ctx, a.flags.name, a.flags.setDefault) + if err != nil { + return exterrors.ServiceFromAzure(err, exterrors.OpUpdateSkill) + } + if a.flags.output == outputJSON { + return printJSON(updated) + } + fmt.Printf("Skill %q default_version set to %q.\n", updated.Name, updated.DefaultVersion) + return printSkillDetail(updated, outputTable) +} + +// runInline handles both pure-inline (--description/--instructions) and +// SKILL.md (--file *.md) updates: parse content, POST inline_content JSON. +func (a *updateAction) runInline(ctx context.Context, client *skill_api.Client) error { content, err := a.buildInlineContent() if err != nil { return err } - version, err := skillCtx.client.CreateVersionInline(ctx, a.flags.name, skill_api.CreateVersionRequest{ + version, err := client.CreateVersionInline(ctx, a.flags.name, skill_api.CreateVersionRequest{ InlineContent: content, Default: true, }) if err != nil { return exterrors.ServiceFromAzure(err, exterrors.OpUpdateSkill) } + return a.printUpdateResult(ctx, client, version) +} + +// runFilePackage uploads a single .zip archive as the next default version +// via multipart/form-data. Mirrors createAction.runFilePackage but wraps +// errors with OpUpdateSkill and uses printUpdateResult. +func (a *updateAction) runFilePackage(ctx context.Context, client *skill_api.Client) error { + info, statErr := os.Stat(a.flags.file) + if statErr != nil { + return exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("cannot stat %s: %s", a.flags.file, statErr), + "verify the path exists and is readable", + ) + } + if info.IsDir() { + return exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("--file %s is a directory; expected a .zip archive", a.flags.file), + "pass a single .zip archive path", + ) + } + + f, openErr := os.Open(a.flags.file) //nolint:gosec // user-supplied path opened on user's behalf + if openErr != nil { + return exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("cannot open %s: %s", a.flags.file, openErr), + "verify the file is readable", + ) + } + defer f.Close() + + version, err := client.CreateVersionFromZip(ctx, a.flags.name, filepath.Base(a.flags.file), f, true) + if err != nil { + return exterrors.ServiceFromAzure(err, exterrors.OpUpdateSkill) + } + return a.printUpdateResult(ctx, client, version) +} + +// runFileDirectory packages the directory as an in-memory zip and uploads +// it via the same multipart path as runFilePackage. The directory must +// contain a SKILL.md at its root (matching what `azd ai skill download` +// writes by default), so the natural download → edit → update flow works +// without any manual zip step. Mirrors createAction.runFileDirectory but +// wraps errors with OpUpdateSkill and uses printUpdateResult. +func (a *updateAction) runFileDirectory(ctx context.Context, client *skill_api.Client) error { + if _, found, err := skill_api.LocateSkillMdInDir(a.flags.file); err != nil { + return exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("cannot inspect SKILL.md in %s: %s", a.flags.file, err), + "verify the directory is readable and SKILL.md is a regular file", + ) + } else if !found { + return exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("--file %s is a directory without a SKILL.md at its root", a.flags.file), + "add a SKILL.md to the directory root (matches `azd ai skill download` output) or pass a .zip archive", + ) + } + data, archiveErr := skill_api.ArchiveDirectory(a.flags.file, skill_api.ArchiveOptions{}) + if archiveErr != nil { + return classifyArchiveDirectoryError(archiveErr, a.flags.file) + } + + archiveName := filepath.Base(filepath.Clean(a.flags.file)) + ".zip" + version, err := client.CreateVersionFromZip(ctx, a.flags.name, archiveName, bytes.NewReader(data), true) + if err != nil { + return exterrors.ServiceFromAzure(err, exterrors.OpUpdateSkill) + } + return a.printUpdateResult(ctx, client, version) +} + +// printUpdateResult prints either the created version envelope (JSON) or, +// for human output, a friendly "updated" message followed by the freshly +// loaded Skill so users see the new default_version / latest_version. +// +// Critically, when --output json is in effect we ONLY emit the version +// envelope — never a human-readable line — so the output stays valid JSON +// for callers piping into jq or similar. +func (a *updateAction) printUpdateResult(ctx context.Context, client *skill_api.Client, version *skill_api.SkillVersion) error { if a.flags.output == outputJSON { return printJSON(version) } fmt.Printf("Skill %q updated; new version %q is now the default.\n", a.flags.name, version.Version) - skill, err := skillCtx.client.GetSkill(ctx, a.flags.name) + skill, err := client.GetSkill(ctx, a.flags.name) if err != nil { + // Don't fail the update just because the follow-up GET failed; fall + // back to printing the version envelope instead. return printSkillVersionDetail(version, outputTable) } return printSkillDetail(skill, outputTable) @@ -128,32 +249,48 @@ func (a *updateAction) buildInlineContent() (*skill_api.SkillInlineContent, erro return content, nil } +// validateFlags is kept for tests that exercise validation in isolation. It +// delegates to selectUpdateMode (which performs the same checks plus mode +// inference) and discards the mode. Production code calls selectUpdateMode +// directly via Run. func (a *updateAction) validateFlags() error { - inlineProvided := a.flags.descriptionSet || a.flags.instructionsSet - fileProvided := a.flags.file != "" - setDefaultProvided := a.flags.setDefault != "" + _, err := selectUpdateMode(a.flags) + return err +} - // --set-default-version is mutually exclusive with content flags. +// selectUpdateMode validates flag combinations and infers the dispatch +// mode. It mirrors selectCreateMode in skill_create.go so the same input +// shapes (.md / .zip / directory / inline) route the same way on update, +// with the additional --set-default-version branch that is unique to +// update. +func selectUpdateMode(f *updateFlags) (updateMode, error) { + inlineProvided := f.descriptionSet || f.instructionsSet + fileProvided := f.file != "" + setDefaultProvided := f.setDefault != "" + + // --set-default-version is a metadata-only update; it cannot be combined + // with content flags. Hand it back as its own mode so Run can dispatch + // the POST /skills/{name} envelope instead of POST /versions. if setDefaultProvided && (inlineProvided || fileProvided) { - return exterrors.Validation( + return updateModeNone, exterrors.Validation( exterrors.CodeConflictingArguments, "--set-default-version cannot be combined with --description / --instructions / --file", "pass --set-default-version on its own, or omit it to create a new default version", ) } if setDefaultProvided { - return nil + return updateModeSetDefault, nil } if !inlineProvided && !fileProvided { - return exterrors.Validation( + return updateModeNone, exterrors.Validation( exterrors.CodeMissingRequiredField, "no fields to update", "pass --description, --instructions, and/or --file ; or use --set-default-version ", ) } if inlineProvided && fileProvided { - return exterrors.Validation( + return updateModeNone, exterrors.Validation( exterrors.CodeConflictingArguments, "--file is mutually exclusive with --description / --instructions on update", "pass either inline flags or --file , not both", @@ -161,37 +298,41 @@ func (a *updateAction) validateFlags() error { } if fileProvided { - // Detect directory uploads before extension matching: update is - // inline-only by design, so a multi-file directory is rejected the - // same way .zip is — with a pointer to `create --force`. - if info, statErr := os.Stat(a.flags.file); statErr == nil && info.IsDir() { - return exterrors.Validation( - exterrors.CodeInvalidSkillFile, - "directory uploads cannot be applied via `skill update`", - "use `azd ai skill create --file --force` to replace the skill "+ - "(this deletes the existing skill and all of its versions first)", - ) + // Detect directories before extension matching so callers can point + // --file at the directory `azd ai skill download` extracted (which + // has no file extension at all). Matches selectCreateMode. + info, statErr := os.Stat(f.file) + if statErr == nil && info.IsDir() { + return updateModeFileDirectory, nil } - ext := strings.ToLower(filepath.Ext(a.flags.file)) + ext := strings.ToLower(filepath.Ext(f.file)) switch ext { case ".md": - return nil + return updateModeFileMd, nil case ".zip": - return exterrors.Validation( - exterrors.CodeInvalidSkillFile, - "ZIP packages cannot be applied via `skill update`", - "use `azd ai skill create --file .zip --force` to replace the skill "+ - "(this deletes the existing skill and all of its versions first)", - ) - default: - return exterrors.Validation( + return updateModeFilePackage, nil + } + // A --file value with no extension can only be a directory; if + // stat failed (typically fs.ErrNotExist), surface the stat error + // so users aren't told the extension is unsupported when the path + // is simply missing or unreadable. + if statErr != nil && ext == "" { + return updateModeNone, exterrors.Validation( exterrors.CodeInvalidSkillFile, - fmt.Sprintf("unsupported --file extension %q on update", ext), - "update only accepts .md files", + fmt.Sprintf("inspect --file %q: %s", f.file, statErr), + "verify the path exists and points to a SKILL.md, a .zip, "+ + "or a directory containing SKILL.md", ) } + return updateModeNone, exterrors.Validation( + exterrors.CodeInvalidSkillFile, + fmt.Sprintf("unsupported --file extension %q on update", ext), + "use .md for inline metadata, .zip for a package upload, "+ + "or a directory containing SKILL.md", + ) } - return nil + + return updateModeInline, nil } func newUpdateCommand(extCtx *azdext.ExtensionContext) *cobra.Command { @@ -201,17 +342,26 @@ func newUpdateCommand(extCtx *azdext.ExtensionContext) *cobra.Command { cmd := &cobra.Command{ Use: "update ", Short: "Create a new default version for a Foundry skill.", - Long: `Skills are versioned and immutable. ` + "`update`" + ` creates a new version from -inline content (--description / --instructions) or a SKILL.md file and sets -it as the skill's new default version. + Long: `Skills are versioned and immutable. ` + "`update`" + ` creates a new version and +sets it as the skill's new default version. ` + "`update`" + ` is non-destructive: +prior versions are preserved and remain reachable. -To repoint default_version at an existing version without uploading new -content, pass --set-default-version . +Accepts the same four input shapes as ` + "`create`" + `: -ZIP packages are not accepted here. To replace the entire skill (deleting all -existing versions), use ` + "`azd ai skill create --file .zip --force`" + `.`, + 1. Inline: --description "..." --instructions "..." + 2. SKILL.md: --file ./SKILL.md (CLI parses YAML front matter + body) + 3. Package: --file ./skill.zip (CLI uploads the archive as multipart/form-data) + 4. Directory: --file ./skill-src (CLI packages the directory as a zip and uploads it) + +Directory mode requires SKILL.md at the root of the directory — the same +layout that ` + "`azd ai skill download`" + ` writes by default. + +To repoint default_version at an existing version without uploading new +content, pass --set-default-version .`, Example: ` azd ai skill update my-skill --description "Updated summary" --instructions "..." azd ai skill update my-skill --file ./SKILL.md + azd ai skill update my-skill --file ./skill.zip + azd ai skill update my-skill --file ./skill-src/ azd ai skill update my-skill --set-default-version 1`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -226,7 +376,9 @@ existing versions), use ` + "`azd ai skill create --file .zip -- cmd.Flags().StringVar(&flags.description, "description", "", "New human-readable summary for the next version") cmd.Flags().StringVar(&flags.instructions, "instructions", "", "New Markdown instructions body for the next version") - cmd.Flags().StringVar(&flags.file, "file", "", "Path to a SKILL.md file whose values become the next version's inline content") + cmd.Flags().StringVar(&flags.file, "file", "", + "Path to a SKILL.md file, a .zip archive, or a directory whose contents become the next version. "+ + "Archives and directories must contain a SKILL.md at the root.") cmd.Flags().StringVar(&flags.setDefault, "set-default-version", "", "Set the skill's default_version to an existing version without uploading new content") azdext.RegisterFlagOptions(cmd, azdext.FlagOptions{ Name: "output", AllowedValues: []string{outputJSON, outputTable}, Default: outputJSON, diff --git a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_update_test.go b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_update_test.go index 8d348256c0b..65dc4d0b8ff 100644 --- a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_update_test.go +++ b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_update_test.go @@ -49,40 +49,106 @@ func TestUpdateAction_ValidInputFailsAtEndpoint(t *testing.T) { "should fail at endpoint resolution with no project configured") } -// TestUpdateAction_ZipSuggestionMentionsDestructive verifies that the error -// message for ZIP files on `update` tells the user the operation is -// destructive (delete-then-create at the skill level). -func TestUpdateAction_ZipSuggestionMentionsDestructive(t *testing.T) { - a := &updateAction{flags: &updateFlags{file: "skill.zip"}} - err := a.validateFlags() +// --- selectUpdateMode routing (issue #8489) --- +// +// Verifies that `update --file` now accepts the same three input shapes +// as `create --file`: a single SKILL.md, a .zip archive, or a directory +// whose root contains SKILL.md. Previously update rejected .zip and +// directories with a pointer to `create --force` (destructive). + +func TestSelectUpdateMode_FileMd(t *testing.T) { + mode, err := selectUpdateMode(&updateFlags{file: "./SKILL.md"}) + require.NoError(t, err) + require.Equal(t, updateModeFileMd, mode) +} + +func TestSelectUpdateMode_FilePackage(t *testing.T) { + for _, f := range []string{"./pkg.zip", "./PKG.ZIP"} { + mode, err := selectUpdateMode(&updateFlags{file: f}) + require.NoError(t, err, "file %q", f) + require.Equal(t, updateModeFilePackage, mode, "file %q", f) + } +} + +func TestSelectUpdateMode_FileDirectory(t *testing.T) { + dir := writeSkillDir(t, "my-skill") + mode, err := selectUpdateMode(&updateFlags{file: dir}) + require.NoError(t, err) + require.Equal(t, updateModeFileDirectory, mode) +} + +func TestSelectUpdateMode_InlineOnly(t *testing.T) { + mode, err := selectUpdateMode(&updateFlags{descriptionSet: true}) + require.NoError(t, err) + require.Equal(t, updateModeInline, mode) +} + +func TestSelectUpdateMode_SetDefaultAlone(t *testing.T) { + mode, err := selectUpdateMode(&updateFlags{setDefault: "2"}) + require.NoError(t, err) + require.Equal(t, updateModeSetDefault, mode) +} + +func TestSelectUpdateMode_SetDefaultConflictsWithContent(t *testing.T) { + for _, f := range []*updateFlags{ + {setDefault: "2", descriptionSet: true}, + {setDefault: "2", instructionsSet: true}, + {setDefault: "2", file: "./SKILL.md"}, + } { + mode, err := selectUpdateMode(f) + require.Error(t, err, "%+v", f) + require.Equal(t, updateModeNone, mode) + var le *azdext.LocalError + require.True(t, errors.As(err, &le)) + require.Equal(t, exterrors.CodeConflictingArguments, le.Code) + } +} + +func TestSelectUpdateMode_InlineAndFileConflict(t *testing.T) { + mode, err := selectUpdateMode(&updateFlags{ + descriptionSet: true, + file: "./SKILL.md", + }) + require.Error(t, err) + require.Equal(t, updateModeNone, mode) + var le *azdext.LocalError + require.True(t, errors.As(err, &le)) + require.Equal(t, exterrors.CodeConflictingArguments, le.Code) +} + +func TestSelectUpdateMode_NoInputFails(t *testing.T) { + mode, err := selectUpdateMode(&updateFlags{}) require.Error(t, err) + require.Equal(t, updateModeNone, mode) + var le *azdext.LocalError + require.True(t, errors.As(err, &le)) + require.Equal(t, exterrors.CodeMissingRequiredField, le.Code) +} + +func TestSelectUpdateMode_UnknownExtension(t *testing.T) { + mode, err := selectUpdateMode(&updateFlags{file: "./SKILL.txt"}) + require.Error(t, err) + require.Equal(t, updateModeNone, mode) var le *azdext.LocalError require.True(t, errors.As(err, &le)) require.Equal(t, exterrors.CodeInvalidSkillFile, le.Code) - require.Contains(t, le.Suggestion, "deletes", - "suggestion must warn that the operation is destructive") } -// TestUpdateAction_DirectoryRejectedWithDestructivePointer verifies that -// directory --file (multi-file uploads) is rejected on update with the same -// `create --force` pointer as .zip — keeping update inline-only by design. -func TestUpdateAction_DirectoryRejectedWithDestructivePointer(t *testing.T) { - dir := t.TempDir() - require.NoError(t, os.WriteFile( - filepath.Join(dir, "SKILL.md"), - []byte("---\nname: my-skill\n---\nbody"), - 0600, - )) - a := &updateAction{flags: &updateFlags{file: dir}} - err := a.validateFlags() +// TestSelectUpdateMode_MissingNoExtPathSurfacesStatError guards the same +// regression as the create-side TestSelectCreateMode_MissingNoExtPathSurfacesStatError: +// a --file value with no extension that does not exist on disk must +// surface the underlying stat failure rather than a misleading +// "unsupported --file extension \"\"" error. +func TestSelectUpdateMode_MissingNoExtPathSurfacesStatError(t *testing.T) { + missing := filepath.Join(t.TempDir(), "does-not-exist") + mode, err := selectUpdateMode(&updateFlags{file: missing}) require.Error(t, err) + require.Equal(t, updateModeNone, mode) var le *azdext.LocalError require.True(t, errors.As(err, &le)) require.Equal(t, exterrors.CodeInvalidSkillFile, le.Code) - require.Contains(t, le.Suggestion, "deletes", - "directory rejection must warn that the create --force fallback is destructive") - require.Contains(t, le.Suggestion, "directory", - "suggestion should point at the --file --force flow") + require.NotContains(t, le.Message, `unsupported --file extension ""`) + require.Contains(t, le.Message, "inspect --file") } func TestUpdateAction_SetDefaultVersion_AcceptsAlone(t *testing.T) { @@ -98,3 +164,47 @@ func TestUpdateAction_SetDefaultVersion_ConflictsWithContent(t *testing.T) { require.True(t, errors.As(err, &le)) require.Equal(t, exterrors.CodeConflictingArguments, le.Code) } + +// --- updateAction run-path preflight (no network) --- + +// TestUpdateAction_DirectoryWithoutSkillMdFails proves runFileDirectory +// rejects a directory missing SKILL.md before any network call, so it can +// be safely exercised with a nil client. Mirrors +// TestCreateAction_DirectoryWithoutSkillMdFails. +func TestUpdateAction_DirectoryWithoutSkillMdFails(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "README.md"), []byte("hi"), 0600)) + a := &updateAction{flags: &updateFlags{name: "my-skill", file: dir}} + err := a.runFileDirectory(context.Background(), nil) + require.Error(t, err) + var le *azdext.LocalError + require.True(t, errors.As(err, &le)) + require.Equal(t, exterrors.CodeInvalidSkillFile, le.Code) +} + +// TestUpdateAction_PackageFileMissingFails proves runFilePackage's stat +// preflight rejects a non-existent .zip before any network call (safe to +// exercise with a nil client). +func TestUpdateAction_PackageFileMissingFails(t *testing.T) { + missing := filepath.Join(t.TempDir(), "does-not-exist.zip") + a := &updateAction{flags: &updateFlags{name: "my-skill", file: missing}} + err := a.runFilePackage(context.Background(), nil) + require.Error(t, err) + var le *azdext.LocalError + require.True(t, errors.As(err, &le)) + require.Equal(t, exterrors.CodeInvalidSkillFile, le.Code) +} + +// TestUpdateAction_PackageFileIsDirectoryFails proves runFilePackage +// rejects a directory passed via the .zip-extension-only path (defense in +// depth — the routing layer would normally send directories through +// runFileDirectory, but the runFilePackage stat check exists for safety). +func TestUpdateAction_PackageFileIsDirectoryFails(t *testing.T) { + dir := t.TempDir() + a := &updateAction{flags: &updateFlags{name: "my-skill", file: dir}} + err := a.runFilePackage(context.Background(), nil) + require.Error(t, err) + var le *azdext.LocalError + require.True(t, errors.As(err, &le)) + require.Equal(t, exterrors.CodeInvalidSkillFile, le.Code) +} diff --git a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_validate_test.go b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_validate_test.go index ebdcb64cd64..570e28ff150 100644 --- a/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_validate_test.go +++ b/cli/azd/extensions/azure.ai.skills/internal/cmd/skill_validate_test.go @@ -121,20 +121,24 @@ func TestUpdateAction_ConflictingArgs(t *testing.T) { require.Equal(t, exterrors.CodeConflictingArguments, le.Code) } -func TestUpdateAction_RejectsZipFile(t *testing.T) { +// TestUpdateAction_AcceptsZipFile guards the issue-#8489 fix: `update` +// must accept `.zip` (and `.ZIP`) inputs and route them to the package +// upload path, matching `create` parity. Prior to the fix the same input +// was rejected with a destructive `create --force` pointer. +func TestUpdateAction_AcceptsZipFile(t *testing.T) { for _, f := range []string{"./pkg.zip", "./PKG.ZIP"} { - a := &updateAction{flags: &updateFlags{file: f}} - err := a.validateFlags() - require.Errorf(t, err, "file %q", f) - var le *azdext.LocalError - require.True(t, errors.As(err, &le), "file %q", f) - require.Equal(t, exterrors.CodeInvalidSkillFile, le.Code, "file %q", f) + mode, err := selectUpdateMode(&updateFlags{file: f}) + require.NoError(t, err, "file %q", f) + require.Equal(t, updateModeFilePackage, mode, "file %q", f) } } func TestUpdateAction_AcceptsMdFile(t *testing.T) { a := &updateAction{flags: &updateFlags{file: "./SKILL.md"}} require.NoError(t, a.validateFlags()) + mode, err := selectUpdateMode(&updateFlags{file: "./SKILL.md"}) + require.NoError(t, err) + require.Equal(t, updateModeFileMd, mode) } func TestUpdateAction_UnknownExtension(t *testing.T) {