Skip to content

[#572] Implement Teams API Drift Detector workflow - #582

Merged
Rodrigo Brandão (rodrigobr-msft) merged 18 commits into
mainfrom
southworks/add/teams-api-drift-detector
Sep 24, 2026
Merged

Rodrigo Brandão (rodrigobr-msft) merged 18 commits into
mainfrom
southworks/add/teams-api-drift-detector

Conversation

@ceciliaavila

@ceciliaavila Cecilia Avila (ceciliaavila) commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #572

Description

This pull request introduces a new GitHub Actions workflow, teams-api-drift-prs.yml, to automatically detect and report API drift for the Microsoft Teams API dependency in pull requests that modify the relevant setup.py. The workflow determines if the dependency version has changed, runs compatibility and contract tests, generates detailed reports (including an AI-generated advisory), and publishes a summary comment on the PR with actionable findings and downloadable artifacts.

The most important changes are:

New workflow for Teams API drift detection:

  • Added .github/workflows/teams-api-drift-prs.yml to automate detection of API drift in microsoft-teams-api dependency changes within libraries/microsoft-agents-hosting-msteams/setup.py on PRs and workflow dispatch events.

Automated compatibility and contract testing:

  • The workflow checks out the code, resolves old and new dependency versions, installs required tools, and runs both static and runtime compatibility tests (using mypy and pytest) to verify extension compatibility with the new API version.

Automated report generation and publishing:

  • Generates a deterministic report summarizing API changes, test results, and compatibility findings, and uploads all evidence as workflow artifacts for transparency and traceability.
  • Publishes a summary comment on the pull request with key actionable findings and a link to download the full report and

Testing

These images show the workflows working and the issues/PR comment generated.
image

image

Copilot AI lite review requested due to automatic review settings September 14, 2026 19:07
@ceciliaavila
Cecilia Avila (ceciliaavila) changed the base branch from main to southworks/update/pin-teams-api-version September 14, 2026 19:07
@ceciliaavila
Cecilia Avila (ceciliaavila) added this pull request to stack #583 September 14, 2026 19:08
@ceciliaavila
Cecilia Avila (ceciliaavila) force-pushed the southworks/add/teams-api-drift-detector branch from 28ffe4c to 2267a7f Compare September 14, 2026 19:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Critical workflow and redaction findings plus moderate detector and reporting defects remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds automated Teams API drift detection, compatibility testing, reporting, and PR/scheduled workflow automation.

Changes:

  • Adds API extraction, comparison, classification, candidate setup, and reporting tooling.
  • Adds usage manifests, capability policies, contracts, tests, documentation, and dependencies.
  • Adds workflows for pull-request and scheduled drift checks with artifacts and advisory publication.
File summaries
File Summary
tests/teams_api_drift/test_reports.py Report and context tests.
tests/teams_api_drift/test_extraction.py API extraction tests.
tests/teams_api_drift/test_analysis.py Resolution and classification tests.
tests/teams_api_drift/contracts.py Mypy API contracts.
tests/teams_api_drift/conftest.py Drift tooling test setup.
tests/hosting_msteams/test_api_boundaries.py Runtime boundary tests.
scripts/teams-api-drift/teams-api-drift.py CLI entry point.
scripts/teams-api-drift/teams-api-agent-report-prompt.md Advisory report prompt.
scripts/teams-api-drift/teams_api_drift/resolve.py Version resolution.
scripts/teams-api-drift/teams_api_drift/report.py Report generation and validation. critical (1 vote): secret redaction misses annotated assignments and quoted keys. moderate (2 votes): action-bullet validation does not enforce the required format.
scripts/teams-api-drift/teams_api_drift/extract.py API model extraction.
scripts/teams-api-drift/teams_api_drift/compare.py API comparison. moderate (1 vote): missing extractor output is not normalized to ValueError; keyword-only parameters are compared by position; implicit zero-argument constructors are not handled correctly.
scripts/teams-api-drift/teams_api_drift/common.py Shared utilities.
scripts/teams-api-drift/teams_api_drift/cli.py Tool subcommands.
scripts/teams-api-drift/teams_api_drift/classify.py Usage-impact classification. moderate (1 vote): optional property additions can be incorrectly classified as blocking.
scripts/teams-api-drift/teams_api_drift/candidate.py Candidate environment setup.
scripts/teams-api-drift/teams_api_drift/__init__.py Tool metadata.
scripts/teams-api-drift/requirements.txt Tool dependencies.
scripts/teams-api-drift/README.md Tool documentation.
scripts/teams-api-drift/mypy.ini Contract-check configuration.
scripts/README.md Script documentation link.
libraries/microsoft-agents-hosting-msteams/teams-api-usage-manifest.json Teams API usage map.
libraries/microsoft-agents-hosting-msteams/pyproject.toml Package discovery configuration.
libraries/microsoft-agents-hosting-msteams/config/teams-capabilities.yaml Capability ownership policies.
dev_dependencies.txt Development dependencies.
.gitignore Drift artifact exclusions.
.github/workflows/teams-api-drift-scheduled.yml Scheduled drift workflow. critical (1 vote): installs an unpinned npm package in a privileged workflow.
.github/workflows/teams-api-drift-prs.yml Pull-request drift workflow. critical (1 vote): installs an unpinned npm package in a privileged workflow. moderate (3 votes): gates summary comments on successful classification, suppressing comments when actionable drift is found.
Review details

Suppressed comments (4)

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

  • An optional property-added change on a consumed model falls through to this fallback. When the usage is marked publicly-exposed, it is classified as blocking, even though the README states that additive capabilities are for review and an optional field is non-breaking. Add an explicit review classification for optional property additions before the public-exposure fallback so --fail-on-drift does not block harmless additive upstream fields.
            else:
                classification = "blocking" if publicly_exposed else "required"

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

  • When the extractor command exits without creating the output file, read_json(output) raises FileNotFoundError, not ValueError; the added test_empty_or_malformed_extraction_fails simulates exactly that case and therefore fails before the intended assertion can pass. Normalize missing/invalid extractor output to the documented ValueError contract (or change the test to assert the lower-level exception).
    model = validate_artifact(read_json(output), "symbols")

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

  • Comparing old_params and new_params by position misclassifies a harmless keyword-only insertion as breaking: changing f(x, *, count=1) to f(x, *, timeout=1, count=1) leaves all old calls valid, but this loop compares count with timeout and returns potentially-breaking. Match keyword-only parameters by name while preserving positional-order checks so the detector does not block compatible upstream releases.
        for previous, current in zip(old_params, new_params):
            if any(
                previous.get(key) != current.get(key)
                for key in ("name", "kind", "type", "default")
            ):

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

  • An empty before list is produced for a class that only has Python's implicit no-argument constructor, but all(...) over that list is True, so any newly introduced constructor is marked non-breaking without checking whether it still accepts zero arguments. A class changing from no explicit __init__ to __init__(token) will therefore be classified only as review instead of a required/blocking adaptation. Treat an empty baseline as an implicit zero-argument signature before applying the compatibility check.
    if all(any(accepts_old_calls(old, new) for new in after) for old in before):
        return "non-breaking"
  • Files reviewed: 27/28 changed files
  • Comments generated: 5
  • Review effort level: Lite

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

Comment thread .github/workflows/teams-api-drift-prs.yml Outdated
Comment thread .github/workflows/teams-api-drift-scheduled.yml Outdated
Comment thread scripts/teams-api-drift/teams_api_drift/report.py Outdated
Comment thread .github/workflows/teams-api-drift-prs.yml Outdated
Comment thread scripts/teams-api-drift/teams_api_drift/report.py Outdated
Copilot AI review requested due to automatic review settings September 14, 2026 19:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Unresolved critical workflow-permission risk and multiple moderate correctness and reporting issues require changes and human review.

Review details

Suppressed comments (8)

.github/workflows/teams-api-drift-prs.yml:210

  • This resolves the latest @github/copilot package on every privileged run without a version or integrity pin. A changed or compromised package can alter the reporting job and makes the workflow non-reproducible; pin a reviewed package version and preferably install from a lockfile/checksum.
        run: npm install --global @github/copilot

.github/workflows/teams-api-drift-prs.yml:253

  • detect --fail-on-drift intentionally exits 1 when it finds a blocking or required change (line 163), and continue-on-error preserves that as steps.classify-usage-impact.outcome == 'failure'. This condition is therefore false exactly when the report contains actionable breaking findings, so the PR comment is skipped. Gate on a completed classification plus the existence of findings.json instead of requiring a successful outcome.
        if: ${{ always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository && steps.render-deterministic-report.outcome == 'success' && steps.classify-usage-impact.outcome == 'success' }}

.github/workflows/teams-api-drift-scheduled.yml:182

  • This resolves the latest @github/copilot package on every scheduled run without a version or integrity pin. A changed or compromised package can alter the reporting job and makes the workflow non-reproducible; pin a reviewed package version and preferably install from a lockfile/checksum.
        run: npm install --global @github/copilot

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

  • The compatibility check matches parameters strictly by position and name, so adding an optional keyword-only parameter before an existing keyword-only parameter is reported as potentially breaking even though all old calls remain valid (for example, (x, *, timeout=1) to (x, *, retries=3, timeout=1)). Because callers classify anything other than non-breaking as required/blocking, valid upstream additions can fail the detector; match keyword-only parameters by name/kind and require only newly added ones to be optional.
        old_params, new_params = old["parameters"], new["parameters"]
        if len(new_params) < len(old_params):
            return False
        for previous, current in zip(old_params, new_params):
            if any(
                previous.get(key) != current.get(key)
                for key in ("name", "kind", "type", "default")
            ):
                return False
            if previous.get("optional") and not current.get("optional"):
                return False
        return all(
            p.get("optional") or p.get("kind") in ("VAR_POSITIONAL", "VAR_KEYWORD")
            for p in new_params[len(old_params) :]

scripts/teams-api-drift/teams_api_drift/report.py:298

  • The validator treats any bullet containing a known TSAPI/EXTAPI token as valid, so output such as - **TSAPI-0001** — not an advisory passes even though the prompt requires the exact ... — Advisory: form (and the exact no-findings form). Because this result gates publication, validate the complete bullet syntax rather than only checking for an ID.
                re.match(r"\s*(?:[-*+]|\d+[.)])\s", line)
                and not re.search(ID_PATTERN, line)
                and not re.match(r"\s*- No ", line, re.I)
                and not aggregate_no_action
            ):

scripts/teams-api-drift/teams_api_drift/report.py:283

  • known is built from the complete findings artifact, but validation receives no omittedReviewFindingIds from prepare_context. Consequently an AI report can cite a review finding whose details were deliberately omitted from its context and still pass validation, contrary to the prompt's prohibition on recommendations about omitted findings. Pass the bounded context's omitted-ID set into validation and reject those references.
    known = {item["id"] for item in findings["findings"]}
    referenced = set(re.findall(ID_PATTERN, report))
    unknown = sorted(referenced - known)
    missing = sorted(
        item["id"]
        for item in findings["findings"]
        if item["classification"] in ("blocking", "required")
        and item["id"] not in referenced
    )

scripts/teams-api-drift/teams_api_drift/report.py:165

  • The secret redaction pattern only matches cases where the quote follows token, client_secret, etc. directly after : or =. Common annotated assignments such as client_secret: str = "..." and token: str = "..." are not matched, so literal credentials in a selected source slice can be sent to Copilot despite the workflow's redaction guarantee. Handle optional type annotations and assignment syntax before publishing the context.
    return re.sub(
        r"(?i)((?:client_?secret|api_?key|password|token)\s*[:=]\s*['\"])[^'\"]+",
        r"\1[REDACTED]",
        text,

scripts/teams-api-drift/teams_api_drift/report.py:282

  • referenced is global across the whole document, so placing a blocking or required finding ID under No action (or another unrelated section) satisfies missing and allows the report to publish. The prompt assigns finding categories to specific sections; validate each finding's allowed section instead of only checking that its ID appears somewhere.
    known = {item["id"] for item in findings["findings"]}
    referenced = set(re.findall(ID_PATTERN, report))
    unknown = sorted(referenced - known)
    missing = sorted(
        item["id"]
        for item in findings["findings"]
        if item["classification"] in ("blocking", "required")
        and item["id"] not in referenced
  • Files reviewed: 27/28 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread .github/workflows/teams-api-drift-prs.yml Outdated
Copilot AI review requested due to automatic review settings September 14, 2026 19:33
@ceciliaavila
Cecilia Avila (ceciliaavila) force-pushed the southworks/add/teams-api-drift-detector branch from 2267a7f to d21307b Compare September 14, 2026 19:33
@ceciliaavila
Cecilia Avila (ceciliaavila) force-pushed the southworks/update/pin-teams-api-version branch from add5099 to afb5228 Compare September 14, 2026 19:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved critical security and correctness findings, along with moderate validation issues, block approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (9)

.github/workflows/teams-api-drift-prs.yml:253

  • detect --fail-on-drift deliberately returns exit code 1 when it finds blocking/required findings, even though it writes findings.json first. This condition therefore suppresses the PR comment for exactly the actionable drift cases the comment is meant to expose; gate on a non-skipped classification plus the findings artifact instead of requiring a successful exit.
        if: ${{ always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository && steps.render-deterministic-report.outcome == 'success' && steps.classify-usage-impact.outcome == 'success' }}

.github/workflows/teams-api-drift-prs.yml:210

  • This installs a floating npm package in a workflow that later runs the resulting copilot executable with GITHUB_TOKEN and write permissions. A future package update or compromised publish can execute code or alter the advisory before publication; pin an audited CLI version and verify its integrity before granting it the token.
        run: npm install --global @github/copilot

.github/workflows/teams-api-drift-prs.yml:229

  • The current Copilot CLI exposes web access as web_fetch/web_search, but this deny list only names url; deny patterns match tool kinds, so the agent can still invoke web tools despite the prompt's no-tools requirement. Use an explicit --excluded-tools/--available-tools allowlist (or deny every relevant current tool kind) so untrusted context cannot trigger network access.
                      ["copilot", "-s",
                       "--deny-tool=shell,write,read,url,memory,github", "--no-ask-user"],

.github/workflows/teams-api-drift-prs.yml:62

  • For manual dispatch, equality is checked before either input is parsed or resolved. Thus from=not-a-version and to=not-a-version are treated as an unchanged comparison and the workflow succeeds without validating that the requested release exists. Validate both exact versions before taking the skip path.
          if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then
            if [[ "$INPUT_FROM" == "$INPUT_TO" ]]; then
              echo "run=false" >> "$GITHUB_OUTPUT"
              echo "No Teams API version change: $INPUT_FROM" >> "$GITHUB_STEP_SUMMARY"
            else
              echo "run=true" >> "$GITHUB_OUTPUT"
            fi

.github/workflows/teams-api-drift-scheduled.yml:201

  • The current Copilot CLI exposes web access as web_fetch/web_search, but this deny list only names url; deny patterns match tool kinds, so the agent can still invoke web tools despite the prompt's no-tools requirement. Use an explicit --excluded-tools/--available-tools allowlist (or deny every relevant current tool kind) so untrusted context cannot trigger network access.
                      ["copilot", "-s",
                       "--deny-tool=shell,write,read,url,memory,github", "--no-ask-user"],

.github/workflows/teams-api-drift-scheduled.yml:182

  • The scheduled workflow installs a floating npm package and then runs its copilot executable with GITHUB_TOKEN and issue/coprocessing permissions. A future package update or compromised publish can execute code or alter the advisory before publication; pin an audited CLI version and verify its integrity before granting it the token.
        run: npm install --global @github/copilot

libraries/microsoft-agents-hosting-msteams/teams-api-usage-manifest.json:4

  • declaredVersion is never read by verify-usage or any detector path; the workflows resolve the pin directly from setup.py. A future Teams API upgrade can therefore leave this checked-in value stale without failing CI, making the manifest misleading. Either remove this field or validate it against the setup pin during usage verification.
  "declaredVersion": "==2.0.16",

scripts/teams-api-drift/README.md:135

  • The repository has no offline tests for publication orchestration or registry metadata; the added tests cover extraction, analysis and report behavior, while publication remains inline workflow JavaScript. This claim overstates the verification provided and should be removed or replaced with the behavior actually tested.
Offline tests validate publication orchestration and registry metadata without
creating live comments or issues. On Windows, use a short environment path or

scripts/teams-api-drift/teams_api_drift/report.py:298

  • This validator only requires a bullet to contain a known finding ID; it does not enforce the prompt's required - **ID** — Advisory: ... form. A report such as - Fix this: TSAPI-0001 would pass and be published despite violating the report contract. Use an anchored allowed-pattern check while retaining the no-findings and No action exceptions.
                re.match(r"\s*(?:[-*+]|\d+[.)])\s", line)
                and not re.search(ID_PATTERN, line)
                and not re.match(r"\s*- No ", line, re.I)
                and not aggregate_no_action
            ):
  • Files reviewed: 27/28 changed files
  • Comments generated: 4
  • Review effort level: Lite

Comment thread .github/workflows/teams-api-drift-prs.yml Outdated
Comment thread .github/workflows/teams-api-drift-scheduled.yml Outdated
Comment thread scripts/teams-api-drift/teams_api_drift/compare.py
Comment thread scripts/teams-api-drift/teams_api_drift/report.py Outdated
Copilot AI review requested due to automatic review settings September 15, 2026 17:03
Comment thread .github/workflows/teams-api-drift-prs.yml Fixed
Comment thread .github/workflows/teams-api-drift-prs.yml Fixed
Comment thread .github/workflows/teams-api-drift-prs.yml Fixed
Comment thread .github/workflows/teams-api-drift-prs.yml Fixed
Comment thread .github/workflows/teams-api-drift-prs.yml Fixed
Comment thread .github/workflows/teams-api-drift-prs.yml Fixed
Comment thread .github/workflows/teams-api-drift-prs.yml Fixed
Comment thread .github/workflows/teams-api-drift-prs.yml Fixed
Comment thread .github/workflows/teams-api-drift-prs.yml Fixed
Comment thread .github/workflows/teams-api-drift-prs.yml Fixed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Critical workflow security concerns and unresolved workflow, classification, dependency determinism, and report-validation issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

scripts/teams-api-drift/teams_api_drift/report.py:275

  • The validator only checks that mandatory IDs occur somewhere in the entire report (referenced); it never verifies that an ID is represented by an action bullet in the section required by its classification/category. For example, a blocking ID can appear only in Summary while Compatibility breaks says no findings, and this still validates. Validate each included finding against its required section/action form and add a regression test for misplaced IDs.

libraries/microsoft-agents-hosting-msteams/config/teams-capabilities.yaml:19

  • These capability prefixes do not match the recorded/source API symbols for the channel and team models: the manifest imports microsoft_teams.api.models.channel_data.ChannelInfo and ...TeamInfo, while classification matches capability areas against extracted canonical names. As a result, changes to those models fall back to the broader activity-data capability instead of channels/teams, producing the wrong owner and adoption policy. Use prefixes that match the extracted symbol paths and add an ownership-classification test.
    upstreamAreas: [microsoft_teams.api.models.channel_data.channel_info, microsoft_teams.api.clients.team]

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

  • The extraction environments install only the requested microsoft-teams-api version and let pip resolve its transitive dependencies afresh. A later run can therefore extract a different historical API version because a compatible dependency (for example microsoft-teams-common) changed, and the diff can attribute that transitive change to Teams API drift; recording resolvedDependencies does not prevent it. Pin/lock the dependency closure or make the comparison account for differing resolved dependency inventories before calling the result deterministic.
            python,
            "-m",
            "pip",
            "install",
            "--disable-pip-version-check",

scripts/teams-api-drift/teams_api_drift/report.py:283

  • This loop validates only lines that already look like list items; any non-empty prose in these sections is ignored. As a result, an advisory can include an unattributed recommendation without a finding ID and still be marked valid and published, despite the prompt requiring every action item to use an allowed ID/no-findings form. Parse each non-blank item (including wrapped items) and reject content that does not satisfy the contract.
    for section in SECTIONS[1:-1]:
        for line in sections.get(section, "").splitlines():
            if not re.match(r"\s*(?:[-*+]|\d+[.)])\s", line):
                continue
  • Files reviewed: 27/28 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread .github/workflows/teams-api-drift-prs.yml Outdated
Comment thread .github/workflows/teams-api-drift-scheduled.yml
Copilot AI review requested due to automatic review settings September 15, 2026 20:31
@ceciliaavila
Cecilia Avila (ceciliaavila) force-pushed the southworks/update/pin-teams-api-version branch from afb5228 to 3c3cfae Compare September 15, 2026 20:36
@ceciliaavila
Cecilia Avila (ceciliaavila) force-pushed the southworks/add/teams-api-drift-detector branch from 7a5f9ac to b56555b Compare September 15, 2026 20:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved workflow security, report validation, and capability classification issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

scripts/teams-api-drift/teams_api_drift/report.py:293

  • The validator accepts any numeric aggregate here without checking it against the findings. The current test passes 147 even though findings() contains zero no-action findings, so a false omitted-count summary can be published as valid. Capture the count and require it to equal the number of no-action findings (the same count exposed in omittedNoActionFindingCount).

.github/workflows/teams-api-drift-prs.yml:200

  • The workflow computes steps.changed.outputs.value, but this condition does not use it. A dependency pin can therefore produce an identical normalized API model and still run Copilot; the final REQUIRE_ADVISORY gate then requires that unnecessary report, so a Copilot outage can fail an otherwise clean no-drift comparison. Mirror the scheduled workflow by gating both context preparation and the final advisory requirement on the comparison result.
        if: ${{ always() && steps.classify-usage-impact.outcome != 'skipped' && steps.render-deterministic-report.outcome == 'success' && (github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false)) }}

libraries/microsoft-agents-hosting-msteams/config/teams-capabilities.yaml:19

  • Capability matching in classify.py uses the extracted qualified symbol name with exact/prefix matching. The manifest records microsoft_teams.api.models.channel_data.ChannelInfo, so the lowercase ...channel_info area never matches; ChannelInfo changes fall back to the broader activity-data policy instead of this capability's review-new-members policy. Use the qualified ...ChannelInfo symbol here.
    upstreamAreas: [microsoft_teams.api.models.channel_data.channel_info, microsoft_teams.api.clients.team]

libraries/microsoft-agents-hosting-msteams/config/teams-capabilities.yaml:24

  • The teams area is also written as a lowercase module-like path, while the manifest's extracted symbol is microsoft_teams.api.models.channel_data.TeamInfo. Because matching is case-sensitive, TeamInfo changes fall back to activity-data and are classified with its strict policy rather than review-new-members. Use the qualified ...TeamInfo symbol.
    upstreamAreas: [microsoft_teams.api.models.channel_data.team_info]
  • Files reviewed: 27/28 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread .github/workflows/teams-api-drift-prs.yml
Comment thread scripts/teams-api-drift/teams_api_drift/report.py
Copilot AI review requested due to automatic review settings September 15, 2026 20:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Seven unresolved moderate findings affect workflow token safety, drift classification, and report validation.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

.github/workflows/teams-api-drift-prs.yml:96

  • This job checks out the pull-request merge ref and then executes PR-controlled Python, build, and test code, but it grants pull-requests: write and copilot-requests: write. A same-repository PR can modify the checked-out workflow/scripts/setup.py or build hooks to use or exfiltrate that write-capable token before the intended comment step. Keep analysis read-only and move Copilot/comment publication to trusted default-branch code that consumes only bounded artifacts, as the scheduled workflow does.
      pull-requests: write
      copilot-requests: write

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

  • optional == false is not sufficient to identify a construction-breaking field: extract.instance_properties marks every public self.attr assignment on ordinary classes as non-optional. Consequently, an additive ApiClient property in a constructed usage is classified as blocking and --fail-on-drift rejects a non-breaking upgrade. Restrict this branch to required fields on Pydantic/model symbols and classify ordinary class-property additions as review/non-breaking.
            elif kind == "property-added" and not change["after"]["optional"]:
                classification = "blocking"

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

  • This property branch never considers usage.exposure. A publicly exposed response model with no propertiesRead therefore falls through at line 120 for removals, renames, and type changes; for example, the manifest marks ConfigResponse as publicly exposed but records no field reads. Such upstream changes become no-action instead of affecting the SDK's public contract, so public/re-exported model usage must be handled here (with additive fields classified separately).
    if kind.startswith("property-"):
        if (change["symbol"], member) in nested_uses(usage, index):
            return True
        return (
            same

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

  • The compatibility rule treats every optional == false property addition as breaking, but the extractor uses that flag for ordinary class instance attributes as well as required Pydantic fields. A new ApiClient member does not invalidate existing constructor calls, yet this records it as breaking and feeds misleading evidence to classification. For additions, restrict the breaking case to required fields on model symbols; keep removals breaking.
                    (
                        "breaking"
                        if new_field is None or not new_field["optional"]
                        else "non-breaking"
                    ),

scripts/teams-api-drift/teams_api_drift/report.py:293

  • This accepts the aggregate count based only on its wording; it is never checked against the actual no-action findings. For example, - 999 additional changes require no action. would validate even when there are no such findings, allowing the published advisory to contain a false count. Parse and compare the count with the authoritative no-action count.
            aggregate_no_action = (
                section == "No action"
                and re.fullmatch(r"- \d+\b.*\brequires? no action\.", line, re.I)
                and not re.search(ID_PATTERN, line)
            )

scripts/teams-api-drift/teams_api_drift/report.py:205

  • Although the context is capped at 12,000 characters, read_text() loads the complete affected file before truncation. A large PR-controlled Python file can therefore consume unbounded memory and defeat the intended bounded-source contract. Read at most MAX_SOURCE_CHARACTERS + 1 characters and derive truncated from that bounded read.
        content = path.read_text(encoding="utf-8")
        original_length = len(content)
  • Files reviewed: 27/28 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread scripts/teams-api-drift/teams_api_drift/report.py Outdated
Copilot AI review requested due to automatic review settings September 16, 2026 15:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Four moderate findings remain unresolved, each with one vote.

Review details

Suppressed comments (4)

libraries/microsoft-agents-hosting-msteams/teams-api-usage-manifest.json:4

  • This duplicated version field is never read or validated, while prepare_context copies the entire manifest into the advisory context. After the first accepted Teams API upgrade, setup.py will move past 2.0.16 but this value will remain ==2.0.16, causing scheduled/PR reports to contain contradictory dependency metadata. Remove the field or make verify-usage validate it against the resolved setup pin and require it to be updated with the pin.
  "declaredVersion": "==2.0.16",

scripts/teams-api-drift/teams_api_drift/candidate.py:101

  • This is the critical installation path that builds the SDK wheels, installs them beside the exact candidate, and relies on the post-install version check; the new test suite does not exercise prepare_candidate_environment or _extension_dependencies. Add an isolated integration test with a temporary candidate environment (or mocked subprocesses) that verifies the candidate Teams API version is preserved and all non-Teams dependencies are installed, so regressions here cannot silently invalidate every later contract test.
        run(
            [
                candidate,
                "-m",
                "pip",
                "install",
                *[wheel for wheel in wheels if wheel != extension],
                *dependencies,
            ]
        )
        # Test the candidate without letting the extension's current pin replace it.
        run([candidate, "-m", "pip", "install", "--no-deps", extension])

scripts/teams-api-drift/teams_api_drift/report.py:205

  • When a dependency release produces many blocking/required findings, this path includes all of them without a bound; only review findings are capped. prepare_context then raises when the 60,000-character check is exceeded, and the same-repository workflows treat the context step as fatal, so a large drift loses the advisory/publication instead of receiving a bounded report. Compact mandatory entries while retaining their IDs, or explicitly fall back to deterministic publication when the bound is hit.
    mandatory, included_reviews, omitted_reviews = _advisory_scope(findings)
    included_findings = mandatory + included_reviews

scripts/teams-api-drift/teams_api_drift/report.py:306

  • The validator accepts any numeric aggregate in the No action section without checking it against the authoritative findings. An advisory such as - 999 additional changes require no action. therefore passes validation and can publish an incorrect count; parse the count and require it to equal the number of no-action findings, with a regression test for a mismatched count.
            aggregate_no_action = (
                section == "No action"
                and re.fullmatch(r"- \d+\b.*\brequires? no action\.", line, re.I)
                and not re.search(ID_PATTERN, line)
            )
  • Files reviewed: 27/28 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@ceciliaavila
Cecilia Avila (ceciliaavila) force-pushed the southworks/update/pin-teams-api-version branch from 3c3cfae to 39ff447 Compare September 24, 2026 13:55
@ceciliaavila
Cecilia Avila (ceciliaavila) removed this pull request from stack #583 September 24, 2026 14:43
Base automatically changed from southworks/update/pin-teams-api-version to main September 24, 2026 16:41
@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) merged commit e42eb36 into main Sep 24, 2026
10 checks passed
@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) deleted the southworks/add/teams-api-drift-detector branch September 24, 2026 16:59
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.

Implement Teams API Drift Detector workflow

4 participants