fix: retain deploy hook extension warnings - #9950
Open
Hui Miao (huimiu) wants to merge 5 commits into
Open
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Hui Miao (huimiu)
marked this pull request as ready for review
September 10, 2026 08:20
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The concurrency regression test does not execute any capture operations concurrently.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Retains deploy lifecycle extension warnings after preview/progress rendering completes.
Changes:
- Adds bounded, shared lifecycle stdout capture.
- Queues output during preview suppression and preserves JSON formatting.
- Ensures final deployment progress renders before queued output.
File summaries
| File | Description |
|---|---|
cli/azd/pkg/input/console.go |
Adds deferred preview teardown and output persistence. |
cli/azd/pkg/input/console_test.go |
Tests persistence, JSON output, and deferred teardown. |
cli/azd/internal/grpcserver/event_service.go |
Captures bounded deploy lifecycle stdout. |
cli/azd/internal/grpcserver/event_service_test.go |
Tests capture, truncation, and grouping. |
cli/azd/internal/cmd/up_graph.go |
Reorders final progress rendering and preview resumption. |
cli/azd/internal/cmd/up_graph_test.go |
Verifies final rendering order. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| require.Contains(t, console.Output(), "RBAC warning") | ||
| } | ||
|
|
||
| func TestEventService_syncExtensionOutput_PersistsConcurrentOutputOnce(t *testing.T) { |
Contributor
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
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.
Why
Deploy lifecycle extensions can print useful warnings during
predeployandpostdeploy. That output used to live only in a temporary preview. Whenazd upor deploy progress rendering paused or cleared the preview, warnings such as the Agents RBAC warning could disappear before the user saw them, even though deployment continued.What changed
keepLogsbehavior.azd up, stop the progress ticker, render the final table, and only then resume the preview. This keeps the final table's cleanup from overwriting a queued warning.keepLogs, and the final progress rendering order.Why this approach
The host already receives extension stdout and owns both the preview and progress rendering. Capturing the output there fixes the lost-warning path without changing the gRPC protocol. Reusing
Console.Messagealso keeps text and JSON output on the existing code paths.Scope and limitations
This PR retains stdout from
predeployandpostdeploylifecycle handlers. It does not capture warnings written to an extension's stderr throughazdext.Output.Warning. Output from concurrent handlers in one extension process can still be combined. A request-correlated structured warning channel would require a separate protocol change.Testing
go test ./pkg/input ./internal/grpcserver -count=1go test ./internal/cmd -run 'TestFinishDeployProgressRendersBeforeResumingPreviewer|TestPhaseTimingBreakdown' -count=1go test -race ./pkg/input ./internal/grpcserver -count=1could not run locally because CGO is disabled.go test ./...was attempted; unrelated environment-dependent failures remain in existing command, middleware, language-tool, and functional tests because of missing external tools, network access, and timing-sensitive startup behavior.Fixes: #7828