Skip to content

[agentserver] Fix review comments for azure-ai-agentserver invoke API#45490

Closed
Copilot wants to merge 4 commits intomainfrom
copilot/suggest-fixes-for-pr-45481
Closed

[agentserver] Fix review comments for azure-ai-agentserver invoke API#45490
Copilot wants to merge 4 commits intomainfrom
copilot/suggest-fixes-for-pr-45481

Conversation

Copy link
Contributor

Copilot AI commented Mar 3, 2026

Addresses all automated review comments from PR #45481 introducing the azure-ai-agentserver package. Fixes bugs that would cause silent failures, incorrect HTTP semantics, and resource leaks.

Bug fixes (server/_base.py, _logger.py)

  • import logging.config missingconfigure_logging() silently failed with AttributeError caught by broad except
  • Unsafe async generator detectionisinstance(result, AsyncGenerator) with typing.AsyncGenerator can raise TypeError at runtime; replaced with inspect.isasyncgen() / inspect.isawaitable()
  • Server errors returned HTTP 400 — invoke/get/cancel exception handlers now return 500; raw str(exc) gated behind AGENT_DEBUG_ERRORS env var to prevent leaking internal details
  • Response validation used request Accept header — now uses "application/json" (the actual response media type)
  • Non-streaming response missing explicit content-type — added media_type="application/json" to Response()
  • Unused import json — removed (Ruff F401)

Test fixes

  • conftest.py fixtures — converted all httpx.AsyncClient fixtures to async generators using async with / yield to ensure proper teardown
  • Inline clients in tests — replaced bare client = httpx.AsyncClient(...) with async with httpx.AsyncClient(...) as client: in test_get_cancel.py and test_openapi_validation.py
  • caplog unused in test_response_validation_logs_warning — added assertion on caplog.records so the warning is actually verified
  • Status code assertions — updated tests expecting 400 → 500 to match corrected server behavior

Packaging / CI

  • Removed unused hard dependencies from pyproject.toml: azure-identity and azure-monitor-opentelemetry are not imported anywhere in the package
  • Sample requirements.txt — removed >=1.0.0b1 version pins from all 6 sample requirements.txt files, consistent with other agentserver samples
  • ci.yml — added azure-ai-agentserver to the Artifacts list so it is built, tested, and linted by the pipeline

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • scanning-api.github.com
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits March 3, 2026 17:07
Co-authored-by: zhiyong-gayang <15938424+zhiyong-gayang@users.noreply.github.com>
Co-authored-by: zhiyong-gayang <15938424+zhiyong-gayang@users.noreply.github.com>
Copilot AI changed the title [WIP] Review comments and suggest fixes for PR 45481 [agentserver] Address PR #45481 review comments Mar 3, 2026
Co-authored-by: zhiyong-gayang <15938424+zhiyong-gayang@users.noreply.github.com>
Copilot AI changed the title [agentserver] Address PR #45481 review comments [agentserver] Fix review comments for azure-ai-agentserver invoke API Mar 3, 2026
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.

2 participants