Conversation
…se errors Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/de0c1443-075c-458a-a43c-18829f80684a Co-authored-by: JeffreyCA <9157833+JeffreyCA@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix conflicts and adopt SDK root command for azure.ai.agents
fix(host): support attached short --output/-o values and surface pre-parse errors
Apr 28, 2026
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 fixes azd host pre-parse behavior so --output/-o works with attached short-form values (e.g. -otable, -ojson) and ensures pre-Cobra global-flag parse errors are actually surfaced to users (instead of a silent non-zero exit).
Changes:
- Print
ParseGlobalFlagsfailures to stderr inExecuteWithAutoInstallso pre-Cobra errors are visible. - Register
--output/-oon the pre-parse flag set (not the global persistent set) to prevent pflag’s unknown-flag walk from mis-parsing-otable. - Add a regression test covering the common
-o/--outputattachment forms.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
cli/azd/cmd/auto_install.go |
Adds stderr printing for pre-parse errors and registers --output/-o on the pre-parse FlagSet to accept attached short values. |
cli/azd/cmd/auto_install_test.go |
Adds a regression test to ensure all common -o/--output syntaxes parse without error. |
Contributor
|
@copilot please fix the lint errors and assess PR comments |
wbreza
reviewed
Apr 28, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review Summary
| Priority | Count |
|---|---|
| Medium | 1 |
| Low | 2 |
| Total | 3 |
Overall Assessment: Comment — no blockers, one actionable suggestion.
✅ What Looks Good
- Root cause fix is correct — registering
-oon the local pre-parse FlagSet avoids shadowing per-command output configuration - Error surfacing is well-placed — prints to stderr before cobra is initialized (no console available yet)
- Tests are well-structured — table-driven, parallel, covering all 6
-o/--outputattachment forms "ERROR:"prefix is consistent with existing patterns inerror.goandux.go
Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/141629a3-33b1-4655-8f85-9f39cbd87e0d Co-authored-by: JeffreyCA <9157833+JeffreyCA@users.noreply.github.com>
Contributor
Author
Addressed both in 4cb9bdf:
|
Contributor
|
@copilot please address PR comments above |
Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/1a56800d-7d74-4770-8876-a3d3c389b6df Co-authored-by: JeffreyCA <9157833+JeffreyCA@users.noreply.github.com>
Contributor
|
/azp run azure-dev - cli |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jongio
reviewed
Apr 29, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
The approach is right - registering -o on the pre-parse set prevents pflag's char-by-char walk from misinterpreting -otable by hitting the known -e flag. Clean fix.
One gap in test coverage noted inline.
wbreza
approved these changes
Apr 29, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review — Updated PR
Previous findings (double-wrapped error, missing test comment, cspell) have all been addressed ✅
✅ What Looks Good
- Root cause fix is precisely scoped — -o registered on pre-parse FlagSet only, avoiding shadowing per-command output.AddOutputParam
- Error surfacing is the only viable approach — console isn't initialized yet; os.Stderr matches existing patterns in main.go
- Tests are thorough — table-driven, parallel, all 6 -o/--output forms covered, with explanatory comment
- Clean commit history — feedback addressed in well-titled follow-up commits
Findings
| Priority | Count |
|---|---|
| Low | 1 |
[Low] Bare -o (no value) now fails at pre-parse instead of deferring to Cobra — marginal UX difference for a user-error case. Not a blocker.
Note: jongio's suggestion to add a combined-flags test case (e.g., "-otable", "-e", "prod") would strengthen regression coverage.
Verdict: Ship it. Solid, well-scoped fix. No blockers.
vhvb1989
approved these changes
Apr 29, 2026
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
|
Contributor
|
/check-enforcer override |
wbreza
approved these changes
Apr 30, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review — Fresh Pass
No new blocking findings. Two low-priority polish items noted below.
✅ What Looks Good
- Excellent root-cause comment explaining why -o is registered on the pre-parse FlagSet only
- Clean table-driven tests with .Parallel(), all 6 -o/--output forms covered
- Error surfacing fix is correct — users no longer get blank non-zero exits
- Prior review feedback cleanly addressed in follow-up commits
Low-Priority Notes (non-blocking)
-
"ERROR: %s" format — Verify output.WithErrorFormat doesn't already apply an "Error:" label, which would double-label the output. Minor consistency check.
-
Stderr path untested — No test triggers a ParseGlobalFlags failure and asserts stderr output. Since the original bug was "silent failures," a regression test capturing stderr would close the loop. Fine as a follow-up.
Verdict: Ship 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 #7949
Two bugs in
ParseGlobalFlags/ExecuteWithAutoInstallthat surface when invoking commands with-otableor-ojson.Root causes
Silent failures — when
ParseGlobalFlagsreturns an error,main.goonly inspectsresult.Errto set the exit code; it never prints. Users got a blank non-zero exit.-otable/-ojsonrejected — pflag'sUnknownFlagsallowlist parses combined short flags char-by-char. Because-e/--environmentwas registered but-o/--outputwas not, the character sequenceo→t→a→b→l→ehit the known string flag-e, which then demanded a value:Changes
cmd/auto_install.goExecuteWithAutoInstall: print to stderr viaoutput.WithErrorFormatbefore returningresult.Err, so pre-cobra failures are visible.ParseGlobalFlags: register--output/-oon the local pre-parse FlagSet only — not inCreateGlobalFlagSet, which is also wired into the root command's persistent flags. Adding it there would causeGetCommandFormatterto reject the empty default withunsupported formaton every extension command.cmd/auto_install_test.goTestParseGlobalFlags_OutputAttachedShortForm: regression test covering all six forms —-otable,-ojson,-o table,-o=table,--output table,--output=table.Created from Copilot CLI via the copilot delegate command.