azd x: non-fatal metadata warnings and azd x init wizard polish#8197
Open
Copilot wants to merge 6 commits into
Open
azd x: non-fatal metadata warnings and azd x init wizard polish#8197Copilot wants to merge 6 commits into
azd x: non-fatal metadata warnings and azd x init wizard polish#8197Copilot wants to merge 6 commits into
Conversation
Copilot
AI
changed the title
[WIP] Fix azd x init to handle extension metadata warnings
Treat extension metadata warnings as non-fatal
May 15, 2026
JeffreyCA
force-pushed
the
copilot/fix-azd-x-init-metadata-warnings
branch
from
7072a55 to
bac949d
Compare
JeffreyCA
changed the base branch from
main
to
copilot/migrate-to-new-extension-root-command
JeffreyCA
force-pushed
the
copilot/fix-azd-x-init-metadata-warnings
branch
from
bac949d to
47e5638
Compare
JeffreyCA
changed the title
azd x: non-fatal metadata warnings and azd x init wizard polish
Base automatically changed from
copilot/migrate-to-new-extension-root-command
to
main
May 18, 2026 18:15
`azd x build` previously treated validation warnings (e.g. missing `providers` when `service-target-provider` is declared) as failures. That aborted `azd x init`, and the build subprocess output was discarded so the user never saw why. Warning handling: - `ux.TaskList`: a task returning `(Warning, err)` no longer cancels subsequent tasks, and warning detail renders with warning styling instead of error styling. - Extract `validateExtensionMetadata` as a shared helper. `azd x init` validates the in-memory schema directly instead of shelling out to `azd x build` and parsing its rendered output. - Show a short summary in the task list and the full bulleted detail once below it. - Drop the redundant "Local extension source already exists." message. - Rephrase warnings to name the YAML field, the capability that needs it, and what to set. Wizard UX (resolves #8207): - Confirmation prompt defaults to "yes". - Tags prompt is optional; also fixes a pre-existing bug where parsed tags were discarded. - Translate dotted namespaces (`ai.project`) into the actual command path (`azd ai project <command>`) in the generated `usage` field and follow-up hints, matching how `bindExtension` registers them. - Generated `extension.yaml` includes a `yaml-language-server` header pointing at the published schema URL. Scaffolded extension root command: - The Go template set `Use` to `"azd <namespace> <command> [options]"`, so cobra parsed `"azd"` as the root command name and rendered `Usage: azd [command]`. Use the leaf segment of the namespace for both `Name` and `Use`, matching the convention in `azure.ai.agents`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JeffreyCA
force-pushed
the
copilot/fix-azd-x-init-metadata-warnings
branch
from
47e5638 to
08859f8
Compare
Contributor
|
/azp run azure-dev - cli |
|
Azure Pipelines successfully started running 1 pipeline(s). |
JeffreyCA
requested review from
hemarina,
jongio,
rajeshkamal5050,
tg-msft,
vhvb1989,
wbreza and
weikanglim
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves azd x init and azd x build extension metadata validation behavior by making warnings non-fatal and polishing generated scaffold UX for extension authors.
Changes:
- Updates task list warning handling so warning-state errors render as warnings and do not stop subsequent tasks.
- Adds shared extension metadata validation with improved warning text and uses it from both build and init flows.
- Improves init wizard/scaffold output for tags, confirmation defaults, dotted namespaces, schema modelines, and generated Go root command usage.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
cli/azd/pkg/ux/task_list.go |
Changes task warning errors to be non-fatal and renders warning details with warning styling. |
cli/azd/pkg/ux/render_test.go |
Adds coverage for warning tasks continuing to subsequent tasks. |
cli/azd/extensions/microsoft.azd.extensions/internal/resources/languages/go/internal/cmd/root.go.tmpl |
Uses the leaf namespace for generated Go cobra root command metadata. |
cli/azd/extensions/microsoft.azd.extensions/internal/cmd/init.go |
Adds metadata validation, warning output, namespace formatting, optional tags, schema header, and build failure output handling. |
cli/azd/extensions/microsoft.azd.extensions/internal/cmd/init_test.go |
Adds tests for namespace formatting and metadata validation behavior. |
cli/azd/extensions/microsoft.azd.extensions/internal/cmd/build.go |
Extracts shared metadata validation and makes warnings non-fatal during build. |
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
wbreza
reviewed
May 18, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review — �zd x: non-fatal metadata warnings & init wizard polish
Automated review (Claude Opus 4.7) — 8 parallel focus-area passes. Multi-reviewer consensus findings called out. Recommendation: comment-only so this is discussion fuel, not a block — but the P1 items below are real and worth resolving before merge.
TL;DR
The core change — splitting Warning from Error in TaskList — is conceptually sound and the new �alidateExtensionMetadata extraction is clean. However the implementation has one real backward-compat regression in an unrelated consumer (cmd/tool.go), a rendering bug (multi-line warning string corrupts the live canvas), an ordering bug (filesystem mutation before validation), and several documentation/test gaps for behavior the PR is explicitly trying to change.
🔴 P1 — Should fix before merge
| # | File:line | Finding |
|---|---|---|
| 1 | pkg/ux/task_list.go:373–376 (shouldCollectTaskError) + cmd/tool.go:~1241,1296,1303–1310 | Silent backward-compat break in unrelated consumer. Pre-PR, any task returning (ux.Warning, err) had its error joined into Run()'s aggregated return. Existing �zd tool install/uninstall already returns (ux.Warning, fmt.Errorf("%s did not succeed", …)) when a tool fails. Post-PR that error is silently dropped → the "Some tools could not be installed…" notice never fires and �zd tool install/uninstall exits 0 even when tools fail. The Warning semantics flip wasn't audited against existing call sites. Fix: either (a) audit and migrate cmd/tool.go to return Error for real failures (preferred — its current "Warning + err" was always semantically muddled), or (b) keep collecting Warning errors but branch on Warning only at the "skip remaining" gate. |
| 2 | �xtensions/microsoft.azd.extensions/internal/cmd/build.go:135 (�alidationWarningsMessage) → rendered via pkg/ux/task_list.go:255–262 | Multi-line warning string corrupts the live canvas. �alidationWarningsMessage returns "validation warnings:\n - foo\n - bar" and is set as the task's error. The TaskList renders it inline as (%s) on a single status row; embedded newlines + bullets break ANSI cursor positioning during the per-second canvas re-render. Pre-PR this string never reached this code path because Warning was treated as Error. Fix: keep the inline summary single-line (e.g. "N validation warning(s)") and emit detailed bullets after askList.Run() returns — exactly the pattern init.go's new writeCollectedWarnings already uses. Have �uild.go reuse it. |
| 3 | �xtensions/microsoft.azd.extensions/internal/cmd/init.go (task order in | |
| unInitAction) | Validation runs after the filesystem mutation. �alidateExtensionAction currently runs after createExtensionDirectoryAction. Schema validation is pure (no I/O); if it returns Error, the partially-created directory is left on disk and confuses subsequent re-runs ("directory exists / continue to create/update files"). Move validation before directory creation. | |
| 4 | �xtensions/microsoft.azd.extensions/internal/cmd/init.go:~373–380 (early-return on askList.Run() != nil) | Collected warnings dropped exactly when users need them. If a later task (build/package/publish) fails, the function returns early and writeCollectedWarnings(buildWarnings) is never invoked — even though the inline status said "see details below". Fix: defer writeCollectedWarnings(buildWarnings) once warnings are populated, or call it unconditionally before the early return. |
| 5 | �xtensions/microsoft.azd.extensions/CHANGELOG.md (top) | No changelog entry for a user-visible extension behavior change. Latest release (0.10.0, 2026-03-04) doesn't include this and there's no Unreleased section. Extension authors need to know about it — �zd x build exit semantics change, and the generated |
| oot.go's Name: field changes from full namespace to leaf (see #9 below). | ||
| 6 | pkg/ux/render_test.go:561–589 (TestTaskList_Run_warningContinues) + :173–195 (TestTaskList_Render_warning) | The actual visual bug fix is untested. The literal change at ask_list.go:261 is WithErrorFormat → WithWarningFormat (red → yellow). Neither test asserts the ANSI format used — they only �ssert.Contains(output, "partial failure"), which would have passed pre-PR too. Add an assertion that the warning row uses output.WithWarningFormat(...) and explicitly does not contain WithErrorFormat(...). Also assert l.errors is empty after the warning-only run, to lock shouldCollectTaskError's contract. |
🟡 P2 — Should fix before merge, lower urgency
| # | File:line | Finding |
|---|---|---|
| 7 | pkg/ux/task_list.go:85–95 (TaskState constants) | Warning, Error, Success, etc. have no GoDoc. The Warning semantics just changed silently; document that (Warning, err) is display-only and execution continues. AGENTS.md requires GoDoc on exported types. |
| 8 | �xtensions/.../build.go (overall) | �zd x build with warnings now silently exits 0, undocumented, no --strict/--warnings-as-errors flag. CI scripts get no signal. Either document explicitly in the command's Long help, or add a flag. |
| 9 | �xtensions/.../resources/languages/go/internal/cmd/root.go.tmpl:13–14 | Template Use: and Name: changed from full {{.Metadata.Namespace}} to {{.LeafNamespace}}. Breaking change for any generated extension that referenced its own name (telemetry/log scopes). At minimum, expose both LeafNamespace and FullNamespace and call out the change in the changelog. |
| 10 | �xtensions/.../init.go collectExtensionMetadataFromFlags (--no-prompt path) | No validation of namespace shape. Values like "a..b" or empty produce |
| amespaceCommandPath == "a b" and LeafNamespace == "", generating Name: "" in | ||
| oot.go.tmpl which cobra rejects at runtime. Validate ^[a-z][a-z0-9](.[a-z][a-z0-9])*$ in both interactive and scripted paths. | ||
| 11 | �xtensions/.../init.go (deleted "Local extension source already exists" notice) | The post-run output.WithWarningFormat(...) notice that fired when localRegistryExists was removed. The Skipped row still appears but the explicit follow-up is gone — UX regression. Restore or route through writeCollectedWarnings. |
| 12 | init.go writeCollectedWarnings vs pkg/ux/task_list.go:43 vs �uild.go validationWarningsMessage | Inconsistent warning text formatting across three sites: "(!) Warning " (TaskList), "WARNING:" (writeCollectedWarnings, ALL CAPS), "validation warnings:" (build.go, lowercase + colon), "%d warning(s) — see details below" (init.go, em-dash). Pick one. |
| 13 | �uild.go:56–72 ↔ init.go:196–214 | Duplicated validation-result handling. Both call �alidateExtensionMetadata, format errors with slightly different prefixes, wrap with common.NewDetailedError("Validation failed", …), and format warnings differently. Extract a shared helper. |
| 14 | init.go buildExtensionAction (~line 226) | cmd.CombinedOutput() captured to �uildCommandOutput only when �rr != nil. If �zd x build succeeds but emits warnings to stdout/stderr (now possible because warnings are non-fatal), the user never sees them. Always retain subprocess output. |
| 15 | init.go buildExtensionAction (~lines 249–292) | PATH-dependent guarantee. Shells out to whichever �zd is on PATH. The "warnings are non-fatal" promise only holds when the spawned binary contains this PR. With an older �zd installed, the subprocess still aborts on warnings. Document the runtime dependency. |
| 16 | init.go (tags prompt) | Tags now optional but no length/control-char sanitization. Malformed tags break the YAML and the new schema-header line. Add a cap and reject control chars. |
| 17 | �uild.go:~73 �alidateExtensionMetadata GoDoc | Comment says warnings "flag recommended but optional fields", but missing |
| amespace (with custom-commands capability) is now reported as an error, not a warning. Update doc. | ||
| 18 | init_test.go:464–526 (TestValidateExtensionMetadata) | Pins exact long multi-sentence error/warning strings. These are tunable user-facing copy and will break on every wording tweak without catching real regressions. Use �ssert.Len + �ssert.Contains on stable key prefixes. |
| 19 | pkg/ux/task_list.go:373–376 (shouldCollectTaskError) | New helper has no direct unit test; only covered transitively. Add a table test covering each TaskState × |
| il/non-nil �rr. | ||
| 20 | �xtensions/.../init.go writeCollectedWarnings, writeFailedCommandOutput | Both write to mt.Println/ mt.Print directly, making them untestable. Refactor to accept an io.Writer. |
🟢 P3 — Nice to have
| # | File:line | Finding |
|---|---|---|
| 21 | pkg/ux/task_list.go:307, 166 | Pre-existing data race: |
| unSyncTasks reads len(t.errors) without �rrorMutex; Run() reads .errors after Wait() without the lock. Safe today via happens-before from waitGroup.Wait(), but the new indirection makes it easier to introduce a real race later. Tighten now or document the synchronization contract. | ||
| 22 | pkg/ux/task_list.go:230–244 (Render) | Render reads ask.State/ ask.Error/ ask.endTime from the render goroutine while runners mutate them unsynchronized. Warning rendering now exposes ask.Error via statusDescription, giving readers a new path to observe a torn interface value. Per-task mutex or snapshot under �rrorMutex. |
| 23 | ||
| oot.go.tmpl namespace template injection | LeafNamespace/Description are user-supplied values dropped into Go string literals via ext/template with no escaping. Low impact (self-scaffold), but a namespace containing ", , or newline produces broken/attacker-controlled Go. Validate input format. | |
| 24 | init.go writeFailedCommandOutput | Subprocess CombinedOutput() may contain ANSI sequences; printing verbatim ignores parent's NO_COLOR. Strip ANSI or set NO_COLOR=1 in the child env. |
| 25 | init.go (confirm prompt default flip from "no" → "yes") | Pressing Enter now confirms creation. Reasonable for happy path but combined with "directory exists → continue to create/update files" can silently overwrite user files. Consider warning if target dir is non-empty. |
| 26 | init.go:~209–213 | Manual pluralization + em-dash literal in code is unusual for this codebase (rest uses ASCII hyphens). Minor. |
| 27 | init_test.go imports | Single import group mixes external ( estify) and internal (github.com/azure/azure-dev/...). Split per AGENTS.md import order. |
| 28 | �uild.go:~58 | Error string starts with capital "Extension contains validation failures..."; parallel init.go:~201 uses lowercase. Pick one. |
| 29 | ExtensionTemplate struct (init.go) | Lacks GoDoc on the type itself (only the new LeafNamespace field has it). |
| 30 | New tests don't call .Parallel(); sub-test with empty namespace produces blank .Run name. |
Multi-reviewer consensus: Findings #1, #2, #3, #4, #8 were each flagged independently by 2+ focus-area reviewers, giving them high confidence.
Not flagged anywhere: No command-injection issues, no permissive file mode regressions, no performance concerns, copyright headers all intact, no interface{} regressions, no Go 1.26 modernization misses.
wbreza
reviewed
May 18, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Re-review @ bd94cbb
Thanks for the iteration. The "Address comments" commit is a well-reasoned, narrowly-scoped fix:
✅ What the delta does well
- Promotes
namespace-missing-with-custom-commandsfrom warning → fatal error- Inline comment clearly explains why it's fatal (
bindExtensionuses the last.-segment as the cobra command name → empty namespace silently installs an unreachable command)- Inline comment explains why
providers-missing stays a warning (init wizard doesn't yet prompt for providers — promoting would block every service-target-provider scaffold)- Test case renamed and expectation moved from
wantWarnings→wantErrorsconsistently
🆕 New findings introduced by this delta
| Priority | File:line | Finding |
|---|---|---|
| P2 | build.go validateExtensionMetadata GoDoc (function-level comment) |
The promotion of namespace-missing from warning → error makes the existing GoDoc more inaccurate. The function now has two reasons to return errors (structural Missing required field: X and capability-conditional fatal misuse), but the doc still frames errors as the structural-only category. The new inline comments compensate at the call site but the public-facing doc is now more misleading than before. |
| P2 | build.go validateExtensionMetadata (new error path itself) |
The warning→error promotion is a breaking validation change for hand-edited extension.yaml files that previously built successfully. The inline comment acknowledges this ("only triggers on hand-edited files") — a reasonable risk assessment, but exactly the kind of change that needs a CHANGELOG entry (see open #5 below). |
| P3 | init_test.go:102–124 |
Renamed test still pins the exact long error string verbatim — same brittleness flagged previously. Worth refactoring to assert.Contains on a stable key prefix alongside this rename, rather than as a separate cleanup. |
📋 Open from prior review (not addressed in this commit)
Still outstanding from the previous review — flagging here so they don't get lost:
P1 (worth fixing before merge):
- #1
cmd/tool.gobackward-compat regression —azd tool install/uninstallsilently exits 0 on failure because(ux.Warning, err)returns are now dropped byshouldCollectTaskError. (Highest-impact finding — affects an unrelated command.) - #2 Multi-line
validationWarningsMessagecorrupts the live TaskList canvas when rendered inline as(%s)with embedded\n -bullets. - #3
validateExtensionActionruns aftercreateExtensionDirectoryAction— schema validation should precede the filesystem mutation. - #4
writeCollectedWarningsis skipped on the early-return path when a later task (build/package/publish) fails — warnings dropped exactly when users need them.defer writeCollectedWarnings(buildWarnings)fixes it. - #5 No
CHANGELOG.mdentry incli/azd/extensions/microsoft.azd.extensions/for a user-visible behavior change (now compounded by the warning→error promotion in this commit). - #6 The literal visual fix (
WithErrorFormat→WithWarningFormat, red → yellow) is untested — neitherTestTaskList_Render_warningnorTestTaskList_Run_warningContinuesasserts the format used.
P2 (still relevant):
- #7
TaskState.Warningconstant lacks GoDoc explaining the new(Warning, err)display-only semantics. - #8
azd x buildexiting 0 on validation warnings is undocumented; consider--strict/--warnings-as-errorsfor CI. - #9
root.go.tmplName:field changed from full to leaf namespace — breaking for downstream extensions that referenced their own name. - #10
--no-promptpath still doesn't validate namespace shape (e.g."a..b", leading/trailing dot) — only thecustom-commands+ empty case is now caught. - #11 Deleted "Local extension source already exists" post-run notice is a UX regression.
- #17
validateExtensionMetadataGoDoc — see new P2 above; the prior staleness is now worse.
Full prior review with all P3 items: #8197 (review)
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address self-review nits on top of the prior commit: - Replace the duplicated success/failure flushes in runInitAction with a single named-return defer so warnings are always printed and subprocess output is surfaced only on failure (avoids dumping the full build transcript on success). - Replace the duplicated flush in runBuildAction with a defer for the same reason; restores the simple 'return taskList.Run()' shape. - Reorder runInitAction tasks so metadata validation runs before any filesystem side-effects, including the local extension source creation. - Drop the redundant '(!) Warning' prefix from the post-run warnings header; the TaskList row already shows that marker. - Add a boundary test asserting exactly maxExtensionTags parses successfully. - Wrap the long assert.True lines in TestValidateExtensionMetadata to satisfy the 125-char lll lint cap. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…publish/install step fails
Contributor
Thanks for the thorough review - addressed the following: Review feedback
Other changes
|
hemarina
reviewed
May 19, 2026
Contributor
hemarina
left a comment
hemarina
left a comment
There was a problem hiding this comment.
Re-review @ 9cc6d11
Follow-up after the prior review on bd94cbb. The 5 new commits (25e770df → 9cc6d11) resolve every P1 item and almost all P2/P3 items from that review — net result is a substantially cleaner change. Calling out two small net-new findings I caught in the runSubprocess/subprocessErrorTail helpers and a couple of items that remain open from the prior review. Not posting blockers; the PR is in good shape.
✅ Resolved from prior review
- P1 #1
cmd/tool.go(Warning, err)regression — now returnsux.Error, covered byTestRunToolOperationUnsuccessfulResultReturnsError - P1 #2 Multi-line warning corrupting the canvas — inline message is now a single-line summary; details flushed after via
writeCollectedWarnings - P1 #3 Validation running after the filesystem mutation —
validateExtensionActionis now the first task - P1 #4 Collected warnings dropped on early-return —
defer writeCollectedWarnings(os.Stdout, buildWarnings)in bothbuild.goandinit.go - P1 #5 Missing CHANGELOG entry — new
0.11.0section - P1 #6 Color-format fix untested —
assert.Contains(output, outputpkg.WithWarningFormat("(partial failure)"))+NotContains(WithErrorFormat(...)) - P2 #7
TaskStateconstants — GoDoc added - P2 #8
azd x buildsilent exit 0 — documented in the newLong:help text (no--strictflag, but the behavior is now discoverable) - P2 #10
--no-promptnamespace shape —validateExtensionNamespaceenforces^[a-z][a-z0-9]*(\.[a-z][a-z0-9]*)*$in both interactive and scripted paths - P2 #16 Tag sanitization —
parseTagsenforcesmaxExtensionTags=10,maxExtensionTagLength=64, rejects control characters - P2 #17
validateExtensionMetadataGoDoc updated to reflect the warning→error promotion - P2 #18 Tests no longer pin exact long strings —
assert.Len+slicesContainSubstringon stable prefixes - P2 #19
shouldCollectTaskError— direct table test added - P2 #20
writeCollectedWarningsnow takesio.Writer;writeFailedCommandOutputreplaced bysubprocessErrorTail - P3 #23 Template injection —
goString = strconv.Quotetemplate helper +TestTemplateGoStringQuotesDescription - P3 #24 ANSI in subprocess output — stripped via
ansiEscapeRegex - P3 #25 Overwrite warning —
isDirNonEmpty+ amended confirm prompt withHelpMessage - P3 #28/29 Capitalization +
ExtensionTemplateGoDoc
🆕 New nits (P3)
| # | File:line | Finding |
|---|---|---|
| 1 | init.go:312-317 (runSubprocess) |
Title duplicated in the wrapped error. common.NewDetailedError(description, fmt.Errorf("%s: %w%s", description, err, ...)) puts description in both the title and the inner message prefix. The live TaskList row uses Description() only so the canvas is fine, but when the error propagates up through taskList.Run() and the CLI error middleware prints it, the final output renders as Build failed\n\nDetails:\nBuild failed: exit status 1: <tail> — "Build failed" appears twice. Drop description from the inner Errorf. |
| 2 | init.go:905-908 (subprocessErrorTail) |
Bare ERROR:/Error: line yields a trailing ": " artifact. Input "ERROR:\n" produces ": " because TrimSpace(TrimPrefix(...)) returns "" but the function still prepends ": ". Wrapped error becomes "Build failed: exit status 1: ". azd subcommands today always emit "ERROR: <message>" so this is defensive, but the fix is one line — if the post-trim string is empty, fall through to fallback instead of returning. Add a test row {name: "bare ERROR", in: "ERROR:\n", want: ""} to TestSubprocessErrorTail. |
📋 Still open from prior review
- P2 #11 Removed "Local extension source already exists" notice — users now see
(-) SkippedagainstCreate local azd extension sourcewith no follow-up explanation. The deferred warnings flush only handles validation warnings, not the registry-skipped case. Minor UX regression; either restore the post-run notice or move it into aHelpMessage-style hint on the task title. - P2 #14 When the spawned subprocess succeeds but emits warnings to stdout, those are still discarded — only the parent''s in-memory validation feeds
writeCollectedWarnings. Documented as PATH-dependent in the newLong:help, so this is acceptable, but worth a one-line note in the help text that nested-build warnings from a different azd version may not surface. - P3 #12 Warning wording still drifts slightly across sites:
(!) Warning(TaskList),Validation warnings:(deferred flush),N validation warning(s); see details below(inline summary). Lower priority than before but not fully harmonized. - P3 #22 Race on
Task.State/Task.Errorreads from the render goroutine — unaddressed (pre-existing).
Other observations (P3 / informational)
runSubprocess''s/* #nosec G204 */waiver covers all four call sites, but three of them pass only hardcoded literals ("x build --skip-install", etc.); only the install action passes user-controlledextensionMetadata.Id. Moving the comment to the install closure would be more precise.runInitActionis now(err error)named return, but inner scopes reuseerrvia:=(e.g. line 220nonEmpty, err := isDirNonEmpty(...)). Every early return is explicit so it works today, but named-return + heavy:=reuse is a known foot-gun. Either name it something distinctive (result error) or skip the named return and passbuildWarningsto a small helper that does the flush.promptExtensionNamespaceloops indefinitely untilvalidateExtensionNamespacepasses. Relies onPrompt()propagating Ctrl+C as an error, which it does today. Worth a one-line comment documenting that assumption.ansiEscapeRegexonly matches CSI sequences; OSC hyperlinks (\x1b]8;;…\x07) would leak through. Low impact —fatih/coloronly emits CSI today.CHANGELOG.mdis dated2026-05-19; verify against the actual release date.
Verdict
Looks good to me. The two runSubprocess nits are the only items I''d actually want to see fixed before merge, and even those are P3-level. Recommend comment-only; happy to re-review once the nits land if you want.
| "Package failed", | ||
| fmt.Errorf("failed to package extension: %w", err), | ||
| description, | ||
| fmt.Errorf("%s: %w%s", description, err, subprocessErrorTail(result)), |
Contributor
There was a problem hiding this comment.
Minor: description is passed as both the NewDetailedError title and the prefix of the inner Errorf. When this error propagates up through taskList.Run() and prints via the CLI error middleware, the final output is:
Build failed
Details:
Build failed: exit status 1: <tail>
"Build failed" ends up doubled. The live canvas is fine (uses Description() only), but the printed error duplicates the title. Suggested fix:
return ux.Error, common.NewDetailedError(
description,
fmt.Errorf("%w%s", err, subprocessErrorTail(result)),
)While here: subprocessErrorTail returns ": " for a bare "ERROR:\n" line because TrimSpace(TrimPrefix(...)) collapses to "" but ": " is still prepended. Unlikely in practice (azd always emits text after ERROR:) but a one-line guard + test row would close it.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #8207
Fixes #8196
This PR makes
azd x buildextension metadata warnings non-fatal so they no longer abortazd x init, surfaces the real cause when an init subtask does fail, and rolls in severalazd x initwizard fixes including Issue #8207.Warning handling
azd x buildpreviously treated validation warnings (e.g. missingproviderswhen theservice-target-providercapability is declared) as command failures.azd x initthen aborted on those warnings, and its build subprocess output was discarded so the user never saw why.ux.TaskList: a task returning(Warning, err)no longer cancels subsequent tasks, and warning details render with warning styling instead of error styling.TaskStateconstants now have GoDoc and a dedicated unit test pins the "warning does not collect as error" contract.azd x buildreports validation warnings via theux.Warningstate and reserves theux.Errorstate for missing required fields.validateExtensionMetadatais extracted as a shared helper soazd x initvalidates the in-memory schema directly. The old subprocess-output-parsing approach (ANSI stripping, marker probing, trailing-paren trimming) is gone.validationFailureError/validationWarningSummaryare shared betweeninitandbuildso the wording stays in sync.(-) Skippedtask line already conveys that).Init subprocess error surfacing
The four child steps in
azd x init(build, package, publish, install) previously bubbled up asexit status 1with no underlying message, and their TaskList canvases replayed inside the parent's UI. A sharedrunSubprocesshelper now capturesCombinedOutput, strips ANSI sequences, and inlines the firstERROR:/Error:line (fallback: last non-empty line) into the wrapped error. The end result for a namespace-conflict install failure is a singleDetails:block with the actual reason — no duplicated rendering, no opaque exit code.Wizard UX (resolves #8207)
tags:was always empty. Tag parsing now enforces count, per-tag length, and control-character limits.^[a-z][a-z0-9]*(\.[a-z][a-z0-9]*)*$) applied to both the interactive and--no-promptpaths.ai.project) are translated into the actual command path (azd ai project <command>) when generating theusagefield and the "Try out the extension" follow-up hint, matching howbindExtensionregisters nested namespaces. The namespace prompt help text now explains this behavior.extension.yamlincludes a# yaml-language-server: $schema=...modeline pointing at the published schema URL so editor tooling validates the file out of the box.Scaffolded extension root command
The Go template produced
Use: "azd {{namespace}} <command> [options]", which made cobra parse"azd"as the command name and renderedUsage: azd [command]in the extension's own--helpoutput. The template now uses the leaf segment of the namespace for bothNameandUse(matching the convention inazure.ai.agents), soazd ai test --helpdisplaysUsage: test <command> [options]as expected. The user-suppliedDescriptionis also routed through agoStringtemplate helper (strconv.Quote) before being embedded as the cobraShort:literal, so descriptions containing quotes, backslashes, or newlines produce valid Go source rather than a broken scaffold.