Skip to content

Restore and deprecate devtools package#410

Merged
lilyydu merged 1 commit intomainfrom
deprecate/devtools-package
Apr 25, 2026
Merged

Restore and deprecate devtools package#410
lilyydu merged 1 commit intomainfrom
deprecate/devtools-package

Conversation

@heyitsaamir
Copy link
Copy Markdown
Collaborator

Summary

  • Restores the devtools package to the workspace (was previously removed)
  • Marks it as deprecated with README warning, FutureWarning on import, and Development Status :: 7 - Inactive classifier
  • Recommends testing with Teams directly or the Agents Playground

Test plan

  • poe check passes
  • pyright passes
  • python -c "import microsoft_teams.devtools" shows FutureWarning
  • README warning renders on GitHub

This package was in preview but won't be maintained to GA.
Added README warning, runtime FutureWarning on import, and
"Development Status :: 7 - Inactive" classifier. Recommends
testing with Teams directly or the Agents Playground.

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 25, 2026 00:29
@lilyydu lilyydu merged commit 09055bb into main Apr 25, 2026
9 checks passed
@lilyydu lilyydu deleted the deprecate/devtools-package branch April 25, 2026 00:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Restores the previously-removed microsoft-teams-devtools package into the workspace and marks it as deprecated (README warning + import-time warning + inactive classifier), while wiring it into type-checking and the publish pipeline.

Changes:

  • Re-add microsoft-teams-devtools to the workspace (pyproject + pyright config + publish pipeline).
  • Implement the DevTools FastAPI/uvicorn server plugin + v3 route handlers and ship prebuilt web UI assets.
  • Add deprecation messaging (README + FutureWarning on import) and basic tests for the production-environment guard.

Reviewed changes

Copilot reviewed 23 out of 38 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
pyrightconfig.json Includes packages/devtools/src in strict type checking.
pyproject.toml Adds microsoft-teams-devtools as a workspace dependency.
packages/devtools/pyproject.toml Defines the devtools package metadata, deps, and “Inactive” classifier.
packages/devtools/README.md Adds deprecation warning + basic usage snippet.
packages/devtools/tests/test_devtools_plugin.py Adds tests for on_init env guard and basic plugin state.
packages/devtools/src/microsoft_teams/devtools/init.py Emits FutureWarning on import and exports plugin types.
packages/devtools/src/microsoft/teams/devtools/init.py Back-compat import shim with DeprecationWarning.
packages/devtools/src/microsoft_teams/devtools/devtools_plugin.py Core DevTools plugin: HTTP server, websocket fanout, activity plumbing.
packages/devtools/src/microsoft_teams/devtools/event.py Pydantic models for devtools activity events.
packages/devtools/src/microsoft_teams/devtools/page.py Dataclass for registering custom pages.
packages/devtools/src/microsoft_teams/devtools/routes/init.py Exposes route entrypoints and exports.
packages/devtools/src/microsoft_teams/devtools/routes/context.py Defines RouteContext passed through routers.
packages/devtools/src/microsoft_teams/devtools/routes/router.py Top-level router (prefixes /v3).
packages/devtools/src/microsoft_teams/devtools/routes/v3/init.py v3 route module exports.
packages/devtools/src/microsoft_teams/devtools/routes/v3/router.py v3 router composition.
packages/devtools/src/microsoft_teams/devtools/routes/v3/conversations/init.py Conversations route exports.
packages/devtools/src/microsoft_teams/devtools/routes/v3/conversations/router.py Conversations router composition.
packages/devtools/src/microsoft_teams/devtools/routes/v3/conversations/activities/init.py Activities route exports.
packages/devtools/src/microsoft_teams/devtools/routes/v3/conversations/activities/router.py Activities router + POST wiring.
packages/devtools/src/microsoft_teams/devtools/routes/v3/conversations/activities/create.py POST handler to create/process activities via the app pipeline.
packages/devtools/src/microsoft_teams/devtools/web/index.html Web UI entrypoint referencing built assets.
packages/devtools/src/microsoft_teams/devtools/web/icon.png UI icon asset.
packages/devtools/src/microsoft_teams/devtools/web/assets/index-DUmBwYhV.css Bundled UI CSS asset.
packages/devtools/src/microsoft_teams/devtools/web/assets/FluentSystemIcons-Light-eu0dDZrh.woff2 Bundled font asset.
packages/devtools/src/microsoft_teams/devtools/web/assets/FluentSystemIcons-Light-DzHdd4FE.woff Bundled font asset.
.azdo/publish.yml Installs devtools in the publish pipeline’s editable installs list.

logger.debug(f"WebSocket {socket_id} disconnected")
break
finally:
del self.sockets[socket_id]
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on_socket_connection unconditionally does del self.sockets[socket_id] in finally. If the socket was already removed elsewhere (e.g., on send failure / disconnect handling), this will raise KeyError and mask the original disconnect. Use self.sockets.pop(socket_id, None) to make cleanup idempotent.

Suggested change
del self.sockets[socket_id]
self.sockets.pop(socket_id, None)

Copilot uses AI. Check for mistakes.
Comment on lines +134 to +149
async def process(token: TokenProtocol, activity: Activity):
response_future = asyncio.get_event_loop().create_future()
if activity.id:
self.pending[activity.id] = response_future
try:
# Convert Activity to CoreActivity
activity_dict = activity.model_dump(by_alias=True, exclude_none=True)
core_activity = CoreActivity.model_validate(activity_dict)
result = self.on_activity_event(ActivityEvent(body=core_activity, token=token))
# If the handler is a coroutine, schedule it
if asyncio.iscoroutine(result):
asyncio.create_task(result)
except Exception as error:
response_future.set_exception(error)
finally:
result = await response_future
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In process(), if self.on_activity_event(...) returns a coroutine and that task raises, the exception is never observed and response_future is never completed (so the HTTP request will hang forever at await response_future). Consider awaiting the handler directly, or attach a done-callback to the task to set_exception on response_future (and/or add a timeout) to guarantee the future is resolved.

Copilot uses AI. Check for mistakes.
Comment on lines +157 to +160
config = uvicorn.Config(app=self.app, host="0.0.0.0", port=self._port, log_level="info")
self._server = uvicorn.Server(config)

logger.info(f"available at http://localhost:{self._port}/devtools")
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The devtools server is bound to 0.0.0.0, which exposes this explicitly-insecure debugging UI/API to the entire network by default. Bind to 127.0.0.1 (or make the host configurable with a safe default) to reduce accidental exposure.

Suggested change
config = uvicorn.Config(app=self.app, host="0.0.0.0", port=self._port, log_level="info")
self._server = uvicorn.Server(config)
logger.info(f"available at http://localhost:{self._port}/devtools")
host = os.getenv("MICROSOFT_TEAMS_DEVTOOLS_HOST", "127.0.0.1")
config = uvicorn.Config(app=self.app, host=host, port=self._port, log_level="info")
self._server = uvicorn.Server(config)
display_host = "localhost" if host == "127.0.0.1" else host
logger.info(f"available at http://{display_host}:{self._port}/devtools")

Copilot uses AI. Check for mistakes.
Comment on lines +57 to +58
logger.error(err)
raise HTTPException(status_code=500, detail=str(err)) from err
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling here logs with logger.error(err) (no stack trace) and then returns the raw exception string to the client via HTTPException(detail=str(err)). Prefer logger.exception(...) (or logger.error(..., exc_info=True)) and return a generic message to avoid leaking internal details over HTTP (even if this is a dev-only tool).

Suggested change
logger.error(err)
raise HTTPException(status_code=500, detail=str(err)) from err
logger.exception("Failed to create activity")
raise HTTPException(status_code=500, detail="Internal server error") from err

Copilot uses AI. Check for mistakes.
Comment on lines +32 to +34
token = JsonWebToken(
jwt.encode({"serviceurl": f"http://localhost:{context.port}"}, "secret", algorithm="HS256")
)
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This JWT is being created with a hard-coded signing key ("secret"). Even if the SDK later decodes without verifying signatures, hard-coded secrets tend to get flagged by security tooling and can be copied into real usage. Consider generating an ephemeral random key per run, or avoid JWT creation entirely by providing a minimal TokenProtocol implementation that only supplies the needed service_url/caller fields.

Copilot uses AI. Check for mistakes.
Comment on lines +15 to +22
name: str
"The unique name of the view (must be URL safe)"

display_name: str
"The display name of the view to be shown in the view header"

url: str
"The URL of the view"
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field descriptions are currently bare string literals following the annotations (e.g., name: str then a quoted string). These strings are no-ops at runtime and won’t show up in generated docs or type help. Use comments/docstrings, or dataclasses.field(metadata={...}) (or switch to a Pydantic model if you want schema/doc generation).

Copilot uses AI. Check for mistakes.
Comment on lines +240 to +248
async def emit_activity_to_sockets(self, event: DevToolsActivityEvent):
data = event.model_dump(mode="json", exclude_none=True)
for socket_id, websocket in self.sockets.items():
try:
await websocket.send_json(data)
except WebSocketDisconnect:
logger.debug(f"WebSocket {socket_id} disconnected")
del self.sockets[socket_id]

Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emit_activity_to_sockets deletes entries from self.sockets while iterating over self.sockets.items(). Mutating a dict during iteration can raise RuntimeError and can also race with on_socket_connection's cleanup. Collect disconnected socket IDs and remove them after the loop (and prefer pop(..., None) to avoid KeyError).

Copilot uses AI. Check for mistakes.
@heyitsaamir heyitsaamir mentioned this pull request Apr 25, 2026
1 task
heyitsaamir added a commit that referenced this pull request Apr 25, 2026
## Summary
- `uv.lock` was missing the devtools workspace member after it was
restored in #410
- Ran `uv lock` to regenerate

## Test plan
- [ ] `uv sync` works without errors

Co-authored-by: Claude <noreply@anthropic.com>
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.

3 participants