Skip to content

Fix signal handling resource leak in azure.ai.agents run command - #7028

Closed
Travis Angevine (trangevi) with Copilot wants to merge 20 commits into
trangevi/dev-commandfrom
copilot/sub-pr-7026
Closed

Fix signal handling resource leak in azure.ai.agents run command#7028
Travis Angevine (trangevi) with Copilot wants to merge 20 commits into
trangevi/dev-commandfrom
copilot/sub-pr-7026

Conversation

Copilot AI commented Mar 6, 2026

Copy link
Copy Markdown
Contributor
  • Fix signal handling in run.go:
    • Add syscall.SIGTERM alongside os.Interrupt for complete signal handling
    • Call signal.Stop(sigCh) (deferred in goroutine) to prevent resource leak
    • Use a done channel to ensure goroutine exits when subprocess exits normally
  • Rebase branch onto main (base changed from trangevi/dev-command to main)

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: trangevi <26490000+trangevi@users.noreply.github.com>
Copilot AI changed the title [WIP] WIP Address feedback on commands support in Agents Extension PR Fix signal handling resource leak in azure.ai.agents run command Mar 6, 2026
@trangevi
Travis Angevine (trangevi) marked this pull request as ready for review March 6, 2026 23:33
@vhvb1989

Copy link
Copy Markdown
Member

Copilot can you rebase this onto main

Wei Lim (weikanglim) and others added 8 commits March 7, 2026 00:52
…sioning progress display (#7004)

## Summary

Replace the `cancelProgress` channel in `BicepProvider.Deploy` with `context.WithCancel` for cancelling the progress-reporting goroutine.

## Impact

Users will observe a faster progress update when the deployment is completed, but we're still polling for new progress operations (which is very common).

## Changes

- **`bicep_provider.go`**: Use `context.WithCancel` instead of a channel. Use `sync.WaitGroup` to ensure the goroutine fully exits before stopping the spinner. 

## Motivation

The channel-based approach blocks the caller on `defer func() { cancelProgress <- true }()` until the goroutine finishes its current `ReportProgress` cycle. With context cancellation:
- `cancelProgress()` returns immediately (broadcast signal via channel close)
- In-flight API calls abort promptly via context
- `wg.Wait()` ensures the goroutine exits before `StopSpinner` is called, eliminating the race where the goroutine could restart the spinner after it was stopped

Contributes to #6915
)

* feat: support deploying Container App Jobs via host: containerapp

Extends host: containerapp to support Azure Container App Jobs (Microsoft.App/jobs).
When Bicep provisions a job tagged with azd-service-name, azd deploy builds/pushes
the container image and updates the job automatically.

Closes #2743

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: update test skip message to reference tracking issue #7014

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat(azdext): add P1 core extension primitives

Implements #6944 core primitives for token provider, scope detection, resilient HTTP client, and pagination with tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(azdext): harden P1 primitives after quality review

Addresses MQ findings for #6944: bounded response reads, nextLink SSRF protections, retry/body semantics, token-over-http guard, deterministic scope rules, and added regression tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore: fix preflight blockers for PR1

Apply required gofmt and cspell updates so mage preflight passes for draft PR #6954.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* security: harden core primitives against hack scan findings

- mcp_security: tighten input validation and error handling
- pagination: add bounds checking on page parameters
- resilient_http_client: strengthen TLS config and timeout enforcement
- resilient_http_client_test: add security-path test coverage

* fix: address profile review findings for stacked PR

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(azdext): satisfy lint and cspell checks

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(azdext): remediate hack findings

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: address copilot review feedback on PR 6954

- block hostname redirects that resolve to private/loopback IPs\n- return explicit nil-client error in stdHTTPDoer path\n- honor MaxRetries=0 as no retries; use negative as default sentinel\n- update TokenProvider usage snippet to current API\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: address follow-up Copilot feedback on PR 6954

- tighten backoff jitter upper bound\n- require absolute HTTPS nextLink\n- return explicit oversized page response error\n- align OnBlocked docs with implemented actions\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore: retrigger CI for PR #6954

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore: retrigger CI for transient external failures

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(azdext): address actionable main PR review items

- remove mutable redirect lookup test hook via injected helper
- document scope detector servicebus ambiguity and ACR scope semantics
- use slices.Sort for deterministic custom rule ordering
- clarify TokenProvider usage guidance

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(azdext): address remaining maintainer review items

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore(agents): remove unrelated whitespace-only changes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(azdext): redact blocked URL details in policy callback path

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(azdext): add x-ms-client-request-id and align resilient headers

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: address PR review feedback from wbreza

- Prepend custom scope rules before defaults so overrides work (#1)
- Redact URL query params in ScopeDetectorError to prevent leaking secrets (#2)
- Add versioned User-Agent string, make configurable via ResilientClientOptions (#3)
- Set done=true on Collect truncation to prevent surprise continuation (#4)
- Add azdext SDK version constant (version.go)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* azure.ai.agents - detect and classify auth errors from azd host gRPC calls

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Refactor error handling to classify auth errors and improve error messaging in AI service interactions

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add x-ms-client-request-id header, use azdext.Run

* Update `github.com/azure/azure-dev/cli/azd`

---------

Co-authored-by: Jeffrey Chen <jeffreychen@microsoft.com>
* Moving azd core to go 1.26

* linter for latest linter
* Add env var check for vnext experience.

Signed-off-by: trangevi <trangevi@microsoft.com>

* cspell

Signed-off-by: trangevi <trangevi@microsoft.com>

* Apply suggestion from @Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Remove unused import

Signed-off-by: trangevi <trangevi@microsoft.com>

---------

Signed-off-by: trangevi <trangevi@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Victor Vazquez (vhvb1989) and others added 10 commits March 7, 2026 00:52
…r output (#6939)

* Silence extension process stderr logs to prevent call-stack-like error output

Extension processes emit verbose gRPC MessageBroker trace logs via
Go's log.Printf, which defaults to os.Stderr. Since azd captures
extension stderr and includes it in error messages, these internal
diagnostics appear to users as call-stack-like error output.

Silence the global logger in ExtensionHost.Run() by setting
log.SetOutput(io.Discard) unless AZD_EXT_DEBUG=true is set.
This ensures backward compatibility with extensions compiled
against older SDK versions that still use log.Printf directly.

* add tests

* Update cli/azd/pkg/azdext/extension_host.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* address feedback, update tests

* update comment

* address feedback

* address feedback

* lll

* address feedback

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* go version update and linter setup

* typo

* code enhances
#7016)

* feat: add sensible defaults for azd env new / init in --no-prompt mode

When --no-prompt is active:
- Auto-generate environment name from working directory basename
  (via CleanName sanitization) when no name is provided
- Auto-select Azure subscription when only one exists; check
  defaults.subscription config first, then fall back to auto-select
- Auto-set new environment as default (skip Confirm prompt)
- Remove hard error requiring --environment with --template

This enables non-interactive CI/CD flows and extensions to use
azd env new / azd init without implementing custom --no-prompt logic.

Fixes #6934

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* address PR review feedback

- Use azdContext.ProjectDirectory() instead of os.Getwd() for env name
  generation, with fallback to os.Getwd() when azdContext is nil
- Add '..' to degenerate name validation checks
- Improve no-subscriptions error message to not assume unauthenticated
- Document that parens are valid per EnvironmentNameRegexp in test
- Clarify init test comment re: expected panics from unmocked deps

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* ci: retrigger pipeline checks

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: JeffreyCA <9157833+JeffreyCA@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Initial plan

* Release changelog for azd 1.23.8

Co-authored-by: rajeshkamal5050 <11532743+rajeshkamal5050@users.noreply.github.com>

* changelog: add PR #7016 to 1.23.8 release notes

Co-authored-by: rajeshkamal5050 <11532743+rajeshkamal5050@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: rajeshkamal5050 <11532743+rajeshkamal5050@users.noreply.github.com>

Copilot AI commented Mar 7, 2026

Copy link
Copy Markdown
Contributor Author

Copilot can you rebase this onto main

The rebase onto main succeeded locally, but the push recovery logic created messy history by merging the old remote branch with the rebased one (the branch now shows 27 commits ahead of main instead of 9, with duplicate main commits). The code content is correct — only azure.ai.agents extension files differ from main.

To get a clean history, please force-push the correct rebased state from the reflog:

git fetch origin copilot/sub-pr-7026
git checkout copilot/sub-pr-7026
git reset --hard dad35fe6
git push --force-with-lease origin copilot/sub-pr-7026

dad35fe6 is the clean state: 9 commits from the original work + my signal-handling fix, all rebased cleanly on top of origin/main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants