Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions cli/azd/extensions/azure.ai.agents/internal/cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
}

Comment thread
therealjohn marked this conversation as resolved.
// 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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
103 changes: 103 additions & 0 deletions cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
}
Loading