From df0f284826120fd14de920a90a11bd2902976349 Mon Sep 17 00:00:00 2001 From: Zhijie Huang Date: Fri, 22 May 2026 18:01:15 +0800 Subject: [PATCH 1/4] feat(agents): reuse existing agent.yaml on init from existing code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #7268. When `azd ai agent init` runs in a directory that already contains a bare `agent.yaml` definition, the from-code action now detects and reuses it instead of either prompting to overwrite (interactive) or failing fast with `CodeInvalidAgentManifest: agent.yaml already exists at ...` (--no-prompt mode, added in #8266). The reuse path: - Validates the file as a bare AgentDefinition (rejects manifest-shaped files that fail upstream detectLocalManifest validation with a targeted error). - Skips the model / instructions / runtime / entry-point prompt sequence in createDefinitionFromLocalAgent. - Ensures an azd environment exists (the prompt sequence normally bootstraps this) and calls the existing addToProject helper. - Never rewrites the on-disk agent.yaml — it is treated as authoritative. Valid local manifest files (top-level `template:`) are unchanged: they continue to be intercepted upstream by detectLocalManifest in init.go and routed through runInitFromManifest before InitFromCodeAction.Run is reached. Design spec: cli/azd/docs/design/azure-ai-agent-init-reuse-agent-yaml.md --- .../azure.ai.agents/internal/cmd/init.go | 42 ++++ .../internal/cmd/init_from_code.go | 29 --- .../internal/cmd/init_from_code_reuse.go | 182 ++++++++++++++++++ 3 files changed, 224 insertions(+), 29 deletions(-) create mode 100644 cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse.go 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 5a7ebe3a710..6223b0854fa 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go @@ -709,6 +709,48 @@ from code-deploy ZIP packaging (uses .gitignore syntax).`, } } + // When no valid manifest was detected upstream, look for a bare + // agent.yaml definition the user wants to reuse. This skips both the + // init-mode prompt and the from-code scaffolding sequence — the + // definition itself is the source of truth. See issue #7268. + if flags.manifestPointer == "" { + checkDir := flags.src + if checkDir == "" { + checkDir = "." + } + existing, findErr := findExistingAgentYaml(checkDir) + if findErr != nil { + return findErr + } + if existing != "" { + useExisting := flags.noPrompt + if !flags.noPrompt { + confirmResp, promptErr := azdClient.Prompt().Confirm(ctx, &azdext.ConfirmRequest{ + Options: &azdext.ConfirmOptions{ + Message: fmt.Sprintf( + "An existing agent definition was found at %q. Use it?", + existing, + ), + DefaultValue: new(true), + }, + }) + if promptErr != nil { + if exterrors.IsCancellation(promptErr) { + return exterrors.Cancelled("initialization was cancelled") + } + return fmt.Errorf("prompting for definition reuse: %w", promptErr) + } + useExisting = *confirmResp.Value + } + if useExisting { + if flags.src == "" { + flags.src = checkDir + } + return runReuseDefinition(ctx, flags, azdClient, httpClient, checkDir, existing) + } + } + } + if flags.manifestPointer != "" { // Fail fast when the user accidentally passes a directory // instead of a manifest file — before downloading templates. diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go index 242b9d18a14..f910cbc94e2 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go @@ -89,35 +89,6 @@ func (a *InitFromCodeAction) Run(ctx context.Context) error { srcDir = "." } - // Check if agent.yaml already exists before the interactive setup so the user - // doesn't complete the full agent configuration only to have it discarded. - agentYamlPath := filepath.Join(srcDir, "agent.yaml") - if _, statErr := os.Stat(agentYamlPath); statErr == nil { - if a.flags.noPrompt { - return exterrors.Validation( - exterrors.CodeInvalidAgentManifest, - fmt.Sprintf("agent.yaml already exists at %q", agentYamlPath), - "delete or move the existing agent.yaml, or run interactively to confirm overwrite", - ) - } - - confirmResp, err := a.azdClient.Prompt().Confirm(ctx, &azdext.ConfirmRequest{ - Options: &azdext.ConfirmOptions{ - Message: fmt.Sprintf("An agent.yaml already exists in %q. Overwrite?", srcDir), - DefaultValue: new(false), - }, - }) - if err != nil { - if exterrors.IsCancellation(err) { - return exterrors.Cancelled("overwrite confirmation was cancelled") - } - return fmt.Errorf("prompting for overwrite confirmation: %w", err) - } - if !*confirmResp.Value { - return exterrors.Cancelled("agent.yaml already exists; overwrite declined") - } - } - // No manifest pointer provided - process local agent code // Create a definition based on user prompts localDefinition, err := a.createDefinitionFromLocalAgent(ctx) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse.go new file mode 100644 index 00000000000..0f162fe1f43 --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse.go @@ -0,0 +1,182 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "azureaiagent/internal/exterrors" + "azureaiagent/internal/pkg/agents/agent_yaml" + "context" + "errors" + "fmt" + "io/fs" + "net/http" + "os" + "path/filepath" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/fatih/color" + "gopkg.in/yaml.v3" +) + +// agentYamlCandidates lists the file names (in priority order) scanned by the +// reuse path. The set intentionally mirrors detectLocalManifest in +// init_from_templates_helpers.go so the same on-disk files trigger either the +// manifest pipeline (when they parse as a valid manifest) or the definition +// reuse pipeline (when they do not). +var agentYamlCandidates = []string{ + "agent.manifest.yaml", + "agent.manifest.yml", + "agent.yaml", + "agent.yml", +} + +// findExistingAgentYaml returns the first agent yaml file found in srcDir, or +// an empty string when none exists. The scan is shallow — only srcDir itself +// is checked, never subdirectories. ErrNotExist is treated as "not found" and +// the next candidate is tried; other I/O errors propagate. +// +// This helper is called by RunE in init.go after detectLocalManifest has been +// given the first chance to claim the file. A path returned here is therefore +// guaranteed to not be a valid manifest (otherwise detectLocalManifest would +// have set flags.manifestPointer first); it is either a bare definition or a +// malformed manifest. runReuseDefinition distinguishes the two and produces a +// targeted error for the malformed case. +func findExistingAgentYaml(srcDir string) (string, error) { + for _, name := range agentYamlCandidates { + candidate := filepath.Join(srcDir, name) + info, err := os.Stat(candidate) + if errors.Is(err, fs.ErrNotExist) { + continue + } + if err != nil { + return "", fmt.Errorf("checking for %s: %w", candidate, err) + } + if info.IsDir() { + continue + } + return candidate, nil + } + + return "", nil +} + +// runReuseDefinition wires an existing bare agent.yaml definition into +// azure.yaml without rewriting the file or running the from-code scaffolding +// prompts. It is invoked from init.go RunE alongside the manifest detection +// branch, so the user is never asked to pick an init mode when a local +// agent.yaml is already present. +// +// The function deliberately does NOT resolve a Foundry project, prompt for +// model deployments, or enrich the definition with deployment metadata — the +// issue (#7268) constrains the definition path to "less to ask and just setup +// azure.yaml". Users who need a Foundry project bound before azd deploy +// should either delete the definition and rerun init interactively, or set +// AZURE_AI_PROJECT_ID in their azd environment by hand. +func runReuseDefinition( + ctx context.Context, + flags *initFlags, + azdClient *azdext.AzdClient, + httpClient *http.Client, + srcDir string, + existingPath string, +) error { + displayPath, err := filepath.Rel(srcDir, existingPath) + if err != nil || displayPath == "" { + displayPath = existingPath + } + + def, err := loadAgentDefinitionFile(existingPath) + if err != nil { + return exterrors.Validation( + exterrors.CodeInvalidAgentManifest, + fmt.Sprintf("agent definition in %s is invalid: %s", displayPath, err), + "Fix the agent.yaml and retry, or remove the file to start a fresh init.", + ) + } + + fmt.Println(color.HiBlackString( + "Detected existing agent definition: %s (name: %s).", + displayPath, def.Name, + )) + + // Reuse the same project/environment bootstrap runInitFromManifest uses so + // addToProject has the project config and env it needs to write azure.yaml. + projectConfig, err := ensureProject(ctx, flags, azdClient) + if err != nil { + return err + } + + env := getExistingEnvironment(ctx, flags.env, azdClient) + if env == nil { + envName := flags.env + if envName == "" { + envName = sanitizeAgentName(def.Name + "-dev") + } + env, err = createNewEnvironment(ctx, azdClient, envName) + if err != nil { + return fmt.Errorf("failed to create azd environment: %w", err) + } + flags.env = env.Name + } + + action := &InitFromCodeAction{ + azdClient: azdClient, + flags: flags, + projectConfig: projectConfig, + environment: env, + httpClient: httpClient, + } + + isCodeDeploy := def.CodeConfiguration != nil + if err := action.addToProject(ctx, srcDir, def.Name, isCodeDeploy); err != nil { + return fmt.Errorf("failed to add agent to azure.yaml: %w", err) + } + + fmt.Println(color.HiBlackString("Reusing existing agent.yaml (name: %s).", def.Name)) + + // Run the same advisory post-init validations the scaffold path emits. + validatePostInit(srcDir, def.CodeConfiguration) + + return nil +} + +// loadAgentDefinitionFile parses path as a bare AgentDefinition (no surrounding +// "template:" wrapper) and validates the name. It is the definition-side +// counterpart to agent_yaml.LoadAndValidateAgentManifest. +// +// The parser uses ContainerAgent so a CodeConfiguration block — when present — +// is preserved for the caller to inspect (it drives isCodeDeploy and the +// post-init language detection). +func loadAgentDefinitionFile(path string) (*agent_yaml.ContainerAgent, error) { + //nolint:gosec // path comes from findExistingAgentYaml against a user-controlled directory + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("read %s: %w", path, err) + } + + // Reject files that are actually manifests. A valid manifest would have been + // routed upstream; an invalid one reaching here is most likely a malformed + // template authored by the user, so the error message points at the cause. + var top map[string]any + if err := yaml.Unmarshal(data, &top); err != nil { + return nil, fmt.Errorf("parse %s: %w", path, err) + } + if _, hasTemplate := top["template"]; hasTemplate { + return nil, fmt.Errorf( + "file contains a 'template:' field but did not parse as a valid agent manifest; " + + "fix the manifest schema and retry", + ) + } + + var def agent_yaml.ContainerAgent + if err := yaml.Unmarshal(data, &def); err != nil { + return nil, fmt.Errorf("parse %s: %w", path, err) + } + + if err := agent_yaml.ValidateAgentName(def.Name); err != nil { + return nil, fmt.Errorf("invalid agent name %q: %w", def.Name, err) + } + + return &def, nil +} From 2c74fbe759a49807b356ed7d10727a2b54aa939b Mon Sep 17 00:00:00 2001 From: Zhijie Huang Date: Mon, 25 May 2026 13:47:24 +0800 Subject: [PATCH 2/4] fix(agents): skip definition reuse when manifest detected but declined Copilot review of #8325 caught an edge case in the lifted dispatch: when detectLocalManifest finds a valid manifest in srcDir but the user declines the 'Use it?' prompt, flags.manifestPointer stays empty and the new definition reuse scan would re-discover the same on-disk file. The bare definition loader rejects files with a top-level 'template:' key, so the declined manifest would be reported as an invalid definition with CodeInvalidAgentManifest, blocking init and contradicting the user's choice to start fresh. Track the decline explicitly with a manifestDetectedButDeclined flag and gate the definition reuse scan on it being false. Non-interactive mode is unaffected because --no-prompt auto-accepts the manifest. --- .../azure.ai.agents/internal/cmd/init.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) 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 6223b0854fa..4002503ce51 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go @@ -671,6 +671,12 @@ from code-deploy ZIP packaging (uses .gitignore syntax).`, // Auto-detect an existing agent manifest in the target directory // when no --manifest flag was provided. + // + // manifestDetectedButDeclined records the case where detectLocalManifest + // found a valid manifest that the user actively declined. The + // definition-reuse scan below must skip in that case so the declined + // file is not mis-classified as an invalid bare definition. See #7268. + manifestDetectedButDeclined := false if flags.manifestPointer == "" { checkDir := flags.src if checkDir == "" { @@ -705,6 +711,8 @@ from code-deploy ZIP packaging (uses .gitignore syntax).`, if flags.src == "" { flags.src = checkDir } + } else { + manifestDetectedButDeclined = true } } } @@ -713,7 +721,12 @@ from code-deploy ZIP packaging (uses .gitignore syntax).`, // agent.yaml definition the user wants to reuse. This skips both the // init-mode prompt and the from-code scaffolding sequence — the // definition itself is the source of truth. See issue #7268. - if flags.manifestPointer == "" { + // + // Skip when detectLocalManifest already found a valid manifest that + // the user declined: the on-disk file would otherwise be re-discovered + // here and rejected as an "invalid definition" because of its top-level + // `template:` key, contradicting the user's choice to start fresh. + if flags.manifestPointer == "" && !manifestDetectedButDeclined { checkDir := flags.src if checkDir == "" { checkDir = "." From 7fd67283fa23d32215867194b2dce200fcaf7f17 Mon Sep 17 00:00:00 2001 From: Zhijie Huang Date: Mon, 25 May 2026 14:06:18 +0800 Subject: [PATCH 3/4] fix(agents): address copilot review feedback on reuse path Five fixes plus targeted unit tests, in response to inline review on #8326. - Use go.yaml.in/yaml/v3 in init_from_code_reuse.go so custom UnmarshalYAML methods on agent_yaml types (e.g. PropertySchema) are honored. The package previously used gopkg.in/yaml.v3 which has an incompatible Node type and silently bypassed custom decoders. - Run agent_yaml.ValidateAgentDefinition on the loaded bytes so missing or invalid kind, structural shape, etc. fail fast with the same error the manifest pipeline produces, instead of slipping through and failing later. - Walk agentYamlCandidates in addToProject's language-detection block instead of hardcoding 'agent.yaml', so dotnet runtimes declared in agent.yml are detected correctly. - Replace hardcoded 'agent.yaml' in user-facing error/info messages with displayPath, so the reuse path correctly references agent.yml or agent.manifest.yaml when those are the detected file. - Mirror InitFromCodeAction.Run's absolute --src normalization: convert an absolute path to a project-relative path before calling addToProject so azure.yaml's RelativePath stays portable. Unit tests added in init_from_code_reuse_test.go cover findExistingAgentYaml (filename precedence, shallow scan, dir-vs-file), loadAgentDefinitionFile (happy path, missing kind, invalid name, manifest-shaped rejection, broken yaml), and the runReuseDefinition failure paths that surface CodeInvalidAgentManifest without needing the project gRPC mock. --- .../internal/cmd/init_from_code.go | 14 +- .../internal/cmd/init_from_code_reuse.go | 38 +++- .../internal/cmd/init_from_code_reuse_test.go | 197 ++++++++++++++++++ 3 files changed, 235 insertions(+), 14 deletions(-) create mode 100644 cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse_test.go diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go index f910cbc94e2..b8f50c8a7e7 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go @@ -968,16 +968,22 @@ func (a *InitFromCodeAction) addToProject(ctx context.Context, targetDir string, if !isCodeDeploy { language = "docker" } else { - // Detect language from agent.yaml runtime - // Re-read agent.yaml to detect the language for azure.yaml service config - langDetectPath := filepath.Join(a.projectConfig.Path, targetDir, "agent.yaml") - if data, err := os.ReadFile(langDetectPath); err == nil { //nolint:gosec // path from project config + // Detect language from the on-disk agent yaml. The from-code scaffold path + // writes agent.yaml; the reuse path (issue #7268) may have agent.yml. Walk + // the same candidates findExistingAgentYaml uses so either case is handled. + for _, name := range agentYamlCandidates { + langDetectPath := filepath.Join(a.projectConfig.Path, targetDir, name) + data, err := os.ReadFile(langDetectPath) //nolint:gosec // path from project config + if err != nil { + continue + } var langDef agent_yaml.ContainerAgent if err := yaml.Unmarshal(data, &langDef); err == nil && langDef.CodeConfiguration != nil && strings.HasPrefix(langDef.CodeConfiguration.Runtime, "dotnet_") { language = "csharp" } + break } } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse.go index 0f162fe1f43..e3751e34783 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse.go @@ -16,7 +16,7 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/azdext" "github.com/fatih/color" - "gopkg.in/yaml.v3" + "go.yaml.in/yaml/v3" ) // agentYamlCandidates lists the file names (in priority order) scanned by the @@ -32,7 +32,7 @@ var agentYamlCandidates = []string{ } // findExistingAgentYaml returns the first agent yaml file found in srcDir, or -// an empty string when none exists. The scan is shallow — only srcDir itself +// an empty string when none exists. The scan is shallow: only srcDir itself // is checked, never subdirectories. ErrNotExist is treated as "not found" and // the next candidate is tried; other I/O errors propagate. // @@ -91,7 +91,7 @@ func runReuseDefinition( return exterrors.Validation( exterrors.CodeInvalidAgentManifest, fmt.Sprintf("agent definition in %s is invalid: %s", displayPath, err), - "Fix the agent.yaml and retry, or remove the file to start a fresh init.", + fmt.Sprintf("Fix %s and retry, or remove the file to start a fresh init.", displayPath), ) } @@ -107,6 +107,20 @@ func runReuseDefinition( return err } + // Mirror InitFromCodeAction.Run: when --src is absolute, convert it to a + // path relative to the azd project root so azure.yaml's RelativePath is + // portable across machines. + if flags.src != "" && filepath.IsAbs(flags.src) { + relPath, err := filepath.Rel(projectConfig.Path, flags.src) + if err != nil { + return fmt.Errorf("failed to convert src path to relative path: %w", err) + } + flags.src = relPath + // srcDir was passed in by the caller; keep it in sync so the messages + // and addToProject targetDir match. + srcDir = relPath + } + env := getExistingEnvironment(ctx, flags.env, azdClient) if env == nil { envName := flags.env @@ -133,7 +147,7 @@ func runReuseDefinition( return fmt.Errorf("failed to add agent to azure.yaml: %w", err) } - fmt.Println(color.HiBlackString("Reusing existing agent.yaml (name: %s).", def.Name)) + fmt.Println(color.HiBlackString("Reusing existing %s (name: %s).", displayPath, def.Name)) // Run the same advisory post-init validations the scaffold path emits. validatePostInit(srcDir, def.CodeConfiguration) @@ -142,8 +156,8 @@ func runReuseDefinition( } // loadAgentDefinitionFile parses path as a bare AgentDefinition (no surrounding -// "template:" wrapper) and validates the name. It is the definition-side -// counterpart to agent_yaml.LoadAndValidateAgentManifest. +// "template:" wrapper) and validates it. It is the definition-side counterpart +// to agent_yaml.LoadAndValidateAgentManifest. // // The parser uses ContainerAgent so a CodeConfiguration block — when present — // is preserved for the caller to inspect (it drives isCodeDeploy and the @@ -169,14 +183,18 @@ func loadAgentDefinitionFile(path string) (*agent_yaml.ContainerAgent, error) { ) } + // Run the same schema validation the manifest pipeline runs on the unwrapped + // template body. This catches missing/invalid kind, missing name, and the + // kind-specific structural checks before the bare definition is wired into + // azure.yaml. + if err := agent_yaml.ValidateAgentDefinition(data); err != nil { + return nil, err + } + var def agent_yaml.ContainerAgent if err := yaml.Unmarshal(data, &def); err != nil { return nil, fmt.Errorf("parse %s: %w", path, err) } - if err := agent_yaml.ValidateAgentName(def.Name); err != nil { - return nil, fmt.Errorf("invalid agent name %q: %w", def.Name, err) - } - return &def, nil } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse_test.go new file mode 100644 index 00000000000..56765a1b329 --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse_test.go @@ -0,0 +1,197 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "azureaiagent/internal/exterrors" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/stretchr/testify/require" +) + +// These tests cover the helpers that back the issue #7268 reuse path. +// runReuseDefinition itself is exercised by manual e2e (see +// cli/azd/docs/design/azure-ai-agent-init-reuse-agent-yaml.md §7) because a +// full unit test would require mocking the azd Project gRPC service in +// addition to the Environment/Prompt mocks that already exist; that +// investment is deferred until the next time a reuse-related change lands. + +func TestFindExistingAgentYaml(t *testing.T) { + t.Parallel() + + t.Run("empty dir returns no match", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + got, err := findExistingAgentYaml(dir) + require.NoError(t, err) + require.Empty(t, got) + }) + + t.Run("finds agent.yaml", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeReuseTestFile(t, dir, "agent.yaml", "kind: hosted\nname: foo\n") + + got, err := findExistingAgentYaml(dir) + require.NoError(t, err) + require.Equal(t, filepath.Join(dir, "agent.yaml"), got) + }) + + t.Run("agent.manifest.yaml takes priority over agent.yaml", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeReuseTestFile(t, dir, "agent.yaml", "kind: hosted\nname: foo\n") + writeReuseTestFile(t, dir, "agent.manifest.yaml", "template:\n kind: hosted\n name: foo\n") + + got, err := findExistingAgentYaml(dir) + require.NoError(t, err) + require.Equal(t, filepath.Join(dir, "agent.manifest.yaml"), got) + }) + + t.Run("directory entries are ignored", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(dir, "agent.yaml"), 0o750)) + + got, err := findExistingAgentYaml(dir) + require.NoError(t, err) + require.Empty(t, got) + }) + + t.Run("scan does not recurse into subdirectories", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + nested := filepath.Join(dir, "src") + require.NoError(t, os.Mkdir(nested, 0o750)) + writeReuseTestFile(t, nested, "agent.yaml", "kind: hosted\nname: foo\n") + + got, err := findExistingAgentYaml(dir) + require.NoError(t, err) + require.Empty(t, got, "shallow scan only; nested agent.yaml must be ignored") + }) +} + +func TestLoadAgentDefinitionFile(t *testing.T) { + t.Parallel() + + t.Run("happy path: bare definition with hosted kind", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := writeReuseTestFile(t, dir, "agent.yaml", + "kind: hosted\nname: my-agent\nmodel:\n id: gpt-4o-mini\n") + + def, err := loadAgentDefinitionFile(path) + require.NoError(t, err) + require.NotNil(t, def) + require.Equal(t, "my-agent", def.Name) + require.Nil(t, def.CodeConfiguration) + }) + + t.Run("happy path: code_configuration preserved", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := writeReuseTestFile(t, dir, "agent.yaml", + "kind: hosted\nname: my-agent\ncode_configuration:\n runtime: python_3_12\n entry_point: main.py\n") + + def, err := loadAgentDefinitionFile(path) + require.NoError(t, err) + require.NotNil(t, def.CodeConfiguration) + require.Equal(t, "python_3_12", def.CodeConfiguration.Runtime) + }) + + t.Run("rejects manifest-shaped file (top-level template key)", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := writeReuseTestFile(t, dir, "agent.yaml", "template:\n kind: hosted\n name: foo\n") + + _, err := loadAgentDefinitionFile(path) + require.Error(t, err) + require.Contains(t, err.Error(), "template") + }) + + t.Run("rejects missing kind via ValidateAgentDefinition", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := writeReuseTestFile(t, dir, "agent.yaml", "name: my-agent\n") + + _, err := loadAgentDefinitionFile(path) + require.Error(t, err) + require.Contains(t, err.Error(), "kind") + }) + + t.Run("rejects invalid agent name via ValidateAgentDefinition", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := writeReuseTestFile(t, dir, "agent.yaml", "kind: hosted\nname: \"!!! invalid\"\n") + + _, err := loadAgentDefinitionFile(path) + require.Error(t, err) + }) + + t.Run("rejects broken yaml", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := writeReuseTestFile(t, dir, "agent.yaml", "name: : :\nmodel: [unterminated\n") + + _, err := loadAgentDefinitionFile(path) + require.Error(t, err) + }) +} + +// TestRunReuseDefinition_InvalidFileReturnsStructuredError covers the +// "malformed template-wrapped YAML returns CodeInvalidAgentManifest" case the +// reviewer explicitly asked for. We invoke the failure path directly because +// it short-circuits before any azd gRPC calls are made and so does not need a +// full client mock. +func TestRunReuseDefinition_InvalidFileReturnsStructuredError(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := writeReuseTestFile(t, dir, "agent.yaml", "name: : :\nmodel: [unterminated\n") + + err := runReuseDefinition(t.Context(), &initFlags{}, nil, nil, dir, path) + require.Error(t, err) + + var localErr *azdext.LocalError + require.True(t, errors.As(err, &localErr), "expected *azdext.LocalError, got %T", err) + require.Equal(t, exterrors.CodeInvalidAgentManifest, localErr.Code) + require.NotEmpty(t, localErr.Suggestion) + // Error message and suggestion should both reference the file by name. + require.Contains(t, localErr.Message, "agent.yaml") + require.Contains(t, localErr.Suggestion, "agent.yaml") +} + +// TestRunReuseDefinition_RejectsManifestShapedFile covers the case where the +// scan picks up a file that looks like a manifest but failed upstream +// validation. The user sees a targeted error instead of falling through to +// scaffolding prompts. +func TestRunReuseDefinition_RejectsManifestShapedFile(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + // Manifest-shaped but missing required fields, so detectLocalManifest would + // have rejected it upstream and our reuse scan picks it up here. + path := writeReuseTestFile(t, dir, "agent.manifest.yaml", + "template:\n # intentionally incomplete\n") + + err := runReuseDefinition(t.Context(), &initFlags{}, nil, nil, dir, path) + require.Error(t, err) + + var localErr *azdext.LocalError + require.True(t, errors.As(err, &localErr)) + require.Equal(t, exterrors.CodeInvalidAgentManifest, localErr.Code) + require.Contains(t, localErr.Message, "agent.manifest.yaml", + "error message must name the actual file, not a hardcoded agent.yaml") +} + +func writeReuseTestFile(t *testing.T, dir, name, contents string) string { + t.Helper() + path := filepath.Join(dir, name) + require.NoError(t, os.WriteFile(path, []byte(contents), 0o600)) + return path +} From ab2e2ad420c8254757d51b4c32c9645f67c2f326 Mon Sep 17 00:00:00 2001 From: Zhijie Huang Date: Mon, 25 May 2026 15:30:16 +0800 Subject: [PATCH 4/4] fix(agents): address second-round copilot review and trim comments Five Copilot findings on the new reuse path: - Lang detection in addToProject (init_from_code.go) restricted to agent.yaml / agent.yml so a leftover agent.manifest.* does not short-circuit detection before the actual definition file is read. - agentYamlCandidates reordered to match detectLocalManifest in init_from_templates_helpers.go (manifest.yaml -> yaml -> manifest.yml -> yml). - errors.As replaced with errors.AsType[T] in init_from_code_reuse_test.go per the cli/azd/AGENTS.md Go 1.26 modernization rule. - Overwrite guard restored inside InitFromCodeAction.Run so the from-code scaffold cannot silently overwrite an existing agent.yaml when the user declined the reuse prompt in RunE. Matches the safety invariant from #8266 (--no-prompt fails fast; interactive defaults to no). Also trims verbose doc and inline comments across the touched files. --- .../azure.ai.agents/internal/cmd/init.go | 18 ++---- .../internal/cmd/init_from_code.go | 43 +++++++++++-- .../internal/cmd/init_from_code_reuse.go | 60 +++++-------------- .../internal/cmd/init_from_code_reuse_test.go | 33 ++++------ 4 files changed, 70 insertions(+), 84 deletions(-) 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 4002503ce51..3441ca67ee4 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go @@ -672,10 +672,8 @@ from code-deploy ZIP packaging (uses .gitignore syntax).`, // Auto-detect an existing agent manifest in the target directory // when no --manifest flag was provided. // - // manifestDetectedButDeclined records the case where detectLocalManifest - // found a valid manifest that the user actively declined. The - // definition-reuse scan below must skip in that case so the declined - // file is not mis-classified as an invalid bare definition. See #7268. + // manifestDetectedButDeclined: gates the definition-reuse scan below so + // a declined manifest is not re-discovered and mis-classified. manifestDetectedButDeclined := false if flags.manifestPointer == "" { checkDir := flags.src @@ -717,15 +715,9 @@ from code-deploy ZIP packaging (uses .gitignore syntax).`, } } - // When no valid manifest was detected upstream, look for a bare - // agent.yaml definition the user wants to reuse. This skips both the - // init-mode prompt and the from-code scaffolding sequence — the - // definition itself is the source of truth. See issue #7268. - // - // Skip when detectLocalManifest already found a valid manifest that - // the user declined: the on-disk file would otherwise be re-discovered - // here and rejected as an "invalid definition" because of its top-level - // `template:` key, contradicting the user's choice to start fresh. + // When no manifest was detected, look for a bare agent.yaml definition + // to reuse (issue #7268). Skips the init-mode prompt and from-code + // scaffolding. Bypassed when the user already declined a manifest above. if flags.manifestPointer == "" && !manifestDetectedButDeclined { checkDir := flags.src if checkDir == "" { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go index b8f50c8a7e7..266358d2928 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go @@ -89,6 +89,42 @@ func (a *InitFromCodeAction) Run(ctx context.Context) error { srcDir = "." } + // Guard against silently overwriting an existing agent definition. Reached + // when the user declined the reuse prompt in RunE or bypassed it; we still + // refuse in --no-prompt and confirm interactively. + if existing, statErr := findExistingAgentYaml(srcDir); statErr == nil && existing != "" { + displayPath, relErr := filepath.Rel(srcDir, existing) + if relErr != nil || displayPath == "" { + displayPath = existing + } + if a.flags.noPrompt { + return exterrors.Validation( + exterrors.CodeInvalidAgentManifest, + fmt.Sprintf("%s already exists at %q", displayPath, existing), + fmt.Sprintf( + "delete or move the existing %s, or run interactively to confirm overwrite", + displayPath, + ), + ) + } + + confirmResp, err := a.azdClient.Prompt().Confirm(ctx, &azdext.ConfirmRequest{ + Options: &azdext.ConfirmOptions{ + Message: fmt.Sprintf("An agent definition already exists at %q. Overwrite?", displayPath), + DefaultValue: new(false), + }, + }) + if err != nil { + if exterrors.IsCancellation(err) { + return exterrors.Cancelled("overwrite confirmation was cancelled") + } + return fmt.Errorf("prompting for overwrite confirmation: %w", err) + } + if !*confirmResp.Value { + return exterrors.Cancelled(fmt.Sprintf("%s already exists; overwrite declined", displayPath)) + } + } + // No manifest pointer provided - process local agent code // Create a definition based on user prompts localDefinition, err := a.createDefinitionFromLocalAgent(ctx) @@ -968,10 +1004,9 @@ func (a *InitFromCodeAction) addToProject(ctx context.Context, targetDir string, if !isCodeDeploy { language = "docker" } else { - // Detect language from the on-disk agent yaml. The from-code scaffold path - // writes agent.yaml; the reuse path (issue #7268) may have agent.yml. Walk - // the same candidates findExistingAgentYaml uses so either case is handled. - for _, name := range agentYamlCandidates { + // Detect language from the on-disk definition. Skip manifest filenames: + // their fields are nested under template: and would not match here. + for _, name := range []string{"agent.yaml", "agent.yml"} { langDetectPath := filepath.Join(a.projectConfig.Path, targetDir, name) data, err := os.ReadFile(langDetectPath) //nolint:gosec // path from project config if err != nil { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse.go index e3751e34783..1694f25f0c7 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse.go @@ -20,28 +20,19 @@ import ( ) // agentYamlCandidates lists the file names (in priority order) scanned by the -// reuse path. The set intentionally mirrors detectLocalManifest in -// init_from_templates_helpers.go so the same on-disk files trigger either the -// manifest pipeline (when they parse as a valid manifest) or the definition -// reuse pipeline (when they do not). +// reuse path. Order matches detectLocalManifest in init_from_templates_helpers.go. var agentYamlCandidates = []string{ "agent.manifest.yaml", - "agent.manifest.yml", "agent.yaml", + "agent.manifest.yml", "agent.yml", } // findExistingAgentYaml returns the first agent yaml file found in srcDir, or -// an empty string when none exists. The scan is shallow: only srcDir itself -// is checked, never subdirectories. ErrNotExist is treated as "not found" and -// the next candidate is tried; other I/O errors propagate. +// an empty string when none exists. The scan is shallow. // -// This helper is called by RunE in init.go after detectLocalManifest has been -// given the first chance to claim the file. A path returned here is therefore -// guaranteed to not be a valid manifest (otherwise detectLocalManifest would -// have set flags.manifestPointer first); it is either a bare definition or a -// malformed manifest. runReuseDefinition distinguishes the two and produces a -// targeted error for the malformed case. +// Called from RunE after detectLocalManifest. A path returned here is either a +// bare definition or a malformed manifest; runReuseDefinition distinguishes them. func findExistingAgentYaml(srcDir string) (string, error) { for _, name := range agentYamlCandidates { candidate := filepath.Join(srcDir, name) @@ -62,17 +53,11 @@ func findExistingAgentYaml(srcDir string) (string, error) { } // runReuseDefinition wires an existing bare agent.yaml definition into -// azure.yaml without rewriting the file or running the from-code scaffolding -// prompts. It is invoked from init.go RunE alongside the manifest detection -// branch, so the user is never asked to pick an init mode when a local -// agent.yaml is already present. +// azure.yaml without rewriting the file or running the from-code prompts. // -// The function deliberately does NOT resolve a Foundry project, prompt for -// model deployments, or enrich the definition with deployment metadata — the -// issue (#7268) constrains the definition path to "less to ask and just setup -// azure.yaml". Users who need a Foundry project bound before azd deploy -// should either delete the definition and rerun init interactively, or set -// AZURE_AI_PROJECT_ID in their azd environment by hand. +// Foundry project resolution and model deployment selection are intentionally +// skipped (issue #7268: "less to ask and just setup azure.yaml"). Users who +// need a project bound before azd deploy can set AZURE_AI_PROJECT_ID by hand. func runReuseDefinition( ctx context.Context, flags *initFlags, @@ -100,24 +85,19 @@ func runReuseDefinition( displayPath, def.Name, )) - // Reuse the same project/environment bootstrap runInitFromManifest uses so - // addToProject has the project config and env it needs to write azure.yaml. projectConfig, err := ensureProject(ctx, flags, azdClient) if err != nil { return err } - // Mirror InitFromCodeAction.Run: when --src is absolute, convert it to a - // path relative to the azd project root so azure.yaml's RelativePath is - // portable across machines. + // Mirror InitFromCodeAction.Run: convert absolute --src to project-relative + // so azure.yaml's RelativePath stays portable. if flags.src != "" && filepath.IsAbs(flags.src) { relPath, err := filepath.Rel(projectConfig.Path, flags.src) if err != nil { return fmt.Errorf("failed to convert src path to relative path: %w", err) } flags.src = relPath - // srcDir was passed in by the caller; keep it in sync so the messages - // and addToProject targetDir match. srcDir = relPath } @@ -149,19 +129,14 @@ func runReuseDefinition( fmt.Println(color.HiBlackString("Reusing existing %s (name: %s).", displayPath, def.Name)) - // Run the same advisory post-init validations the scaffold path emits. validatePostInit(srcDir, def.CodeConfiguration) return nil } // loadAgentDefinitionFile parses path as a bare AgentDefinition (no surrounding -// "template:" wrapper) and validates it. It is the definition-side counterpart -// to agent_yaml.LoadAndValidateAgentManifest. -// -// The parser uses ContainerAgent so a CodeConfiguration block — when present — -// is preserved for the caller to inspect (it drives isCodeDeploy and the -// post-init language detection). +// "template:" wrapper) and runs the same schema validation the manifest +// pipeline does. func loadAgentDefinitionFile(path string) (*agent_yaml.ContainerAgent, error) { //nolint:gosec // path comes from findExistingAgentYaml against a user-controlled directory data, err := os.ReadFile(path) @@ -169,9 +144,8 @@ func loadAgentDefinitionFile(path string) (*agent_yaml.ContainerAgent, error) { return nil, fmt.Errorf("read %s: %w", path, err) } - // Reject files that are actually manifests. A valid manifest would have been - // routed upstream; an invalid one reaching here is most likely a malformed - // template authored by the user, so the error message points at the cause. + // Reject manifest-shaped files. A valid manifest would have been routed + // upstream; an invalid one reaching here is a malformed template. var top map[string]any if err := yaml.Unmarshal(data, &top); err != nil { return nil, fmt.Errorf("parse %s: %w", path, err) @@ -183,10 +157,6 @@ func loadAgentDefinitionFile(path string) (*agent_yaml.ContainerAgent, error) { ) } - // Run the same schema validation the manifest pipeline runs on the unwrapped - // template body. This catches missing/invalid kind, missing name, and the - // kind-specific structural checks before the bare definition is wired into - // azure.yaml. if err := agent_yaml.ValidateAgentDefinition(data); err != nil { return nil, err } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse_test.go index 56765a1b329..f0f36d7f520 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code_reuse_test.go @@ -14,12 +14,9 @@ import ( "github.com/stretchr/testify/require" ) -// These tests cover the helpers that back the issue #7268 reuse path. -// runReuseDefinition itself is exercised by manual e2e (see -// cli/azd/docs/design/azure-ai-agent-init-reuse-agent-yaml.md §7) because a -// full unit test would require mocking the azd Project gRPC service in -// addition to the Environment/Prompt mocks that already exist; that -// investment is deferred until the next time a reuse-related change lands. +// Helpers backing the issue #7268 reuse path. runReuseDefinition's happy path +// is covered by manual e2e; only its failure branches are unit-tested here +// because they short-circuit before any azd gRPC calls. func TestFindExistingAgentYaml(t *testing.T) { t.Parallel() @@ -143,11 +140,8 @@ func TestLoadAgentDefinitionFile(t *testing.T) { }) } -// TestRunReuseDefinition_InvalidFileReturnsStructuredError covers the -// "malformed template-wrapped YAML returns CodeInvalidAgentManifest" case the -// reviewer explicitly asked for. We invoke the failure path directly because -// it short-circuits before any azd gRPC calls are made and so does not need a -// full client mock. +// Malformed YAML must surface as CodeInvalidAgentManifest. Runs against the +// failure path so no gRPC mock is needed. func TestRunReuseDefinition_InvalidFileReturnsStructuredError(t *testing.T) { t.Parallel() @@ -157,33 +151,28 @@ func TestRunReuseDefinition_InvalidFileReturnsStructuredError(t *testing.T) { err := runReuseDefinition(t.Context(), &initFlags{}, nil, nil, dir, path) require.Error(t, err) - var localErr *azdext.LocalError - require.True(t, errors.As(err, &localErr), "expected *azdext.LocalError, got %T", err) + localErr, ok := errors.AsType[*azdext.LocalError](err) + require.True(t, ok, "expected *azdext.LocalError, got %T", err) require.Equal(t, exterrors.CodeInvalidAgentManifest, localErr.Code) require.NotEmpty(t, localErr.Suggestion) - // Error message and suggestion should both reference the file by name. require.Contains(t, localErr.Message, "agent.yaml") require.Contains(t, localErr.Suggestion, "agent.yaml") } -// TestRunReuseDefinition_RejectsManifestShapedFile covers the case where the -// scan picks up a file that looks like a manifest but failed upstream -// validation. The user sees a targeted error instead of falling through to -// scaffolding prompts. +// A manifest-shaped file that failed upstream validation must produce a +// targeted error here, not fall into scaffolding. func TestRunReuseDefinition_RejectsManifestShapedFile(t *testing.T) { t.Parallel() dir := t.TempDir() - // Manifest-shaped but missing required fields, so detectLocalManifest would - // have rejected it upstream and our reuse scan picks it up here. path := writeReuseTestFile(t, dir, "agent.manifest.yaml", "template:\n # intentionally incomplete\n") err := runReuseDefinition(t.Context(), &initFlags{}, nil, nil, dir, path) require.Error(t, err) - var localErr *azdext.LocalError - require.True(t, errors.As(err, &localErr)) + localErr, ok := errors.AsType[*azdext.LocalError](err) + require.True(t, ok) require.Equal(t, exterrors.CodeInvalidAgentManifest, localErr.Code) require.Contains(t, localErr.Message, "agent.manifest.yaml", "error message must name the actual file, not a hardcoded agent.yaml")