Fix extension lifecycle event handlers being silently dropped#7562
Merged
JeffreyCA merged 4 commits intoApr 8, 2026
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes a Go closure identity bug where multiple extensions registering lifecycle event handlers could cause later registrations (and context-cleanup removals) to be silently misidentified and dropped/removed.
Changes:
- Replace code-pointer-based handler deduplication with unique monotonic registration IDs to preserve distinct handler registrations.
- Update context-based cleanup to remove handlers by registration ID to avoid removing the wrong handler.
- Adjust and add tests to validate same-literal closures register independently and context cancellation removes only the intended handler.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| cli/azd/pkg/ext/event_dispatcher.go | Tracks handlers via per-registration IDs; context cleanup removes by ID; updates handler storage and dispatch loop. |
| cli/azd/pkg/ext/event_dispatcher_test.go | Updates existing tests for multi-registration behavior and adds coverage for same-literal closure registration and cleanup correctness. |
2dc2c0c to
4457b19
Compare
e2d45ca to
e406089
Compare
804abb4 to
84fdcbb
Compare
7afe7ba to
c99a115
Compare
Replace fmt.Sprintf-based handler dedup in EventDispatcher with ID-based registration and context-scoped lifecycle management. The old dedup compared closure code pointers, which are shared across closures from the same function literal, causing only one extension's handler to survive when multiple extensions subscribe to the same event. - EventDispatcher: unique ID per registration, context-based cleanup, stale handler skip in RaiseEvent - Inline hooks: fresh temp script per execution via PrepareExecutionPath - Service hooks: signature-based idempotent registration on ServiceConfig - Workflow steps: scoped child contexts with defer cancel for handler cleanup - Config reloads: CopyRuntimeStateTo preserves dispatchers and hook state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
9d8ec6f to
92697e6
Compare
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
|
jongio
approved these changes
Apr 8, 2026
jongio
left a comment
Member
There was a problem hiding this comment.
Solid fix for the closure code-pointer aliasing bug. The registration ID approach is clean, the stale-context skip in RaiseEvent is good defense-in-depth, and the ctx.Done() != nil guard prevents unnecessary goroutines.
Tests cover the key scenarios - same-literal dedup, context cleanup, and the race condition that originally triggered #6216.
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 #6216
Problem
When multiple extensions (e.g.,
microsoft.azd.demoandazure.ai.agents) are installed and both subscribe to the same lifecycle event (e.g.,preprovision), only the first extension's handler runs. The second extension's handler is silently dropped during registration.This happens because
EventDispatcher.AddHandlercompared handlers by code pointer (fmt.Sprintf("%v", handler)) to deduplicate them. Closures from the same function literal can share the same code pointer even when they capture different state (different extensions, brokers, etc.), so the second handler could be incorrectly treated as a duplicate. The behavior is non-deterministic because which extension registers first depends on startup timing, and whether closures share a code pointer depends on Go compiler inlining decisions.The dedup was added in #6012 to address #6011, where #5964 changed
ProjectConfigfrom transient to singleton, causing hooks to accumulate across workflow steps. That fix solved the immediate accumulation problem, but the underlying issue is lifecycle management — which this PR addresses holistically.Fix
This change replaces the pointer-based dedup with proper lifecycle management at each layer:
EventDispatcher: ID-based handler trackingAddHandlercall assigns a unique monotonic ID — no morefmt.Sprintfpointer comparison.RemoveHandler(pointer-based) is replaced byremoveByID(ID-based), called automatically when the registration context is cancelled.RaiseEventskips handlers whose registration context is already cancelled, providing a synchronous guard against stale handlers during the window before the async cleanup goroutine runs.Inline hooks create a fresh temp script per execution
HookConfig.validate()no longer creates and caches a one-time temp file.PrepareExecutionPath()creates the temp script immediately before execution.Service hook registration is explicitly idempotent
ServiceConfigtracks the currently installedazure.yamlservice-hook registration by a stable signature of the hook configuration plus the registration context lifetime.EnsureHooksRegisteredskips re-registration when the signature matches and the context is still alive.RollbackHookRegistrationcleans up after failed registration attempts.Workflow steps get scoped child contexts
workflowCmdAdapter.ExecuteContextwraps each step incontext.WithCancel+defer cancel().Initializemethods) are marked as expired and cleaned up.azure.yamlare registered at the top-level context, so they persist correctly across all workflow steps.Config reloads preserve runtime hook state
CopyRuntimeStateToonProjectConfig/ServiceConfigpreserves event dispatchers and hook registration state during config reloads, so updates do not accidentally drop or duplicate in-memory hook behavior.Testing
New and updated tests cover:
Test_Closures_From_Same_Literal)Test_Context_Cleanup_With_Same_Literal_Closures)Test_Workflow_Step_Race_No_Duplicate_Execution)Inline_Hook_Can_Run_Twice)RemoveHandlertests)