[#572] Implement Teams API Drift Detector workflow - Add metadata check

[Cecilia Avila (ceciliaavila)]

Addresses #572

Description

This pull request introduces a comprehensive workflow and validation framework to ensure that changes to Microsoft Teams API usage and capability metadata are properly reviewed and acknowledged. It does so by updating the CI workflow, extending the drift tooling, documenting the review process, and adding automated tests for the new logic.

CI/CD and Metadata Validation Enhancements:

• New CI workflow for metadata validation:
  The main Python package workflow (.github/workflows/python-package.yml) now detects changes to MSTeams-relevant source or metadata files and conditionally runs a new validation job. This job checks if all relevant changes are properly reflected and acknowledged in the metadata files.

• Source review acknowledgment process:
  The documentation (scripts/teams-api-drift/README.md) now describes how to acknowledge source changes that do not require metadata updates. It introduces a structured way to record the outcome and reasoning for such acknowledgments in both teams-api-usage-manifest.json and teams-capabilities.yaml.

Tooling and Test Coverage Improvements:

• Drift tool enhancements:
  The drift tool (scripts/teams-api-drift/teams_api_drift/cli.py) and its underlying logic now support validating that source changes are accompanied by proper metadata updates or explicit acknowledgments, using a new --base-ref option to compare against the base branch. [1] [2] [3]

• Automated tests for metadata review logic:
  A new test suite (tests/teams_api_drift/test_metadata.py) covers scenarios such as unrecorded imports, missing or stale metadata, proper and improper review acknowledgments, and capability review requirements.

Capability Ownership Updates:

• Capability and ownership mapping updates:
  The Teams capabilities configuration (libraries/microsoft-agents-hosting-msteams/config/teams-capabilities.yaml) is updated to clarify ownership, add new owners for certain capabilities, and reference the new review acknowledgment process. [1] [2]

These changes together ensure that all relevant source and metadata changes are reviewed, acknowledged, and validated automatically, improving the reliability and maintainability of Teams API integrations.

Testing

These images show the CI workflow failing when issues are detected. And then passing after the issues are corrected.

[Copilot]

🟡 Changes recommended

Unresolved critical workflow security risks and moderate drift-validation defects block approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR adds Teams API drift detection, metadata validation, compatibility analysis, reporting, and CI workflows.

Changes:

• Adds extraction, comparison, classification, candidate, metadata, and reporting tooling.
• Adds metadata manifests, capability ownership, acknowledgments, documentation, and tests.
• Adds PR/scheduled workflows and Python CI integration.

Review status: unresolved critical workflow security risks and moderate drift-validation issues remain.

File summaries

File	Summary
tests/teams_api_drift/test_reports.py	Tests report rendering and validation.
tests/teams_api_drift/test_metadata.py	Tests metadata review enforcement.
tests/teams_api_drift/test_extraction.py	Tests API model extraction.
tests/teams_api_drift/test_analysis.py	Tests comparison and classification.
tests/teams_api_drift/contracts.py	Defines candidate API contracts.
tests/teams_api_drift/conftest.py	Configures drift tooling tests.
tests/hosting_msteams/test_api_boundaries.py	Tests Teams API boundaries.
scripts/teams-api-drift/teams-api-drift.py	Provides the CLI entry point.
scripts/teams-api-drift/teams-api-agent-report-prompt.md	Defines the advisory report format.
scripts/teams-api-drift/teams_api_drift/resolve.py	Resolves dependency versions.
scripts/teams-api-drift/teams_api_drift/report.py	Renders and validates reports.
scripts/teams-api-drift/teams_api_drift/metadata.py	Validates usage and capability metadata.
scripts/teams-api-drift/teams_api_drift/extract.py	Extracts upstream API models.
scripts/teams-api-drift/teams_api_drift/compare.py	Compares API versions.
scripts/teams-api-drift/teams_api_drift/common.py	Provides shared utilities.
scripts/teams-api-drift/teams_api_drift/cli.py	Implements drift subcommands.
scripts/teams-api-drift/teams_api_drift/classify.py	Classifies API impact.
scripts/teams-api-drift/teams_api_drift/candidate.py	Builds candidate environments.
scripts/teams-api-drift/teams_api_drift/__init__.py	Defines tooling metadata.
scripts/teams-api-drift/requirements.txt	Lists tooling dependencies.
scripts/teams-api-drift/README.md	Documents tooling and review processes.
scripts/teams-api-drift/mypy.ini	Configures static checking.
scripts/README.md	Links drift tooling documentation.
libraries/microsoft-agents-hosting-msteams/teams-api-usage-manifest.json	Records Teams API usage.
libraries/microsoft-agents-hosting-msteams/pyproject.toml	Configures package discovery.
libraries/microsoft-agents-hosting-msteams/config/teams-capabilities.yaml	Defines capability ownership and policies.
dev_dependencies.txt	Adds development dependencies.
.gitignore	Excludes generated drift artifacts.
.github/workflows/teams-api-drift-scheduled.yml	Adds scheduled drift detection.
.github/workflows/teams-api-drift-prs.yml	Adds PR drift checks.
.github/workflows/python-package.yml	Integrates metadata validation into CI.

Review details

Suppressed comments (4)

scripts/teams-api-drift/teams_api_drift/classify.py:128

• Required-field additions on publicly exposed upstream types are not treated as relevant unless the SDK constructs or validates that type. The manifest explicitly marks type-reference usages as publicly exposed, so a downstream handler can consume the upstream model even when this package never constructs it; this logic currently downgrades such an incompatible public contract to no action/review. Include publicly-exposed/re-exported exposure in this relevance condition for required additions and requiredness changes.

                and not change["after"]["optional"]
                or kind == "property-requiredness-changed"
                and change["after"] is False
            )
            and usage.get("constructsOrValidates", False)

scripts/teams-api-drift/teams_api_drift/compare.py:90

• An empty before list is the extractor's representation for a class without an explicit __init__, whose existing callable contract is no-argument. all(...) over that empty list is vacuously true, so adding a constructor with a required parameter is classified as non-breaking and a consumed change can be missed. Model the implicit no-argument constructor (and add a regression case) before evaluating compatibility.

    if all(any(accepts_old_calls(old, new) for new in after) for old in before):
        return "non-breaking"
    return "potentially-breaking"

scripts/teams-api-drift/teams_api_drift/metadata.py:479

• The documented contract requires updating the usage manifest when an import is removed, but this branch lets a fresh no-usage-metadata-change acknowledgment bypass removed_recorded. Since current validation only rejects missing imports, a PR can delete a Teams API import and leave its stale usage entry in the manifest while passing review. Require the stale entry to be removed instead of accepting the non-impact acknowledgment for this case.

    if removed_recorded and not explicit_usage_review:
        errors.append(
            "Removed direct Teams API imports remain recorded: "
            + ", ".join(removed_recorded)
            + f"; update the usage or follow {SOURCE_REVIEW_GUIDE}"

scripts/teams-api-drift/teams_api_drift/metadata.py:352

• Because _run_git returns result.stdout for successful commands, git merge-base --is-ancestor returns an empty string on success. The is not None check is therefore false whenever the metadata commit is a descendant of the source commit but not identical, so valid PRs that update metadata in a later commit are rejected as stale. Check the successful return value explicitly (or expose the return code).

        or _run_git(
            root,
            "merge-base",
            "--is-ancestor",
            source_commit,
            metadata_commit,
            check=False,
        )
        is not None

• Files reviewed: 30/31 changed files
• Comments generated: 6
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Five unresolved moderate findings must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

scripts/teams-api-drift/teams_api_drift/metadata.py:635

• The replacement validator no longer validates adoptionPolicy, although read_capabilities previously required one of the three supported policies (scripts/teams-api-drift/teams_api_drift/classify.py:69-75). A misspelled policy can pass verify-usage, and classify will then fail to apply the intended review/strict behavior for new upstream members; validate the policy as part of this check.

        if not isinstance(owners, list) or not isinstance(areas, list):
            errors.append(f"Capability {name} must include owners and upstreamAreas")
            continue

scripts/teams-api-drift/teams_api_drift/metadata.py:609

• An empty files list is accepted here, whereas the existing manifest validator requires every usage to name at least one source file (scripts/teams-api-drift/teams_api_drift/classify.py:23-29). That permits a phantom usage record to pass validation while contributing no affected files to drift classification or reports; reject empty lists as well.

            not isinstance(symbol, str)
            or not isinstance(usage.get("usage"), str)
            or not isinstance(files, list)

scripts/teams-api-drift/teams_api_drift/metadata.py:146

• The static visitor does not record a direct call to an imported model or client class: _value_symbol only updates variable tracking, while members contains only attribute reads/writes and method calls. Since constructsOrValidates is never checked later, adding construction of an already-recorded symbol can pass without updating the usage manifest, despite the maintenance contract requiring construction changes to be recorded. Track constructor calls and validate the corresponding usage metadata.

    def _value_symbol(self, value):
        if not isinstance(value, ast.Call):
            return None
        if isinstance(value.func, ast.Name):
            return self.aliases.get(value.func.id)

scripts/teams-api-drift/teams_api_drift/metadata.py:203

• _imports_from_text accepts import microsoft_teams.api... imports, but this alias collection only handles ImportFrom. Consequently code such as import microsoft_teams.api.models as models; models.ChannelData.model_validate(...) is recorded as an import but its model members, calls, and public annotations are never statically checked, so required metadata can be removed without detection. Add module-alias/member resolution here or reject unsupported module-import forms explicitly.

            isinstance(node, ast.ImportFrom)
            and node.module
            and node.module.startswith("microsoft_teams.api")
        ):
            for name in node.names:
                aliases[name.asname or name.name] = f"{node.module}.{name.name}"
    visitor = _UsageVisitor(aliases)

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Eleven unresolved moderate findings affect CI enforcement and metadata validation correctness.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (10)

.github/workflows/python-package.yml:18

• This job is gated to pull requests, so the workflow's push trigger and PUSH_BASE_SHA fallback are never used. A direct push to main or release/* can change Teams source or metadata without running this validation, defeating the enforcement described in the PR. Allow the job for push events as well.

    if: ${{ github.event_name == 'pull_request' }}

scripts/teams-api-drift/teams_api_drift/metadata.py:480

• An explicit no-usage-metadata-change review suppresses this error, so a direct Teams import can be removed from source while its old usage entry remains in the manifest and still pass validation. The README requires the manifest to change when imports are removed; acknowledgments are only for edits that do not affect metadata. Treat removed_recorded as an unconditional failure and reserve the acknowledgment for cases without stale records.

    if removed_recorded and not explicit_usage_review:
        errors.append(
            "Removed direct Teams API imports remain recorded: "
            + ", ".join(removed_recorded)
            + f"; update the usage or follow {SOURCE_REVIEW_GUIDE}"
        )

scripts/teams-api-drift/teams_api_drift/metadata.py:159

• This assignment tracking only recognizes direct model construction or validation calls. It does not infer the return symbol of a helper call; for example, _utils.py assigns channel_data = _try_get_channel_data(activity) even though that helper returns ChannelData, so subsequent field reads are invisible to propertiesRead validation. A source edit can therefore bypass the metadata check; add return-flow handling and a regression test.

    def visit_Assign(self, node):
        symbol = self._value_symbol(node.value)
        if symbol:
            for target in node.targets:
                if isinstance(target, ast.Name):

scripts/teams-api-drift/teams_api_drift/metadata.py:253

• The static check is one-way: it reports source members missing from the manifest, but never reports members that remain declared after being removed from source. A deleted event_type can stay in propertiesRead; updating sourceReview makes the change pass while leaving stale metadata that can affect later drift classification. Compare base/current static usages or reject stale declarations.

                action = {"method": "called", "write": "written", "read": "read"}[kind]
                errors.append(
                    f"{symbol}.{member} is statically {action} in {file}:{line} but "
                    f"is absent from {field}"
                )

scripts/teams-api-drift/teams_api_drift/metadata.py:146

• _value_symbol uses a direct TeamsModel(...) call only to bind an assigned variable; the visitor never records the construction itself or validates constructsOrValidates. Thus changing a recorded type-reference (for example MeetingInfo) to instantiate the model can pass verify-usage with constructsOrValidates: false, while classify.relevant() relies on that flag for constructor and required-field drift. Track constructor calls and cover this transition with a regression test.

    def _value_symbol(self, value):
        if not isinstance(value, ast.Call):
            return None
        if isinstance(value.func, ast.Name):
            return self.aliases.get(value.func.id)

scripts/teams-api-drift/teams_api_drift/metadata.py:610

• The previous validate_manifest required each usage to have a non-empty files list, but this replacement only checks that it is a list. An orphan usage entry can now pass verify-usage without pointing to any source file, weakening the manifest coverage check; retain the non-empty invariant.

        if (
            not isinstance(symbol, str)
            or not isinstance(usage.get("usage"), str)
            or not isinstance(files, list)
        ):

scripts/teams-api-drift/teams_api_drift/metadata.py:69

• The former validate_manifest explicitly rejected from microsoft_teams.api... import *, but this replacement records ...* as an ordinary symbol and accepts it when the manifest contains the same entry. Because verify-usage no longer calls the former validator, wildcard imports can bypass the fully-qualified usage tracking this tool relies on; reject them here.

            for name in node.names:
                imports.append(
                    TeamsImport(f"{node.module}.{name.name}", file, node.lineno)
                )

scripts/teams-api-drift/teams_api_drift/metadata.py:561

• The previous validate_manifest path rejected a non-object manifest through validate_artifact, but this replacement calls .get before checking the decoded type. If the JSON is accidentally [], verify-usage raises AttributeError with a traceback instead of the intended validation error; guard isinstance(manifest, dict) before accessing its keys.

    if manifest.get("schemaVersion") != 1 or manifest.get("dependency") != DEPENDENCY:
        errors.append(
            "Usage manifest must be a schemaVersion 1 document for the dependency"
        )

scripts/teams-api-drift/teams_api_drift/metadata.py:581

• declared_requirement() accepts ranges and wildcards, and comparing str(requirement.specifier) to the manifest only checks consistency. Updating setup.py to microsoft-teams-api>=2.0.16 and the manifest accordingly would make verify-usage pass, despite the repository contract and resolve_versions requiring one exact == pin; enforce that constraint with declared_version before accepting the metadata.

        if manifest.get("declaredVersion") != str(requirement.specifier):
            errors.append(
                "Usage manifest declaredVersion must match setup.py "
                f"({requirement.specifier})"
            )

scripts/teams-api-drift/teams_api_drift/metadata.py:244

• This coverage test truncates both paths to their first component, so declaring settings also covers settings.selected_channel and settings.selected_channel.id. A source change that starts reading a nested Teams field can therefore pass without recording the nested path required by the manifest documentation; compare normalized full paths instead.

            covered = any(
                value == member
                or value.replace("[]", "").split(".")[0] == member.split(".")[0]
                for value in declared

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

The base branch was changed.