-
Notifications
You must be signed in to change notification settings - Fork 313
feat(azure.ai.agents): agent-driven init pre-flow + --from-code routing #8376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
therealjohn
wants to merge
9
commits into
Azure:main
Choose a base branch
from
therealjohn:feat/azure-ai-agents-init-preflow
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
d87fe2b
feat(azure.ai.agents): agent-driven init pre-flow + --from-code routing
therealjohn 1e79032
Merge remote-tracking branch 'upstream/main' into feat/azure-ai-agent…
therealjohn a09f726
fix(azure.ai.agents): address PR review comments
therealjohn 8579c39
fix(azure.ai.agents): satisfy golangci-lint (errorlint, gofmt, gosec,…
therealjohn ea14677
fix(azure.ai.agents): gate init preflow to greenfield only
therealjohn 0969539
Merge branch 'main' into therealjohn/feat/azure-ai-agents-init-preflow
trangevi ee44911
linter
trangevi b4b58fd
fix: update command syntax for installing skills in init pre-flow and…
therealjohn d44aea3
feat(help): add custom help output for agents with sections on gettin…
therealjohn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
92 changes: 92 additions & 0 deletions
92
cli/azd/extensions/azure.ai.agents/internal/cmd/ensure_project_helpers_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // TestIsCwdEmptyForInit covers the empty/non-empty branch ensureProject | ||
| // uses to decide whether to scaffold the full starter template or just | ||
| // write a minimal azure.yaml inline. | ||
| func TestIsCwdEmptyForInit(t *testing.T) { | ||
| t.Run("empty dir reports empty", func(t *testing.T) { | ||
| dir := t.TempDir() | ||
| empty, err := isCwdEmptyForInit(dir) | ||
| require.NoError(t, err) | ||
| assert.True(t, empty) | ||
| }) | ||
|
|
||
| t.Run("dir with a regular file reports non-empty", func(t *testing.T) { | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.WriteFile(filepath.Join(dir, "x.txt"), []byte("x"), 0o644)) //nolint:gosec | ||
| empty, err := isCwdEmptyForInit(dir) | ||
| require.NoError(t, err) | ||
| assert.False(t, empty) | ||
| }) | ||
|
|
||
| t.Run("dir with only an installed skill folder reports non-empty", func(t *testing.T) { | ||
| // This is the case that broke after Round 2: pre-flow installs | ||
| // `.agents/skills/azd-ai-skill/...`, then `azd ai agent init -m | ||
| // <url> --no-prompt` runs in that dir. ensureProject must NOT | ||
| // dispatch `azd init -t` here (the workflow auto-declines its | ||
| // "directory not empty" prompt under --no-prompt and fails). | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, ".agents", "skills", "azd-ai-skill"), 0o755)) //nolint:gosec | ||
| empty, err := isCwdEmptyForInit(dir) | ||
| require.NoError(t, err) | ||
| assert.False(t, empty, | ||
| "a dir containing the installed AZD AI skill MUST report non-empty so "+ | ||
| "ensureProject takes the minimal-azure.yaml path instead of the "+ | ||
| "starter-template scaffold workflow") | ||
| }) | ||
| } | ||
|
|
||
| // TestWriteMinimalAzureYaml covers the contract ensureProject relies on | ||
| // when the cwd is non-empty: a 3-line azure.yaml exists afterwards with | ||
| // a derived name and the schema comment, and a pre-existing file is | ||
| // never clobbered. | ||
| func TestWriteMinimalAzureYaml(t *testing.T) { | ||
| t.Run("writes a 3-line azure.yaml with derived name", func(t *testing.T) { | ||
| dir := t.TempDir() | ||
| // Name the dir something sanitizeAgentName accepts as-is so we | ||
| // can assert on the substituted name without depending on the | ||
| // full sanitization rules. | ||
| projDir := filepath.Join(dir, "my-agent-proj") | ||
| require.NoError(t, os.MkdirAll(projDir, 0o755)) //nolint:gosec | ||
|
|
||
| require.NoError(t, writeMinimalAzureYaml(projDir)) | ||
|
|
||
| body, err := os.ReadFile(filepath.Join(projDir, "azure.yaml")) //nolint:gosec | ||
| require.NoError(t, err) | ||
|
|
||
| text := string(body) | ||
| assert.Contains(t, text, "name: my-agent-proj", | ||
| "name MUST be derived from the cwd basename so `azd` picks the right project name") | ||
| assert.Contains(t, text, "yaml-language-server", | ||
| "schema comment MUST be present so editors light up YAML completions") | ||
| assert.NotContains(t, text, "services:", | ||
| "minimal azure.yaml MUST NOT seed a services section -- addToProject does that later") | ||
| }) | ||
|
|
||
| t.Run("never clobbers an existing azure.yaml", func(t *testing.T) { | ||
| dir := t.TempDir() | ||
| existing := "name: existing-project\nservices: {}\n" | ||
| path := filepath.Join(dir, "azure.yaml") | ||
| require.NoError(t, os.WriteFile(path, []byte(existing), 0o644)) //nolint:gosec | ||
|
|
||
| require.NoError(t, writeMinimalAzureYaml(dir), | ||
| "writeMinimalAzureYaml on a dir that already has an azure.yaml must be a safe no-op (the existing file is what `Project().Get()` should see)") | ||
|
|
||
| body, err := os.ReadFile(path) //nolint:gosec | ||
| require.NoError(t, err) | ||
| assert.Equal(t, existing, string(body), | ||
| "existing azure.yaml MUST be preserved byte-for-byte; clobbering it would lose the user's project config") | ||
| }) | ||
| } |
160 changes: 160 additions & 0 deletions
160
cli/azd/extensions/azure.ai.agents/internal/cmd/ext_lookup.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| // ext_lookup.go provides helpers for talking to the azd host's extension | ||
| // layer from inside this extension. Two responsibilities: | ||
| // | ||
| // 1. Detect whether a sibling extension (e.g. azure.ai.docs) is | ||
| // installed locally so a cross-extension dispatch is safe. | ||
| // 2. Run a child `azd` subprocess to invoke another extension's | ||
| // command (skill install, ext install, etc.). | ||
| // | ||
| // Both helpers shell out to `azd` because the gRPC SDK does not (yet) | ||
| // expose extension-management RPCs from inside an extension. Pattern | ||
| // matches the existing exec.Command("azd", ...) sites in | ||
| // microsoft.azd.extensions and microsoft.azd.concurx. | ||
| // | ||
| // # Why pre-check instead of relying on azd's built-in auto-install | ||
| // | ||
| // `azd` ships an auto-install feature (cli/azd/cmd/auto_install.go) | ||
| // that detects when a command belongs to an uninstalled extension and | ||
| // offers to install it. In `--no-prompt` mode `console.Confirm` returns | ||
| // the prompt's DefaultValue (`true` for the auto-install prompt), so in | ||
| // theory shelling out to `azd ai doc skills install --no-prompt` would | ||
| // silently install azure.ai.docs and re-run the command. | ||
| // | ||
| // In practice the re-run breaks for our use case. The pre-parser | ||
| // `extractFlagsWithValues` only knows about flags declared on the | ||
| // CURRENT command tree -- extension-specific flags like `--target` and | ||
| // `--path` do not exist until azure.ai.docs is installed. So the | ||
| // pre-parser treats `copilot` (a `--target` value) and `json` (an | ||
| // `--output` value) as positional args, mis-detects the command, and | ||
| // the re-run fails with `unknown flag: --target` even though the | ||
| // extension was just installed successfully. | ||
| // | ||
| // Pre-checking with `azd ext list -o json` + an explicit consent | ||
| // prompt + an explicit `azd ext install` shell-out avoids this entirely | ||
| // because we only dispatch the install command once azure.ai.docs is | ||
| // known to be present. As a bonus the parent process owns the consent | ||
| // UX (single clean prompt) instead of the child emitting a surprise | ||
| // warning mid-flow, and CI users get one clear "install azure.ai.docs" | ||
| // hint from us instead of the two scattered messages auto-install | ||
| // produces. | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "io" | ||
| "os/exec" | ||
| "strings" | ||
| ) | ||
|
|
||
| // extListItem mirrors the wire shape emitted by `azd ext list -o json`. | ||
| // Only the fields we need are decoded; the SDK adds extra fields freely. | ||
| type extListItem struct { | ||
| ID string `json:"id"` | ||
| Namespace string `json:"namespace"` | ||
| InstalledVersion string `json:"installedVersion"` | ||
| } | ||
|
|
||
| // extLookup describes the install state of one sibling extension. The | ||
| // shape stays small on purpose -- callers only need to know "is it | ||
| // installed?" and "what's the namespace I'd invoke?" (for nicer error | ||
| // messages when the answer is no). | ||
| type extLookup struct { | ||
| ID string | ||
| Namespace string | ||
| Installed bool | ||
| } | ||
|
|
||
| // azdRunner abstracts the exec.Command wiring so tests can inject a | ||
| // fake. Default production runner is osAzdRunner below. | ||
| type azdRunner interface { | ||
| // Run executes `azd <args...>` with the given stdout/stderr writers | ||
| // and returns the process error (nil on exit 0). Cancellation is | ||
| // honored when ctx is canceled. | ||
| Run(ctx context.Context, args []string, stdout, stderr io.Writer) error | ||
| // Output executes `azd <args...>` and returns combined stdout + | ||
| // error (mirrors exec.Command.Output). Used by the JSON-parsing | ||
| // helpers where streaming is not needed. | ||
| Output(ctx context.Context, args []string) ([]byte, error) | ||
| } | ||
|
|
||
| // osAzdRunner is the default production runner. | ||
| type osAzdRunner struct{} | ||
|
|
||
| func (osAzdRunner) Run(ctx context.Context, args []string, stdout, stderr io.Writer) error { | ||
| // #nosec G204 -- invoking the azd CLI by fixed name with caller-supplied args is intentional. | ||
| cmd := exec.CommandContext(ctx, "azd", args...) | ||
| cmd.Stdout = stdout | ||
| cmd.Stderr = stderr | ||
| return cmd.Run() | ||
| } | ||
|
|
||
| func (osAzdRunner) Output(ctx context.Context, args []string) ([]byte, error) { | ||
| // #nosec G204 -- invoking the azd CLI by fixed name with caller-supplied args is intentional. | ||
| cmd := exec.CommandContext(ctx, "azd", args...) | ||
| return cmd.Output() | ||
| } | ||
|
|
||
| // lookupExtension returns the install state for the given extension ID. | ||
| // Returns (lookup, nil) when the listing succeeds and the ID is found; | ||
| // returns (lookup with Installed=false, nil) when listing succeeds and | ||
| // the ID is missing; returns (zero, err) when the listing itself fails. | ||
| func lookupExtension(ctx context.Context, runner azdRunner, id string) (extLookup, error) { | ||
| out, err := runner.Output(ctx, []string{"ext", "list", "-o", "json"}) | ||
| if err != nil { | ||
| return extLookup{}, fmt.Errorf("run `azd ext list -o json`: %w", err) | ||
| } | ||
|
|
||
| var items []extListItem | ||
| if err := json.Unmarshal(out, &items); err != nil { | ||
| return extLookup{}, fmt.Errorf("parse `azd ext list` output: %w", err) | ||
| } | ||
|
|
||
| for _, it := range items { | ||
| if !strings.EqualFold(it.ID, id) { | ||
| continue | ||
| } | ||
| return extLookup{ | ||
| ID: it.ID, | ||
| Namespace: it.Namespace, | ||
| Installed: strings.TrimSpace(it.InstalledVersion) != "", | ||
| }, nil | ||
| } | ||
|
|
||
| // Not present in the catalog at all (no registry source advertises | ||
| // it, or the user has not added the right source). Return a lookup | ||
| // with Installed=false so the caller surfaces an "install it" hint. | ||
| return extLookup{ID: id, Installed: false}, nil | ||
| } | ||
|
|
||
| // installExtension shells out to `azd ext install <id>`. Streams output | ||
| // through stdout/stderr so the user sees install progress live. Used | ||
| // when the user opts in to auto-installing a missing dependency. | ||
| func installExtension(ctx context.Context, runner azdRunner, id string, stdout, stderr io.Writer) error { | ||
| args := []string{"ext", "install", id} | ||
| if err := runner.Run(ctx, args, stdout, stderr); err != nil { | ||
| return fmt.Errorf("install extension %q: %w", id, err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // runChildAzd invokes `azd <args...>` with stdout/stderr streamed | ||
| // through. Returns the process error verbatim so the caller can pattern- | ||
| // match on exit codes / unwrap exec.ExitError when needed. | ||
| // | ||
| // Used by the init pre-flow to dispatch `azd ai doc skills install`. | ||
| // Always pass --no-prompt + --output json from the caller; this helper | ||
| // makes no assumption about flags so it can be reused for other | ||
| // cross-extension calls in the future. | ||
| func runChildAzd(ctx context.Context, runner azdRunner, args []string, stdout, stderr io.Writer) error { | ||
| return runner.Run(ctx, args, stdout, stderr) | ||
| } | ||
|
|
||
| // defaultAzdRunner is the package-level production runner. Tests | ||
| // construct their own runner and call the *With helpers directly. | ||
| var defaultAzdRunner azdRunner = osAzdRunner{} | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.