Azure / Azure/azure-dev

Consolidate duplicated code from AI extensions into azd core and extension SDK

Open
#7,613 0 comments 0 reactions 0 assignees View on GitHub
ai area/code-improvements enhancement
Dominant language
Go
Stars
569
Forks
364
Avg merge
2d 19h
Merged PRs (30d)
136

Description

## Summary

An analysis of all 5 AI-related extensions in `cli/azd/extensions/` reveals significant code duplication and generic utilities that should be moved to azd core (`cli/azd/pkg/`) or the extension SDK (`pkg/azdext/`). This would reduce maintenance burden, improve consistency, and shrink extension binary sizes.

**Extensions analyzed:** `azure.ai.agents`, `azure.ai.finetune`, `azure.ai.models`, `microsoft.azd.ai.builder`, `azure.coding-agent`

**Update (2026-04-23):** Deep-dive verification against the codebase confirmed all 15 original findings and uncovered **17 additional issues** (items 16-32), including a security concern, behavioral drift bugs already shipped, and architectural inconsistencies. The phased plan has been restructured to address correctness/security first and resolve module strategy earlier.

---

## Priority 0: Security & Correctness (NEW)

### 16. Security: `InsecureAllowCredentialWithHTTP` Enabled
**In:** azure.ai.finetune

`internal/providers/factory/provider_factory.go:99` sets `InsecureAllowCredentialWithHTTP: true`, allowing credentials to be sent over plaintext HTTP.
**Fix:** Disable by default; gate behind explicit dev-only flag.

### 17. `AZD_NO_PROMPT` Env-Var Handler Missing in 4 of 5 Extensions
**In:** azure.ai.models, azure.ai.finetune, microsoft.azd.ai.builder, azure.coding-agent

Only `azure.ai.agents` wires the `AZD_NO_PROMPT` PreRunE handler in `root.go`. The other 4 AI extensions silently ignore this env var, meaning non-interactive/CI mode doesn't propagate correctly.
**Fix:** Part of the broader root command consolidation (item 28).

### 18. `FORCE_COLOR` Init Handler Missing in 2 of 5 Extensions
**In:** azure.ai.agents, azure.ai.finetune

The `FORCE_COLOR` env handling in `main.go` exists in models, ai.builder, and coding-agent but is absent from agents and finetune. This causes inconsistent color behavior in CI/pipelines.
**Fix:** Part of root command consolidation or `azdext.Run()` adoption.

### 19. ARM Correlation/User-Agent Policy Missing in ai.builder
**In:** microsoft.azd.ai.builder

`azure_client.go` passes `nil` ClientOptions (vs agents which passes `NewArmClientOptions()` with correlation + UA policy). ARM calls from ai.builder lose telemetry correlation silently.
**Fix:** Use shared ARM client options (item 28 SDK helper).

### 20. `parseProjectEndpoint` Inconsistent Validation
**In:** azure.ai.models, azure.ai.finetune

- models (`init.go:492`): strict — requires exactly 3 path segments + non-empty project name
- finetune (`init.go:198`): loose — allows 3+ segments, no non-empty check
- Neither validates HTTPS scheme

**Fix:** Centralize endpoint parser with consistent invariant enforcement.

---

## Priority 1: Duplicated Across 3+ Extensions

### 1. Foundry Client / Project Endpoint Parsing
**Duplicated in:** azure.ai.agents, azure.ai.models, microsoft.azd.ai.builder

| Extension | File |
|-----------|------|
| azure.ai.agents | `internal/pkg/azure/foundry_projects_client.go` (290 lines) |
| azure.ai.models | `internal/client/foundry_client.go` (471 lines) |
| microsoft.azd.ai.builder | `internal/pkg/azure/ai/model_catalog.go` (389 lines) |

**What:** HTTP client for AI Foundry endpoints, project discovery, model listing, async polling, nextLink pagination.

**Note:** These three clients hit *different API surfaces* (agents: connections data-plane, models: models data-plane, ai.builder: ARM management-plane). Consolidation should target the **shared construction pattern** (pipeline setup, credential scoping to `https://ai.azure.com/.default`, URL construction, API version management), not necessarily merge all operations into one client.

**Target:** New `pkg/ai/foundry/` package in core, or extend existing `pkg/ai/`.

### 2. Subscription / Location / Environment Bootstrap
**Duplicated in:** ALL 5 extensions

| Extension | File | Functions |
|-----------|------|-----------|
| azure.ai.agents | `internal/cmd/init_foundry_resources_helpers.go` | `ensureSubscription`, `ensureLocation`, `extractProjectDetails` |
| azure.ai.models | `internal/cmd/init.go` | `ensureEnvironment`, `ensureAzureContext`, `buildProjectEndpoint`, `parseProjectEndpoint` |
| azure.ai.finetune | `internal/cmd/init.go` | Similar bootstrap flow |
| microsoft.azd.ai.builder | `internal/cmd/start.go` | `ensureAzureContext` |
| azure.coding-agent | `internal/cmd/coding_agent_config.go` | Subscription/env resolution |

**What:** Prompt for subscription, resolve location, create/load environment, set env values.
**Target:** `pkg/azdext/` SDK helpers.

### 3. Credential Creation Boilerplate
**Duplicated in:** ALL 5 extensions

Every extension manually calls `azidentity.NewAzureDeveloperCLICredential` instead of using the existing `TokenProvider` in `pkg/azdext/token_provider.go`.

**Scope is larger than initially documented:** Verification found **27 call sites** across all 5 AI extensions:
- azure.ai.agents: **8 call sites** (with inconsistent TenantID/AllowedTenants — some empty options, some with wildcard `["*"]`)
- azure.ai.models: **7 call sites**
- azure.ai.finetune: **6 call sites**
- microsoft.azd.ai.builder: **1 call site**
- azure.coding-agent: **1 call site**

**Zero extensions use `azdext.TokenProvider`** despite it being available in the SDK.

**Target:** Shared credential factory via `azdext.TokenProvider` with consistent tenant resolution + AllowedTenants policy.

### 4. Output / Table Formatting
**Duplicated in:** azure.ai.finetune, azure.ai.models

| Extension | File |
|-----------|------|
| azure.ai.finetune | `internal/utils/output.go` (269 lines) |
| azure.ai.models | `internal/utils/output.go` (183 lines) |

**What:** Generic reflection-based table/JSON/YAML printing. Finetune is a superset (adds YAML, `time.Duration` handling). Core functions (`printSliceAsTable`, `getTableColumns`, `formatFieldValue`) are near-identical.
**Target:** Extend `pkg/azdext/output.go` and `pkg/output/table.go`.

### 21. AzureContext Reconstruction Pattern (NEW)
**Duplicated in:** azure.ai.agents, azure.ai.models, azure.ai.finetune, microsoft.azd.ai.builder

Every extension does the same dance: `GetCurrent` → get environment name → `GetValues` → get all env key-values → manually build `azdext.AzureContext{Scope: {TenantId: envValueMap["AZURE_TENANT_ID"], ...}}`.

Found in:
- `azure.ai.agents/internal/cmd/init_foundry_resources_helpers.go:641-661`
- `azure.ai.finetune/internal/cmd/init.go:569-586`
- `azure.ai.models/internal/cmd/init.go:335+`
- `microsoft.azd.ai.builder/internal/cmd/start.go:665-684`

**Target:** `azdext.GetAzureContextFromEnvironment(ctx, client)`.

### 22. No Extension Uses `azdext.ResilientClient` (NEW)
**In:** azure.ai.agents, azure.ai.models (confirmed), likely others

The SDK provides `azdext.ResilientClient` (`resilient_http_client.go`) with retry/backoff, but extensions create raw `http.Client` instances instead:
- models `foundry_client.go:75,209`
- agents `invoke.go:299,433,503,596,834,904`
- agents `helpers.go:333`

**Note:** Not all current call patterns may be drop-in compatible (loopback calls, streaming, custom redirect handling). Adoption should be evaluated per call site.
**Target:** Migrate applicable HTTP calls to `azdext.ResilientClient`; extend client for missing capabilities.

### 23. Timeout Policies Inconsistent (NEW)
**In:** azure.ai.agents, azure.ai.models

| Timeout | Location |
|---------|----------|
| 10s | `agents/internal/cmd/helpers.go:333` |
| 30s | `models/internal/client/foundry_client.go:75`, `agents/invoke.go:834,904` |
| 5 min | `models/internal/azcopy/installer.go:96` |
| **None** | `agents/internal/pkg/agents/agent_api/operations.go:610` |

**Target:** Central timeout defaults by operation class; forbid zero-timeout clients.

### 24. User-Agent Header Missing from Raw HTTP Calls (NEW)
**In:** azure.ai.agents, azure.ai.models

Many raw HTTP calls set no User-Agent (e.g., models `foundry_client.go:91/129/172`, agents `invoke.go:297`). Some paths do have UA via SDK policy. Inconsistent observability.
**Target:** Shared HTTP request builder that always sets UA + correlation headers.

### 25. Resource ID Parser Duplicated Within Agents (NEW)
**In:** azure.ai.agents (internal duplication)

- `internal/project/agent_identity_rbac.go:413-421` — `extractSubscriptionID`
- `internal/cmd/init_foundry_resources_helpers.go:90-98` — `extractSubscriptionId`

Models/finetune use `arm.ParseResourceID` instead — that should be the standard.
**Target:** Consolidate to `arm.ParseResourceID` within agents.

---

## Priority 2: Generic Utilities (Not AI-Specific)

### 5. AzCopy Tool Integration
**In:** azure.ai.models (but entirely generic)

- `internal/azcopy/installer.go` — download, extract, platform detection (253 lines)
- `internal/azcopy/runner.go` — exec wrapper (323 lines)

**What:** Tool discovery/install/run — same pattern as `pkg/tools/` (docker, kubectl, gh, etc.).
**Target:** `pkg/tools/azcopy/`

### 6. RBAC / Role Assignment Workflow
**In:** azure.ai.agents, azure.coding-agent

| Extension | File | What |
|-----------|------|------|
| azure.ai.agents | `internal/project/agent_identity_rbac.go` | Parse resource IDs, poll SP visibility, create role assignments, verify propagation (422 lines) |
| azure.coding-agent | `internal/cmd/coding_agent_config.go` | `createFederatedCredential` — properly imports core `pkg/armmsi` but adds credential adapter |

**Note:** coding-agent properly imports `pkg/armmsi.ArmMsiService` (compile-time check at `interfaces.go:15`), not a full duplication. The issue is the credential adapter pattern.
**Target:** Extend existing `pkg/armmsi/` and `pkg/entraid/`.

### 7. ARM Client Wrappers (Locations, Subscriptions)
**In:** azure.ai.agents, microsoft.azd.ai.builder

**Verified as byte-for-byte identical** between:
- `azure.ai.agents/internal/pkg/azure/azure_client.go` (56 lines)
- `microsoft.azd.ai.builder/internal/pkg/azure/azure_client.go` (57 lines)

Only difference: agents passes `NewArmClientOptions()`, ai.builder passes `nil` (see item 19).
**Target:** Already exists in `pkg/account/subscriptions.go` and `pkg/azureutil/location.go`.

### 8. Retry Utilities
**In:** azure.ai.finetune (direct utility), azure.ai.models (manual polling loops)

**Note:** Only finetune has a direct retry wrapper (`internal/utils/retry.go`, 42 lines). Models has manual polling loops (`foundry_client.go:257`). Agents/models pull `go-retry` as an indirect dependency only.
**Target:** Extend `pkg/azdext/resilient_http_client.go` retry helpers.

### 9. Structured Error Handling
**In:** azure.ai.agents

- `internal/exterrors/errors.go` (227 lines) + `codes.go` (156 lines)

**Note:** `exterrors` already builds on SDK types (`azdext.LocalError`, `azdext.ServiceError`). What needs promoting are the **factory functions** (`Validation(...)`, `Auth(...)`) and gRPC error converters.
**Target:** Promote factory functions to `pkg/azdext/`.

### 10. Decision Tree / Prompt Orchestration
**In:** microsoft.azd.ai.builder

- `internal/pkg/qna/decision_tree.go` — branching question flow with state management (235 lines)
- `internal/pkg/qna/prompt.go` — prompt adapters over azdext APIs (186 lines)

**Target:** `pkg/azdext/` — this fills an extension framework gap.

---

## Priority 3: Shared AI Domain Types & Architecture

### 11. AI Model Types
**In:** azure.ai.finetune (`pkg/models/`), azure.ai.models (`pkg/models/`), core (`pkg/azdext/ai_model.pb.go`)

Largely **domain-specific** (finetune: finetuning job DTOs, models: upload/registration DTOs) — low direct overlap.
**Target:** Extend `pkg/ai/` shared types where genuine overlap exists.

### 12. Model / Region Selection Logic
**In:** azure.ai.agents, microsoft.azd.ai.builder

Core already has `pkg/ai/model_service.go`. Consolidate there.

### 26. `microsoft.azd.ai.builder` Has No `go.mod` (NEW — Architectural)
**In:** microsoft.azd.ai.builder

ai.builder is the **only** AI extension compiled as part of the main azd module (no `go.mod`). The other 4 are independently versioned. This means:
- ai.builder cannot import from other extension modules
- Shared extension libraries are structurally blocked without a module strategy decision
- It explains why ai.builder duplicates `azure_client.go` from agents

**Fix:** Either promote ai.builder to its own module or establish a shared extension library strategy. **This is a prerequisite for serious cross-extension consolidation.**

### 27. Dependency Version Divergence Is Already Live (NEW)
**In:** ALL 4 extensions with `go.mod`

| Library | agents | models | finetune | coding-agent |
|---------|--------|--------|----------|--------------|
| `azd` | **v1.23.14** | v1.23.13 | v1.23.13 | v1.23.13 |
| `azcore` | **v1.21.0** | v1.20.0 | **v1.21.0** | v1.20.0 |
| `azidentity` | **v1.14.0-beta.3** | v1.13.1 | v1.13.1 | v1.13.1 |
| `grpc` | **v1.80.0** | v1.79.3 | v1.79.3 | v1.79.3 |
| `armcognitiveservices` | **v2.0.0** | v1.8.0 | v1.8.0 | n/a |

Key risks:
- `armcognitiveservices` v1 vs v2 — **major version split**, types not interchangeable
- `azidentity` beta (v1.14.0-beta.3) in agents vs stable everywhere else

**Fix:** Introduce `go.work` workspace or enforce shared dependency baseline.

---

## Priority 4: Extension Framework & Boilerplate

### 13. Git/GitHub CLI Wrappers
**In:** azure.coding-agent — `internal/cmd/interfaces.go` missing only `ListRemotes` from core.
**Fix:** Add `ListRemotes` to core `pkg/tools/git/git.go`.

### 14. Browser Launch Helper
**In:** azure.coding-agent — `internal/cmd/debt.go` (`openWithDefaultBrowser`) is a near-copy of core `cmd/util.go` (self-documented with comment: *"this is an internal func from 'azd'. Copied here"*).
**Fix:** Export core helper to `pkg/azdext/` or `pkg/osutil/`.

### 15. File/Directory Utilities
**In:** azure.ai.agents (`internal/cmd/init_copy.go`), microsoft.azd.ai.builder (`internal/pkg/util/util.go`)
Core already has `pkg/osutil/file.go` with `IsDirEmpty`. Extensions should use it.

### 28. Cobra Root Command Boilerplate (NEW)
**In:** ALL 5 AI extensions (scope varies per component)

| Component | Present in |
|-----------|-----------|
| `newVersionCommand` | All 5 |
| `newMetadataCommand` | 4/5 (not ai.builder — capability gap) |
| `rootFlagsDefinition` (--debug, --no-prompt) | All 5 |
| `AZD_NO_PROMPT` PreRunE handler | 1/5 (only agents) |
| `SilenceUsage/SilenceErrors/SetHelpCommand(Hidden)` | All 5 |
| `listen.go` wrapper | 2/5 (agents, demo) |

**Target:** `azdext.NewRootCommand(RootOptions{ID, Use, Short, Subcommands...})` that wires all standard components automatically.

### 29. Build Script Duplication (NEW)
**In:** ALL 5 AI extensions

`build.ps1` (~61 lines), `build.sh` (~50 lines), `ci-build.ps1` near-identical across all 5. Only differences: `-X` ldflags path and one extension's `$PSNativeCommandArgumentPassing`. `cli/azd/extensions/scripts/` already exists but is unused.
**Fix:** Replace per-extension scripts with shims. Standardize version package location first.

### 30. Mock Infrastructure Duplication (NEW)
**In:** azure.coding-agent

5 mockgen-generated mock files (~49 KB) for interfaces from azd core. As more extensions adopt mockgen, this grows.
**Target:** Ship `pkg/azdext/azdexttest/` with canonical mocks.

### 31. `extension.yaml` Schema Gaps (NEW)
**In:** Multiple extensions

- Only 2/5 AI extensions reference `$schema` (others lose IDE validation)
- `requiredAzdVersion` only set on agents (others can install against incompatible azd)
- No `versionPackage` field (blocks build-script consolidation)

**Fix:** Extend schema; make `requiredAzdVersion` mandatory.

### 32. Environment Value Loading Boilerplate (NEW)
**In:** azure.ai.agents, azure.ai.models, azure.ai.finetune, microsoft.azd.ai.builder (4/5)

The pattern `GetValues → range KeyValues → build map[string]string` is duplicated. Finetune has its own utility (`internal/utils/environment.go:26-47`); others do it inline.
**Target:** `azdext.EnvValueMap(ctx, client)`.

---

## Compatibility Considerations

### Environment Variable & Flag Renames
Items 18-19 from the original issue proposed standardizing env vars (`AZURE_AI_PROJECT_ENDPOINT` vs `AZURE_PROJECT_ENDPOINT`) and flags (`--project-id` vs `--project-resource-id`). These need:
- Backward-compatible fallback period (accept both old and new names)
- Deprecation warnings pointing users to the new name
- Minimum one release cycle before removing the old name
- Documentation updates for all affected extensions

---

## Root Cause: Extension SDK Gaps

| Gap | Impact | Extensions Affected |
|-----|--------|-------------------|
| No Azure context bootstrap helper | Every init flow duplicated | All 5 |
| No shared Foundry/AI client construction | 3 separate HTTP clients | agents, models, ai.builder |
| `TokenProvider` exists but unused by all | 27 manual `azidentity` calls | All 5 |
| No `GetAzureContextFromEnvironment()` | 4 duplicate implementations | agents, models, finetune, ai.builder |
| No `EnvValueMap()` builder | Inline everywhere | 4/5 |
| No `NewRootCommand()` | ~600 LOC boilerplate | All 5 |
| No `NewArmClientOptions()` | Telemetry silently broken | ai.builder |
| No `ResilientClient` adoption | Raw http.Client everywhere | agents, models |
| No shared mock package | ~49KB per extension | coding-agent (growing) |

---

## Phased Rollout Plan

### Phase 0 — Security & Correctness Fixes
*Bug fixes only. No API changes, no renames.*

- [ ] **16.** Remove `InsecureAllowCredentialWithHTTP: true` from finetune
- [ ] **19.** Add ARM correlation/UA policy to ai.builder's `azure_client.go`
- [ ] **20.** Fix `parseProjectEndpoint` validation inconsistency + add HTTPS check
- [ ] **25.** Consolidate ARM resource ID parsers within agents to `arm.ParseResourceID`

### Phase 1 — Module Strategy & Dependency Alignment
*Prerequisite for cross-extension consolidation. Resolve structural blockers first.*

- [ ] **26.** Decide module strategy: `go.work` workspace OR ai.builder gets own `go.mod`
- [ ] **27.** Pin all extensions to same `azcore`, `azidentity` (stable, not beta), `grpc` versions
- [ ] **27.** Resolve `armcognitiveservices` v1→v2 migration
- [ ] **31.** Extend `extension.yaml` schema (`requiredAzdVersion` mandatory)

### Phase 2 — SDK Adoption (Use What Already Exists)
*Migrate extensions to use existing SDK/core capabilities.*

- [ ] **3.** Migrate credential call sites to `azdext.TokenProvider`
- [ ] **22.** Migrate applicable HTTP calls to `azdext.ResilientClient`
- [ ] **7.** Replace duplicated `azure_client.go` with core `pkg/account/` + `pkg/azureutil/`
- [ ] **15.** Import core `IsDirEmpty` instead of reimplementing
- [ ] **14.** Export and import core browser launch helper
- [ ] **13.** Add `ListRemotes` to core `pkg/tools/git/git.go`

### Phase 3 — SDK Enrichment (Fill Gaps)
*New SDK helpers to eliminate highest-impact duplication.*

- [ ] **21.** `azdext.GetAzureContextFromEnvironment()` helper
- [ ] **32.** `azdext.EnvValueMap()` builder
- [ ] **28.** `azdext.NewRootCommand()` — unified root/version/metadata (also fixes items 17, 18)
- [ ] **24.** Shared HTTP request builder with UA + correlation + timeout defaults (also fixes 23)
- [ ] **2.** `azdext.EnsureSubscription/Location/Environment()` helpers
- [ ] **9.** Promote `exterrors` factory functions to SDK
- [ ] **10.** QnA/decision tree framework → `pkg/azdext/`

### Phase 4 — Shared AI Layer
*Consolidate AI-specific logic into core packages.*

- [ ] **1.** New `pkg/ai/foundry/` — shared client construction (pipeline, credential, baseURL)
- [ ] **12.** Consolidate model catalog + SKU selection into `pkg/ai/model_service.go`
- [ ] **8.** Shared retry/polling utility
- [ ] **5.** `pkg/tools/azcopy/` tool integration
- [ ] **6.** Extend `pkg/armmsi/` for RBAC workflow

### Phase 5 — Framework Polish & Cleanup
*Build scripts, mocks, output, remaining duplication.*

- [ ] **29.** Consolidate build scripts to use `extensions/scripts/`
- [ ] **30.** Ship `pkg/azdext/azdexttest/` mock package
- [ ] **4.** Consolidate table/JSON/YAML formatting into `pkg/azdext/output.go`
- [ ] **11.** Shared AI model DTOs where genuine overlap exists

### Compatibility Phase (Parallel)
*Can proceed alongside other phases with deprecation period.*

- [ ] Standardize env var names with backward-compatible fallback + deprecation warnings
- [ ] Align flag names with backward-compatible aliases + deprecation warnings

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.