diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go index ffbf0ffb163..46b84762104 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go @@ -278,6 +278,12 @@ func newInitCommand(rootFlags *rootFlagsDefinition) *cobra.Command { } if flags.manifestPointer != "" { + // Fail fast when the user accidentally passes a directory + // instead of a manifest file — before downloading templates. + if err := checkNotDirectory(flags.manifestPointer); err != nil { + return err + } + if err := runInitFromManifest(ctx, flags, azdClient, httpClient); err != nil { if exterrors.IsCancellation(err) { return exterrors.Cancelled("initialization was cancelled") @@ -463,6 +469,11 @@ func (a *InitAction) Run(ctx context.Context) error { ) } + // Catch the common mistake of passing a directory instead of a file + if err := checkNotDirectory(a.flags.manifestPointer); err != nil { + return err + } + // Download/read agent.yaml file from the provided URI or file path agentManifest, targetDir, err := a.downloadAgentYaml(ctx, a.flags.manifestPointer, a.flags.src) if err != nil { @@ -746,6 +757,68 @@ func (a *InitAction) isLocalFilePath(path string) bool { return false } +// checkNotDirectory returns a validation error when path is a directory +// instead of a manifest file. If an AgentManifest (a YAML file with a +// top-level "template" field) is found inside the directory, the suggestion +// includes the candidate manifest file path. +func checkNotDirectory(path string) error { + info, err := os.Stat(path) + if err != nil || !info.IsDir() { + return nil + } + + // Look for a manifest file inside the directory. We check several + // common names and only suggest a candidate when it actually looks like + // an AgentManifest (has a top-level "template" key) rather than an + // AgentDefinition that happens to share the same file name. + for _, name := range []string{"agent.manifest.yaml", "agent.manifest.yml", "agent.yaml", "agent.yml"} { + candidate := filepath.Join(path, name) + if looksLikeManifest(candidate) { + return exterrors.Validation( + exterrors.CodeInvalidManifestPointer, + fmt.Sprintf( + "'%s' is a directory, not a manifest file", + path, + ), + fmt.Sprintf( + "the --manifest flag must point to a manifest file, not a directory. Did you mean:\n -m %q", + candidate, + ), + ) + } + } + + return exterrors.Validation( + exterrors.CodeInvalidManifestPointer, + fmt.Sprintf("'%s' is a directory, not a manifest file", path), + "the --manifest flag must point to a manifest file (e.g. agent.manifest.yaml), not a directory", + ) +} + +// looksLikeManifest returns true when path is a regular file whose YAML +// content contains a top-level "template" key — the hallmark of an +// AgentManifest as opposed to an AgentDefinition. +func looksLikeManifest(path string) bool { + fi, err := os.Stat(path) + if err != nil || fi.IsDir() { + return false + } + + //nolint:gosec // candidate path comes from a user-provided directory + known file names + data, err := os.ReadFile(path) + if err != nil { + return false + } + + var top map[string]any + if err := yaml.Unmarshal(data, &top); err != nil { + return false + } + + _, hasTemplate := top["template"] + return hasTemplate +} + func (a *InitAction) isGitHubUrl(manifestPointer string) bool { // Check if it's a GitHub URL based on the patterns from downloadGithubManifest parsedURL, err := url.Parse(manifestPointer) @@ -837,6 +910,12 @@ func (a *InitAction) downloadAgentYaml( // Check if manifestPointer is a local file path or a URI if a.isLocalFilePath(manifestPointer) { + // Guard against directories (defense in depth — the caller should + // have caught this already, but check here for safety). + if err := checkNotDirectory(manifestPointer); err != nil { + return nil, "", err + } + // Handle local file path fmt.Printf("Reading agent.yaml from local file: %s\n", manifestPointer) //nolint:gosec // manifest path is an explicit user-provided local path diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go index 6fb935235a3..7ebadfb8ad5 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go @@ -5,10 +5,13 @@ package cmd import ( "context" + "errors" "os" "path/filepath" + "strings" "testing" + "azureaiagent/internal/exterrors" "github.com/azure/azure-dev/cli/azd/pkg/azdext" "google.golang.org/genproto/googleapis/rpc/errdetails" "google.golang.org/grpc/codes" @@ -394,3 +397,103 @@ func TestParseGitHubUrlNaive(t *testing.T) { }) } } + +func TestCheckNotDirectory_ReturnsNilForFile(t *testing.T) { + t.Parallel() + + file := filepath.Join(t.TempDir(), "agent.yaml") + //nolint:gosec // test fixture file permissions are intentional + if err := os.WriteFile(file, []byte("name: test"), 0644); err != nil { + t.Fatalf("write file: %v", err) + } + + if err := checkNotDirectory(file); err != nil { + t.Fatalf("expected nil for a regular file, got: %v", err) + } +} + +func TestCheckNotDirectory_ReturnsNilForNonexistentPath(t *testing.T) { + t.Parallel() + + if err := checkNotDirectory(filepath.Join(t.TempDir(), "nope")); err != nil { + t.Fatalf("expected nil for nonexistent path, got: %v", err) + } +} + +func TestCheckNotDirectory_ErrorForDirectoryWithManifest(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + manifest := filepath.Join(dir, "agent.manifest.yaml") + // Must include a "template" key so looksLikeManifest recognises it as a manifest. + content := "name: test\ntemplate:\n kind: hosted\n" + //nolint:gosec // test fixture file permissions are intentional + if err := os.WriteFile(manifest, []byte(content), 0644); err != nil { + t.Fatalf("write agent.manifest.yaml: %v", err) + } + + err := checkNotDirectory(dir) + if err == nil { + t.Fatal("expected error for directory containing agent.manifest.yaml") + } + + localErr, ok := errors.AsType[*azdext.LocalError](err) + if !ok { + t.Fatalf("expected *azdext.LocalError, got %T", err) + } + + if localErr.Code != exterrors.CodeInvalidManifestPointer { + t.Errorf("expected code %q, got %q", exterrors.CodeInvalidManifestPointer, localErr.Code) + } + + if !strings.Contains(localErr.Message, "directory") { + t.Errorf("message should mention 'directory', got: %s", localErr.Message) + } + + if !strings.Contains(localErr.Suggestion, "-m") { + t.Errorf("suggestion should include '-m' flag, got: %s", localErr.Suggestion) + } + + if !strings.Contains(localErr.Suggestion, "agent.manifest.yaml") { + t.Errorf("suggestion should include candidate path, got: %s", localErr.Suggestion) + } +} + +func TestCheckNotDirectory_NoSuggestionForAgentDefinition(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + // An AgentDefinition has "kind" at root but no "template" — should NOT + // be suggested as a manifest file. + defContent := "kind: hosted\nname: my-agent\n" + //nolint:gosec // test fixture file permissions are intentional + if err := os.WriteFile(filepath.Join(dir, "agent.yaml"), []byte(defContent), 0644); err != nil { + t.Fatalf("write agent.yaml: %v", err) + } + + err := checkNotDirectory(dir) + if err == nil { + t.Fatal("expected error for directory") + } + + // The error should NOT suggest the agent.yaml since it's a definition, not a manifest. + errMsg := err.Error() + if strings.Contains(errMsg, "agent.yaml") { + t.Errorf("should not suggest AgentDefinition file, got: %s", errMsg) + } +} + +func TestCheckNotDirectory_ErrorForDirectoryWithoutManifest(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + err := checkNotDirectory(dir) + if err == nil { + t.Fatal("expected error for empty directory") + } + + errMsg := err.Error() + if !strings.Contains(errMsg, "directory") { + t.Errorf("error should mention 'directory', got: %s", errMsg) + } +}