feat(azure.ai.projects): migrate project endpoint commands from azure.ai.agents#8243
feat(azure.ai.projects): migrate project endpoint commands from azure.ai.agents#8243huimiu wants to merge 1 commit into
Conversation
….ai.agents Migrate the project endpoint set/unset/show logic introduced in #8162 (originally added to `azure.ai.agents` as `azd ai agent project ...`) into the new `azure.ai.projects` extension. Subcommands hang directly off the extension root (which is already `project`), so users get: - `azd ai project set <endpoint>` — persist a default Foundry project endpoint - `azd ai project unset` — clear the persisted endpoint - `azd ai project show` — show the resolved endpoint and source Key adjustments vs source PR: - Module path `azure.ai.projects` - Config namespace `extensions.ai-projects.context` (independent of the agents extension's store) - Suggestion strings reference `azd ai project set` - 5-level resolver split into its own `project_resolver.go` The resolver still implements the spec'd cascade: flag → active azd env (`AZURE_AI_PROJECT_ENDPOINT`) → global config → host `FOUNDRY_PROJECT_ENDPOINT` → structured `missing_project_endpoint` error. Invalid values at any level are hard validation errors (no silent fallback). A minimal `internal/exterrors` package is introduced with only the `Validation` / `Dependency` factories and codes required by the migrated commands.
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
There was a problem hiding this comment.
Pull request overview
This PR migrates Foundry project endpoint persistence and resolution into the azure.ai.projects extension, adding top-level set, unset, and show commands plus validation, config storage, and tests.
Changes:
- Adds endpoint validation and 5-level resolution cascade.
- Adds global config persistence helpers and project endpoint commands.
- Adds command/unit tests and updates module dependencies.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
internal/exterrors/errors.go |
Adds structured local error helpers. |
internal/exterrors/codes.go |
Defines endpoint-related error codes. |
internal/cmd/root.go |
Registers set, unset, and show commands. |
internal/cmd/project_set.go |
Implements endpoint persistence command. |
internal/cmd/project_unset.go |
Implements endpoint clearing command. |
internal/cmd/project_show.go |
Implements endpoint display command. |
internal/cmd/project_endpoint.go |
Adds endpoint validation and source types. |
internal/cmd/project_resolver.go |
Adds endpoint resolution cascade. |
internal/cmd/project_context_store.go |
Adds global config read/write/clear helpers. |
internal/cmd/extension_context.go |
Adds nil-safe extension context helper. |
internal/cmd/*_test.go |
Adds tests for commands, resolver, validation, and flag metadata. |
go.mod |
Updates direct/indirect dependencies. |
go.sum |
Removes unused module checksums. |
| // Read existing state first so we can return the previous endpoint. | ||
| state, found, err := getProjectContext(ctx, azdClient) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| if found { |
wbreza
left a comment
There was a problem hiding this comment.
Thanks @huimiu! Nice clean structure on the cascade resolver and the test coverage on validation is strong. A few things worth resolving before this lands — most are architectural questions about how this fits alongside azure.ai.agents, plus a couple of correctness items.
High
1. Is this actually a migration?
The PR title says "migrate project endpoint commands from azure.ai.agents", but the old files in cli/azd/extensions/azure.ai.agents/internal/cmd/project_{set,show,unset,endpoint,context_store}.go are unchanged. After this lands, both extensions expose the same project set/show/unset commands.
Could you clarify the intent?
- If this is a true migration: consider removing the commands from
azure.ai.agents(and the duplicatedinternal/exterrorspackage) in this PR, along with a CHANGELOG note pointing users to the new location. - If
azure.ai.agentsis staying around for a while: could the PR title and description be updated to reflect "add project endpoint commands toazure.ai.projects", with a short note in both extensions' READMEs explaining which to prefer? Otherwise reviewers and users won't know which one is canonical.
2. Persisted config key change risks silent data loss
The old extension persists to extensions.ai-agents.project.context and the new one persists to extensions.ai-projects.context (note: the new path drops .project, so it's not just a namespace rename). Users with an existing persisted endpoint in the old key will see "no endpoint set" in the new extension with no signal that anything was lost.
Two options worth considering:
- One-time legacy read: when the new resolver hits level 3 (global config) and finds nothing, fall back to reading the old
extensions.ai-agents.project.contextkey and, if present, surface a note like "found endpoint persisted byazure.ai.agents; runazd ai project set <endpoint>to migrate." - Or explicitly call out the re-
setrequirement in the PR description / release notes so this is a known break rather than a silent one.
3. internal/exterrors is duplicated rather than shared
The new internal/exterrors is a thin subset of the identical package in azure.ai.agents/internal/exterrors. Both will need parallel maintenance for any future factory or code. If a shared location exists (or one can be added under cli/azd/extensions/shared/), importing from there would avoid the divergence. If sharing is intentionally out of scope for this PR, a short comment in the new package noting "subset of azure.ai.agents/internal/exterrors; consolidation tracked in #..." would help future maintainers.
4. Suggestion string is built by concatenation in project_show.go
return exterrors.Dependency(
exterrors.CodeMissingProjectEndpoint,
localErr.Message,
"run `azd ai project set <endpoint>` to persist a default, or " + localErr.Suggestion,
)If localErr.Suggestion is empty the user sees a trailing "…persist a default, or ". If non-empty, two suggestion strings get glued together with potentially mismatched grammar. Prefer a single canonical suggestion authored at this call site, and either ignore localErr.Suggestion or branch on it explicitly.
Medium
5. fmt.Errorf("...: %w", err) wraps NewAzdClient() errors
In project_set.go and project_unset.go:
azdClient, err := azdext.NewAzdClient()
if err != nil {
return fmt.Errorf("failed to create azd client: %w", err)
}Per the extensions error-handling contract (see azure.ai.agents/AGENTS.md), wrapping with fmt.Errorf defeats classification by host middleware (status.FromError, errors.AsType[*LocalError]) — the wrapper string gets surfaced instead of the structured error's category/code. Returning the error unchanged, or classifying it to a structured exterrors.* here, is the documented pattern. Same applies in any sibling that wraps gRPC/structured errors.
6. Test seam mutates a package-level function pointer
// project_resolver.go
var readAzdHostedSourcesFunc = readAzdHostedSources
// project_resolver_test.go — stubAzdHostedSources mutates the globalNo race today because the resolver tests don't use t.Parallel() — but other tests in the same package do, and given the table-driven structure here it's very tempting to add. The moment a parallel test calls stubAzdHostedSources you'll get a data race under -race. Safer: inject readAzdHostedSources via the existing resolveProjectEndpointOpts struct so each test owns its own seam.
7. unset idempotent path reads as a failure
When no endpoint is set, unset prints "No active project endpoint to clear." and returns "cleared": false in JSON. The command succeeded (it was idempotent), but both the message and the boolean read like rejections. Renaming the field (hadPreviousEndpoint or wasSet) and/or rephrasing the message to confirm the no-op state would make automation and users happier.
Notes (non-blocking)
- The 5-level cascade is well-factored and the design spec correctly counts the hard-error as level 5 — no change needed there.
- Security pass came back clean: HTTPS-only enforced, host allowlist (
.services.ai.azure.com), explicit ports rejected, and crucially all four input sources are re-validated before use, so an invalid intermediate level errors hard instead of silently falling through. Nice. - Related to #2 above and the existing Copilot inline on
project_context_store.go: while touchingclear/getProjectContext, consider making theunsetpath best-effort when reading the previous value, so a malformed persisted blob can always be cleared (the bot's suggestion).
Summary
Migrate the project endpoint set/unset/show logic introduced in #8162 (originally added to
azure.ai.agentsasazd ai agent project ...) into the newazure.ai.projectsextension. Subcommands hang directly off the extension root (which is alreadyproject), so users get:azd ai project set <endpoint>— persist a default Foundry project endpointazd ai project unset— clear the persisted endpointazd ai project show— show the resolved endpoint and sourceThe resolver implements the spec'd 5-level cascade:
--project-endpointflag (level 1 — exposed by callers; not surfaced on these three subcommands today)AZURE_AI_PROJECT_ENDPOINTextensions.ai-projects.context.endpointin~/.azd/config.jsonFOUNDRY_PROJECT_ENDPOINTmissing_project_endpointerror with actionable suggestionInvalid values at any level produce a hard validation error (no silent fallback to lower levels).
Key adjustments vs source PR #8162
azure.ai.agents(source)azure.ai.projects(this PR)azureaiagentazure.ai.projectsextensions.ai-agents.project.contextextensions.ai-projects.contextazd ai agent project set ...azd ai project set ...projectgroupagent_context.goproject_resolver.goA minimal
internal/exterrorspackage is introduced with only theValidation/Dependencyfactories and the two error codes (invalid_parameter,missing_project_endpoint) required by the migrated commands.Files
internal/exterrors/{codes.go,errors.go}internal/cmd/{extension_context,project_endpoint,project_context_store,project_resolver,project_set,project_unset,project_show}.gointernal/cmd/{flag_options,project_endpoint,project_resolver,project_set,project_unset,project_show}_test.gointernal/cmd/root.go— registered the new subcommandsgo.mod—go mod tidypromotedstretchr/testifyandgoogle.golang.org/grpcto direct deps; previously-scaffoldedazidentity/armresourcesdemoted to indirect (not used)Test
From
cli/azd/extensions/azure.ai.projects:Test coverage mirrors PR #8162:
--outputdefault + allowed values, root has the three subcommands)Related
context,version, andmetadatacommands in this extension are untouched