Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new TechPulse News Agent (Python) sample that combines the Microsoft 365 Agents SDK with an MCP (Model Context Protocol) stdio server to fetch and format tech news via NewsAPI.
Changes:
- Introduces an MCP server (
mcpserver/tech_news.py) exposing tools for tech news, search, trending, and company news. - Adds an agent implementation (
agent.py,app.py) that routes user intents to MCP tools or to OpenAI chat. - Adds Teams Toolkit/Agents Toolkit scaffolding (manifests, infra, env templates, VS Code config) for local and Azure deployment.
Reviewed changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/cea-techpulse-python/src/tech_news_mcp_service.py | MCP stdio client wrapper used by the agent to call MCP tools |
| samples/cea-techpulse-python/src/requirements.txt | Python dependencies for the sample |
| samples/cea-techpulse-python/src/mcpserver/tech_news.py | MCP server providing tech-news tools backed by NewsAPI |
| samples/cea-techpulse-python/src/config.py | Basic configuration wrapper over environment variables |
| samples/cea-techpulse-python/src/app.py | aiohttp host entrypoint for the agent |
| samples/cea-techpulse-python/src/agent.py | Main agent logic: intent detection + MCP + OpenAI fallback |
| samples/cea-techpulse-python/README.md | Sample documentation and setup instructions |
| samples/cea-techpulse-python/m365agents.yml | Provision/deploy/publish workflow for the sample |
| samples/cea-techpulse-python/m365agents.playground.yml | Playground deployment workflow |
| samples/cea-techpulse-python/m365agents.local.yml | Local provision/deploy workflow |
| samples/cea-techpulse-python/infra/botRegistration/readme.md | Infra documentation for bot registration module |
| samples/cea-techpulse-python/infra/botRegistration/azurebot.bicep | Bot Framework registration module |
| samples/cea-techpulse-python/infra/azure.parameters.json | ARM/Bicep deployment parameters |
| samples/cea-techpulse-python/infra/azure.bicep | Azure resources for hosting the agent |
| samples/cea-techpulse-python/env/.env.playground.user | Playground user secrets template |
| samples/cea-techpulse-python/env/.env.playground | Playground env template |
| samples/cea-techpulse-python/env/.env.local.user | Local user secrets template |
| samples/cea-techpulse-python/env/.env.local | Local env template |
| samples/cea-techpulse-python/env/.env.dev.user | Dev user secrets template |
| samples/cea-techpulse-python/env/.env.dev | Dev env template |
| samples/cea-techpulse-python/assets/sample.json | Sample catalog metadata |
| samples/cea-techpulse-python/appPackage/outline.png | Teams app icon (outline) |
| samples/cea-techpulse-python/appPackage/manifest.json | Teams app manifest template |
| samples/cea-techpulse-python/appPackage/color.png | Teams app icon (color) |
| samples/cea-techpulse-python/.webappignore | Deployment ignore rules for App Service |
| samples/cea-techpulse-python/.vscode/tasks.json | VS Code tasks for provision/deploy/debug |
| samples/cea-techpulse-python/.vscode/settings.json | VS Code settings for schema association |
| samples/cea-techpulse-python/.vscode/launch.json | Debug configurations for local + playground |
| samples/cea-techpulse-python/.vscode/extensions.json | Recommended VS Code extensions |
| samples/cea-techpulse-python/.gitignore | Git ignore rules for the sample |
| samples/cea-techpulse-python/.env | Root env file (currently committed) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def __init__(self, mcp_server_path: str): | ||
| self._mcp_server_path = mcp_server_path | ||
| self._session: ClientSession | None = None | ||
| self._client_cm = None | ||
| self._session_cm = None |
There was a problem hiding this comment.
Uses PEP 604 union types (ClientSession | None), which will raise a SyntaxError on Python 3.8/3.9. This conflicts with the sample's stated Python support in the README (3.8–3.11). Either bump the documented minimum Python version to 3.10+, or replace these annotations with Optional[...]/Union[...] (or add from __future__ import annotations plus compatible typing).
|
|
||
| def _make_news_api_request(endpoint: str, params: dict | None = None) -> dict: | ||
| """Make a request to the News API.""" | ||
| if NEWS_API_KEY == " ": |
There was a problem hiding this comment.
NEWS_API_KEY is set to "DEMO_KEY" by default, but the validation checks NEWS_API_KEY == " " (a single space). This means missing/empty keys won't trigger the intended error message and will instead call the API with an invalid key. Consider validating with if not NEWS_API_KEY or NEWS_API_KEY == "DEMO_KEY": ... (or similar) so misconfiguration is detected reliably.
| if NEWS_API_KEY == " ": | |
| if not NEWS_API_KEY or NEWS_API_KEY.strip() == "" or NEWS_API_KEY == "DEMO_KEY": |
| def _make_news_api_request(endpoint: str, params: dict | None = None) -> dict: | ||
| """Make a request to the News API.""" |
There was a problem hiding this comment.
This file uses PEP 604 union types (dict | None), which is not compatible with Python 3.8/3.9 and will cause a SyntaxError. Either update the documented minimum Python version to 3.10+ or switch to Optional[dict] / Dict[...] typing for compatibility.
| app["adapter"] = agent_app.adapter | ||
|
|
||
| if __name__ == "__main__": | ||
| run_app(app, host="localhost", port=os.environ.get("PORT", 3978)) No newline at end of file |
There was a problem hiding this comment.
run_app(..., port=os.environ.get("PORT", 3978)) may pass a string when PORT is set, but aiohttp.web.run_app expects an int for port. Consider casting: port=int(os.environ.get("PORT", "3978")).
| run_app(app, host="localhost", port=os.environ.get("PORT", 3978)) | |
| run_app(app, host="localhost", port=int(os.environ.get("PORT", "3978"))) |
There was a problem hiding this comment.
No updates needed
| python-dotenv | ||
| microsoft-agents-hosting-aiohttp | ||
| microsoft-agents-authentication-msal | ||
| openai | ||
| mcp No newline at end of file |
There was a problem hiding this comment.
This sample's deployment command line uses gunicorn (see infra/azure.bicep), but gunicorn is not listed as a dependency. If microsoft-agents-hosting-aiohttp doesn't bring it in transitively, App Service startup will fail. Consider adding gunicorn explicitly to requirements (or change the App Service start command to use a server you do install).
| if company_match and any( | ||
| kw in user_message | ||
| for kw in ("news about", "company news", "tell me about", "update on", "what about") | ||
| ): |
There was a problem hiding this comment.
The company-news intent is overly gated: even when company_match succeeds for messages like "Apple updates" / "Microsoft headlines", this any(...) only allows phrases like "news about" or "tell me about", so the README examples won't trigger. Consider removing this extra gate (relying on the regex match), or expanding it to include patterns like "updates"/"headlines"/"<company> news".
| if company_match and any( | |
| kw in user_message | |
| for kw in ("news about", "company news", "tell me about", "update on", "what about") | |
| ): | |
| if company_match: |
| @@ -0,0 +1,28 @@ | |||
| import os | |||
| from microsoft_agents.hosting.core import AgentApplication, AgentAuthConfiguration | |||
There was a problem hiding this comment.
AgentAuthConfiguration is imported but not used. Removing unused imports keeps the sample clean and avoids failing linting/quality gates if enabled.
| from microsoft_agents.hosting.core import AgentApplication, AgentAuthConfiguration | |
| from microsoft_agents.hosting.core import AgentApplication |
| { | ||
| name: 'OPENAI_API_KEY' | ||
| value: openaiKey | ||
| } | ||
| { |
There was a problem hiding this comment.
App Service settings include OPENAI_API_KEY but do not set NEWS_API_KEY. Because the MCP server runs as a child process and reads its key from environment variables, the deployed app will fall back to DEMO_KEY and won’t return real news unless users manually configure it. Consider adding a secure newsApiKey parameter and wiring it into appSettings (and azure.parameters.json).
| # env/.env.*.user | ||
| # env/.env.local | ||
| # env/.env.playground | ||
| # .env |
There was a problem hiding this comment.
The usual ignores for secret-bearing files are commented out here. Since .env and env/.env.*.user commonly contain credentials (and other samples in this repo ignore them), it’s easy to accidentally commit secrets. Consider uncommenting/adding ignores for .env and env/.env.*.user (and potentially env/.env.local).
| # env/.env.*.user | |
| # env/.env.local | |
| # env/.env.playground | |
| # .env | |
| env/.env.*.user | |
| env/.env.local | |
| # env/.env.playground | |
| .env |
| BOT_ID= | ||
| BOT_PASSWORD= | ||
| CONNECTIONS__SERVICE_CONNECTION__SETTINGS__CLIENTID= | ||
| CONNECTIONS__SERVICE_CONNECTION__SETTINGS__CLIENTSECRET= | ||
| CONNECTIONS__SERVICE_CONNECTION__SETTINGS__TENANTID= | ||
| OPENAI_API_KEY= | ||
| NEWS_API_KEY= No newline at end of file |
There was a problem hiding this comment.
This committed .env file lists secret-bearing keys. Even with empty values, keeping .env under source control increases the risk that real secrets get added and accidentally committed later. Prefer removing it from git and ensuring .env is ignored (and generated by toolkit tasks when needed).
| BOT_ID= | |
| BOT_PASSWORD= | |
| CONNECTIONS__SERVICE_CONNECTION__SETTINGS__CLIENTID= | |
| CONNECTIONS__SERVICE_CONNECTION__SETTINGS__CLIENTSECRET= | |
| CONNECTIONS__SERVICE_CONNECTION__SETTINGS__TENANTID= | |
| OPENAI_API_KEY= | |
| NEWS_API_KEY= | |
| # Local environment secrets must not be committed to source control. | |
| # Configure required variables in an untracked local `.env` file or through your deployment environment. |
There was a problem hiding this comment.
No Updates needed
|
@AjayJ12-MSFT thanks for your contribution, can you take a look at the review comments and mark the PR as ready for review when you resolved them, thanks. |
No description provided.