diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 2be45c2a8..7c2466a0a 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -13,6 +13,70 @@ on: branches: [ "main", "release/*" ] jobs: + teams-api-metadata-changes: + name: Detect MSTeams metadata-relevant changes + if: ${{ github.event_name == 'pull_request' }} + runs-on: ubuntu-latest + outputs: + changed: ${{ steps.changed.outputs.changed }} + base_ref: ${{ steps.changed.outputs.base_ref }} + steps: + - name: Checkout repository with full history + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + - id: changed + name: Check changed paths + env: + PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PUSH_BASE_SHA: ${{ github.event.before }} + shell: bash + run: | + base_ref="$PR_BASE_SHA" + if [[ -z "$base_ref" ]]; then + base_ref="$PUSH_BASE_SHA" + fi + if [[ -z "$base_ref" || "$base_ref" == "0000000000000000000000000000000000000000" ]]; then + echo "changed=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + if git diff --quiet "$base_ref" "$GITHUB_SHA" -- \ + libraries/microsoft-agents-hosting-msteams/microsoft_agents/hosting/msteams \ + libraries/microsoft-agents-hosting-msteams/setup.py \ + libraries/microsoft-agents-hosting-msteams/teams-api-usage-manifest.json \ + libraries/microsoft-agents-hosting-msteams/config/teams-capabilities.yaml; then + echo "changed=false" >> "$GITHUB_OUTPUT" + else + echo "changed=true" >> "$GITHUB_OUTPUT" + fi + echo "base_ref=$base_ref" >> "$GITHUB_OUTPUT" + + teams-api-metadata: + name: Validate Teams API metadata accuracy + needs: teams-api-metadata-changes + if: ${{ needs.teams-api-metadata-changes.outputs.changed == 'true' }} + runs-on: ubuntu-latest + steps: + - name: Checkout repository with full history + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + - name: Set up Python 3.12 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.12" + cache: pip + cache-dependency-path: scripts/teams-api-drift/requirements.txt + - name: Install Teams API drift tooling + run: python -m pip install -r scripts/teams-api-drift/requirements.txt + - name: Validate usage and capability metadata + env: + BASE_REF: ${{ needs.teams-api-metadata-changes.outputs.base_ref }} + run: >- + python scripts/teams-api-drift/teams-api-drift.py verify-usage + --base-ref "$BASE_REF" + build: runs-on: ubuntu-latest @@ -80,4 +144,4 @@ jobs: python -m pip install ./dist/microsoft_agents_testing*.whl - name: Test with pytest run: | - pytest -W "ignore:SelectableGroups dict interface is deprecated. Use select.:DeprecationWarning" \ No newline at end of file + pytest -W "ignore:SelectableGroups dict interface is deprecated. Use select.:DeprecationWarning" diff --git a/libraries/microsoft-agents-hosting-msteams/config/teams-capabilities.yaml b/libraries/microsoft-agents-hosting-msteams/config/teams-capabilities.yaml index fd573fff4..850b90abf 100644 --- a/libraries/microsoft-agents-hosting-msteams/config/teams-capabilities.yaml +++ b/libraries/microsoft-agents-hosting-msteams/config/teams-capabilities.yaml @@ -1,11 +1,13 @@ # Curated ownership map. Direct compatibility always takes precedence over adoption. +# For capability maintenance and sourceReview acknowledgments, see +# scripts/teams-api-drift/README.md#source-review-acknowledgments. schemaVersion: 1 dependency: package: microsoft-teams-api capabilities: teams-api-client: description: Creates, caches and exposes the Teams API client for a turn. - owners: [microsoft_agents/hosting/msteams/_teams_api_client.py, microsoft_agents/hosting/msteams/teams_turn_context.py] + owners: [microsoft_agents/hosting/msteams/_teams_api_client.py, microsoft_agents/hosting/msteams/teams_agent_extension.py, microsoft_agents/hosting/msteams/teams_turn_context.py] upstreamAreas: [microsoft_teams.api.clients] adoptionPolicy: strict-compatibility activity-data: @@ -25,7 +27,7 @@ capabilities: adoptionPolicy: review-new-members meetings: description: Routes meeting lifecycle and participant events. - owners: [microsoft_agents/hosting/msteams/meeting] + owners: [microsoft_agents/hosting/msteams/meeting, microsoft_agents/hosting/msteams/teams_activity.py] upstreamAreas: [microsoft_teams.api.models.meetings, microsoft_teams.api.clients.meeting] adoptionPolicy: review-new-members message-extensions: diff --git a/scripts/teams-api-drift/README.md b/scripts/teams-api-drift/README.md index b0c514792..9301452c9 100644 --- a/scripts/teams-api-drift/README.md +++ b/scripts/teams-api-drift/README.md @@ -80,14 +80,53 @@ The adjacent `config/teams-capabilities.yaml` maps upstream module areas to feature owners and adoption policies. It supports strict compatibility, review-new-members and advisory-only policies. It does not authorize adoption. +### Source review acknowledgments + +The main Python CI workflow checks extension source changes against both metadata +documents. +Update `teams-api-usage-manifest.json` when a changed file adds, removes, or +changes Teams API imports, calls, model fields, construction, validation, or +public exposure. Update `config/teams-capabilities.yaml` when files are added, +removed, moved between features, or change the upstream areas owned by a feature. + +Some source edits do not affect either document. For a usage-related edit that +has been reviewed and needs no usage metadata change, add or change this property +in `teams-api-usage-manifest.json`: + +```json +"sourceReview": { + "outcome": "no-usage-metadata-change", + "reason": "Explain specifically why the changed source preserves recorded usage." +} +``` + +For a structural or capability-related edit that needs no capability metadata +change, add or change this property in `config/teams-capabilities.yaml`: + +```yaml +sourceReview: + outcome: no-capability-metadata-change + reason: Explain specifically why capability ownership and upstream areas remain accurate. +``` + +The acknowledgment must differ from the base branch, include a non-empty reason, +and be committed with or after the source change. A previous acknowledgment cannot +silently approve later edits. Run the same review locally with: + +```bash +python scripts/teams-api-drift/teams-api-drift.py verify-usage --base-ref main +``` + ## Workflows -PRs to `main` or `release/*` run dependency analysis only when the exact Teams -version pin in `setup.py` changes. The base branch pin is the baseline and the PR -pin is the candidate. Manual PR-workflow dispatch takes explicit `from` and `to` -versions. The weekly workflow runs Monday at 08:00 UTC and compares the current -pin (currently 2.0.16) with the latest stable release, including future major -versions. Once maintainers approve an upgrade, changing the pin establishes the +The Python package workflow validates usage and capability metadata whenever the +MSTeams source or either metadata document changes. The Teams API drift PR +workflow still runs only when the exact Teams version pin in `setup.py` changes. +The base branch pin is the baseline and the PR pin is the candidate. Manual +PR-workflow dispatch takes explicit `from` and `to` versions. The weekly workflow +runs Monday at 08:00 UTC and compares the current pin (currently 2.0.16) with the +latest stable release, including future major versions. +Once maintainers approve an upgrade, changing the pin establishes the new baseline. When the resolved versions are identical, manual and scheduled runs finish successfully after version resolution; comparison, tests, reports, AI and publication are skipped. diff --git a/scripts/teams-api-drift/teams_api_drift/cli.py b/scripts/teams-api-drift/teams_api_drift/cli.py index a5b51af87..63f669c63 100644 --- a/scripts/teams-api-drift/teams_api_drift/cli.py +++ b/scripts/teams-api-drift/teams_api_drift/cli.py @@ -22,6 +22,7 @@ write_text, ) from .compare import compare_versions +from .metadata import validate_teams_api_metadata from .report import prepare_context, render_report, validate_agent_report from .resolve import resolve_versions @@ -74,6 +75,10 @@ def main(command=None, argv=None): elif command == "verify-usage": parser.add_argument("--manifest", "-m", type=Path, default=MANIFEST) parser.add_argument("--capabilities", type=Path, default=CAPABILITIES) + parser.add_argument( + "--base-ref", + help="Git ref used to verify metadata review for changed source files", + ) elif command == "prepare-candidate": parser.add_argument("--version", required=True) parser.add_argument("--environment", type=Path, required=True) @@ -144,8 +149,9 @@ def main(command=None, argv=None): write_json(destination(args.output, "resolved-versions.json"), result) print(json.dumps(result)) elif command == "verify-usage": - manifest = validate_manifest(read_json(args.manifest)) - read_capabilities(args.capabilities) + manifest = validate_teams_api_metadata( + args.manifest, args.capabilities, base_ref=args.base_ref + ) print(f"Verified {len(manifest['usages'])} Teams API usages") elif command == "prepare-candidate": result = prepare_candidate_environment( diff --git a/scripts/teams-api-drift/teams_api_drift/metadata.py b/scripts/teams-api-drift/teams_api_drift/metadata.py new file mode 100644 index 000000000..411eaefc7 --- /dev/null +++ b/scripts/teams-api-drift/teams_api_drift/metadata.py @@ -0,0 +1,693 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +"""Validate Teams API usage and capability metadata against extension source.""" + +import ast +from dataclasses import dataclass +import fnmatch +import json +from pathlib import Path +import subprocess + +import yaml + +from .common import ( + CAPABILITIES, + DEPENDENCY, + MANIFEST, + PACKAGE_ROOT, + ROOT, + declared_requirement, +) + +SOURCE_REVIEW_GUIDE = "scripts/teams-api-drift/README.md#source-review-acknowledgments" +USAGE_REVIEW_OUTCOME = "no-usage-metadata-change" +CAPABILITY_REVIEW_OUTCOME = "no-capability-metadata-change" + + +@dataclass(frozen=True) +class TeamsImport: + symbol: str + file: str + line: int + + +def _run_git(root, *args, check=True): + result = subprocess.run( + ["git", *args], + cwd=root, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + if check and result.returncode: + raise ValueError(result.stderr.strip() or f"git {' '.join(args)} failed") + return result.stdout if result.returncode == 0 else None + + +def _git_files(root, *args): + value = _run_git(root, *args) or "" + return {item.replace("\\", "/") for item in value.split("\0") if item} + + +def _git_file(root, revision, file): + return _run_git(root, "show", f"{revision}:{file}", check=False) + + +def _imports_from_text(file, text): + imports = [] + for node in ast.walk(ast.parse(text, filename=file)): + if ( + isinstance(node, ast.ImportFrom) + and node.module + and node.module.startswith("microsoft_teams.api") + ): + for name in node.names: + imports.append( + TeamsImport(f"{node.module}.{name.name}", file, node.lineno) + ) + elif isinstance(node, ast.Import): + for name in node.names: + if name.name.startswith("microsoft_teams.api"): + imports.append(TeamsImport(name.name, file, node.lineno)) + return imports + + +def _source_files(package_root, source_root): + return sorted( + file.relative_to(package_root).as_posix() for file in source_root.rglob("*.py") + ) + + +def _collect_imports(package_root, files): + return [ + imported + for file in files + for imported in _imports_from_text( + file, (package_root / file).read_text(encoding="utf-8") + ) + ] + + +def _annotation_symbol(node, aliases): + if isinstance(node, ast.Name): + return aliases.get(node.id) + if isinstance(node, ast.Subscript): + return _annotation_symbol(node.slice, aliases) + if isinstance(node, ast.BinOp) and isinstance(node.op, ast.BitOr): + return _annotation_symbol(node.left, aliases) or _annotation_symbol( + node.right, aliases + ) + return None + + +class _UsageVisitor(ast.NodeVisitor): + def __init__(self, aliases): + self.aliases = aliases + self.variables = {} + self.members = [] + self.public_types = set() + self.public_class_depth = 0 + + def visit_ClassDef(self, node): + public = not node.name.startswith("_") + if public: + self.public_class_depth += 1 + self.generic_visit(node) + if public: + self.public_class_depth -= 1 + + def visit_FunctionDef(self, node): + previous = self.variables.copy() + arguments = [*node.args.posonlyargs, *node.args.args, *node.args.kwonlyargs] + if node.args.vararg: + arguments.append(node.args.vararg) + if node.args.kwarg: + arguments.append(node.args.kwarg) + for argument in arguments: + symbol = _annotation_symbol(argument.annotation, self.aliases) + if symbol: + self.variables[argument.arg] = symbol + if self.public_class_depth or not node.name.startswith("_"): + self.public_types.add(symbol) + return_symbol = _annotation_symbol(node.returns, self.aliases) + if return_symbol and (self.public_class_depth or not node.name.startswith("_")): + self.public_types.add(return_symbol) + self.generic_visit(node) + self.variables = previous + + visit_AsyncFunctionDef = visit_FunctionDef + + 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) + if ( + isinstance(value.func, ast.Attribute) + and isinstance(value.func.value, ast.Name) + and value.func.attr in ("model_validate", "parse_obj") + ): + return self.aliases.get(value.func.value.id) + return None + + def visit_Assign(self, node): + symbol = self._value_symbol(node.value) + if symbol: + for target in node.targets: + if isinstance(target, ast.Name): + self.variables[target.id] = symbol + self.generic_visit(node) + + def visit_AnnAssign(self, node): + symbol = _annotation_symbol( + node.annotation, self.aliases + ) or self._value_symbol(node.value) + if symbol and isinstance(node.target, ast.Name): + self.variables[node.target.id] = symbol + self.generic_visit(node) + + def visit_Attribute(self, node): + segments = [node.attr] + root = node.value + while isinstance(root, ast.Attribute): + segments.insert(0, root.attr) + root = root.value + if isinstance(root, ast.Name): + symbol = self.variables.get(root.id) or self.aliases.get(root.id) + if symbol: + parent = getattr(node, "_teams_parent", None) + kind = ( + "method" + if isinstance(parent, ast.Call) and parent.func is node + else "write" if isinstance(node.ctx, ast.Store) else "read" + ) + self.members.append((symbol, ".".join(segments), kind, node.lineno)) + self.generic_visit(node) + + +def _static_usage(file, text): + tree = ast.parse(text, filename=file) + aliases = {} + for node in ast.walk(tree): + for child in ast.iter_child_nodes(node): + child._teams_parent = node + if ( + 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) + visitor.visit(tree) + return visitor.members, visitor.public_types + + +def _validate_static_usage(package_root, files, manifest, errors): + for file in files: + members, public_types = _static_usage( + file, (package_root / file).read_text(encoding="utf-8") + ) + usages = [ + usage for usage in manifest["usages"] if file in usage.get("files", []) + ] + for symbol in public_types: + matching = [ + usage for usage in usages if usage.get("upstreamSymbol") == symbol + ] + if matching and not any( + usage.get("exposure") in ("publicly-exposed", "re-exported") + for usage in matching + ): + errors.append( + f"{symbol} appears in a public annotation in {file} but is not " + "marked publicly exposed" + ) + reported = set() + for symbol, member, kind, line in members: + matching = [ + usage for usage in usages if usage.get("upstreamSymbol") == symbol + ] + if not matching: + continue + field = { + "method": "methodsCalled", + "write": "propertiesWritten", + "read": "propertiesRead", + }[kind] + declared = {value for usage in matching for value in usage.get(field, [])} + covered = any( + value == member + or value.replace("[]", "").split(".")[0] == member.split(".")[0] + for value in declared + ) + key = (symbol, member, kind) + if not covered and key not in reported: + reported.add(key) + 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}" + ) + + +def _matches_owner(owner, file): + owner = owner.replace("\\", "/").rstrip("/") + file = file.replace("\\", "/") + if "*" in owner: + return fnmatch.fnmatchcase(file, owner) + return file == owner or file.startswith(owner + "/") + + +def _owners_for(document, file): + return sorted( + name + for name, capability in document.get("capabilities", {}).items() + if any(_matches_owner(owner, file) for owner in capability.get("owners", [])) + ) + + +def _owner_patterns_for(document, file): + return sorted( + owner + for capability in document.get("capabilities", {}).values() + for owner in capability.get("owners", []) + if _matches_owner(owner, file) + ) + + +def _upstream_areas(symbol): + if symbol == "microsoft_teams.api.ApiClient": + return ["microsoft_teams.api.clients"] + exact = { + "microsoft_teams.api.models.channel_data.ChannelInfo": ( + "microsoft_teams.api.models.channel_data.channel_info" + ), + "microsoft_teams.api.models.channel_data.TeamInfo": ( + "microsoft_teams.api.models.channel_data.team_info" + ), + } + if symbol in exact: + return [exact[symbol]] + prefixes = { + "microsoft_teams.api.models.ChannelData": "models.channel_data", + "microsoft_teams.api.models.ChannelInfo": "models.channel_data.channel_info", + "microsoft_teams.api.models.TeamInfo": "models.channel_data.team_info", + "microsoft_teams.api.models.NotificationInfo": "models.channel_data", + "microsoft_teams.api.models.FeedbackLoop": "models.channel_data", + "microsoft_teams.api.models.MeetingInfo": "models.meetings", + "microsoft_teams.api.models.FileConsent": "models.file", + "microsoft_teams.api.models.MessagingExtension": "models.messaging_extension", + "microsoft_teams.api.models.TaskModule": "models.task_module", + "microsoft_teams.api.models.AppBasedLinkQuery": "models.app_based_link_query", + } + for prefix, area in prefixes.items(): + if symbol.startswith(prefix): + return [f"microsoft_teams.api.{area}"] + module = symbol.rsplit(".", 1)[0] + return [module] if module != "microsoft_teams.api" else [] + + +def _valid_review(review, outcome): + return ( + isinstance(review, dict) + and review.get("outcome") == outcome + and isinstance(review.get("reason"), str) + and bool(review["reason"].strip()) + ) + + +def _review_changed(before, after, outcome): + return before != after and _valid_review(after, outcome) + + +def _latest_commit(root, base, files): + if not files: + return None + value = _run_git(root, "log", "-1", "--format=%H", f"{base}..HEAD", "--", *files) + return value.strip() or None + + +def _metadata_is_fresh(root, base, working, source_files, metadata_file): + if any(file in working for file in source_files): + return metadata_file in working + if metadata_file in working: + return True + source_commit = _latest_commit(root, base, source_files) + metadata_commit = _latest_commit(root, base, [metadata_file]) + if not source_commit or not metadata_commit: + return False + return ( + source_commit == metadata_commit + or _run_git( + root, + "merge-base", + "--is-ancestor", + source_commit, + metadata_commit, + check=False, + ) + is not None + ) + + +def _read_base_document(root, base, file, loader): + text = _git_file(root, base, file) + if text is None: + return None + try: + return loader(text) + except (ValueError, yaml.YAMLError): + return None + + +def _capability_change_addressed(change, before, after): + file = change["file"] + if not change["baseExists"] and change["currentExists"]: + return any( + pattern not in _owner_patterns_for(before, file) + for pattern in _owner_patterns_for(after, file) + ) + if change["baseExists"] and not change["currentExists"]: + return any( + pattern not in _owner_patterns_for(after, file) + for pattern in _owner_patterns_for(before, file) + ) + if change["baseOwners"] != change["currentOwners"]: + return True + if not change["importChanged"]: + return False + return any( + before.get("capabilities", {}).get(name, {}).get("owners") + != after.get("capabilities", {}).get(name, {}).get("owners") + or before.get("capabilities", {}).get(name, {}).get("upstreamAreas") + != after.get("capabilities", {}).get(name, {}).get("upstreamAreas") + for name in set(change["baseOwners"] + change["currentOwners"]) + ) + + +def _validate_change_reviews( + root, + package_root, + base_ref, + manifest, + capabilities, + current_imports, + errors, + manifest_path, + capabilities_path, +): + base = (_run_git(root, "merge-base", "HEAD", base_ref, check=False) or "").strip() + if not base: + raise ValueError(f"Unable to resolve Git base ref {base_ref}") + package_path = package_root.relative_to(root).as_posix() + manifest_file = manifest_path.relative_to(root).as_posix() + capabilities_file = capabilities_path.relative_to(root).as_posix() + base_manifest = _read_base_document(root, base, manifest_file, json.loads) + base_capabilities = _read_base_document( + root, base, capabilities_file, yaml.safe_load + ) + if not base_manifest or not base_capabilities: + return + + committed = _git_files( + root, "diff", "--name-only", "--no-renames", "-z", base, "HEAD" + ) + working = _git_files(root, "diff", "--name-only", "--no-renames", "-z", "HEAD") + working |= _git_files(root, "ls-files", "--others", "--exclude-standard", "-z") + changed = committed | working + source_prefix = f"{package_path}/{manifest['sourceRoot'].rstrip('/')}/" + changed_source = sorted( + file[len(package_path) + 1 :] + for file in changed + if file.startswith(source_prefix) and file.endswith(".py") + ) + if not changed_source: + return + + current_usage_files = { + file for usage in manifest["usages"] for file in usage.get("files", []) + } + base_usage_files = { + file + for usage in base_manifest.get("usages", []) + for file in usage.get("files", []) + } + current_by_file = {} + for imported in current_imports: + current_by_file.setdefault(imported.file, set()).add(imported.symbol) + base_by_file = {} + for file in changed_source: + text = _git_file(root, base, f"{package_path}/{file}") + if text is not None: + base_by_file[file] = { + imported.symbol for imported in _imports_from_text(file, text) + } + usage_relevant = [ + file + for file in changed_source + if file in current_usage_files + or file in base_usage_files + or current_by_file.get(file) + or base_by_file.get(file) + ] + usage_sources = [f"{package_path}/{file}" for file in usage_relevant] + usage_fresh = _metadata_is_fresh(root, base, working, usage_sources, manifest_file) + explicit_usage_review = ( + _review_changed( + base_manifest.get("sourceReview"), + manifest.get("sourceReview"), + USAGE_REVIEW_OUTCOME, + ) + and usage_fresh + ) + removed_recorded = sorted( + f"{symbol} in {file}" + for file in changed_source + for symbol in base_by_file.get(file, set()) - current_by_file.get(file, set()) + if any( + usage.get("upstreamSymbol") == symbol and file in usage.get("files", []) + for usage in manifest["usages"] + ) + ) + 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}" + ) + elif usage_relevant and not ( + (base_manifest.get("usages") != manifest.get("usages") and usage_fresh) + or explicit_usage_review + ): + errors.append( + f"{len(usage_relevant)} Teams API usage-related source file(s) changed " + "without a fresh usage-manifest update or non-impact review; see " + f"{SOURCE_REVIEW_GUIDE}" + ) + + capability_changes = [] + for file in changed_source: + current_exists = (package_root / file).is_file() + base_exists = _git_file(root, base, f"{package_path}/{file}") is not None + current_owners = _owners_for(capabilities, file) if current_exists else [] + base_owners = _owners_for(base_capabilities, file) if base_exists else [] + import_changed = bool(current_owners or base_owners) and ( + sorted(current_by_file.get(file, set())) + != sorted(base_by_file.get(file, set())) + ) + if ( + current_exists != base_exists + or current_owners != base_owners + or import_changed + ): + capability_changes.append( + { + "file": file, + "currentExists": current_exists, + "baseExists": base_exists, + "currentOwners": current_owners, + "baseOwners": base_owners, + "importChanged": import_changed, + } + ) + capability_sources = [ + f"{package_path}/{change['file']}" for change in capability_changes + ] + capability_fresh = _metadata_is_fresh( + root, base, working, capability_sources, capabilities_file + ) + explicit_capability_review = ( + _review_changed( + base_capabilities.get("sourceReview"), + capabilities.get("sourceReview"), + CAPABILITY_REVIEW_OUTCOME, + ) + and capability_fresh + ) + targeted_update = capability_fresh and all( + _capability_change_addressed(change, base_capabilities, capabilities) + for change in capability_changes + ) + if capability_changes and not explicit_capability_review and not targeted_update: + errors.append( + f"{len(capability_changes)} capability ownership or upstream-area source " + "change(s) lack a fresh targeted capability update or non-impact review; " + f"see {SOURCE_REVIEW_GUIDE}" + ) + + +def validate_teams_api_metadata( + manifest_path=MANIFEST, + capabilities_path=CAPABILITIES, + package_root=PACKAGE_ROOT, + repo_root=ROOT, + base_ref=None, +): + """Validate current metadata and, when requested, its review against Git.""" + manifest_path = Path(manifest_path).resolve() + capabilities_path = Path(capabilities_path).resolve() + package_root = Path(package_root).resolve() + repo_root = Path(repo_root).resolve() + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + capabilities = yaml.safe_load(capabilities_path.read_text(encoding="utf-8")) + errors = [] + + if manifest.get("schemaVersion") != 1 or manifest.get("dependency") != DEPENDENCY: + errors.append( + "Usage manifest must be a schemaVersion 1 document for the dependency" + ) + if not isinstance(manifest.get("usages"), list): + errors.append("Usage manifest must contain a usages array") + if ( + not isinstance(capabilities, dict) + or capabilities.get("schemaVersion") != 1 + or capabilities.get("dependency", {}).get("package") != DEPENDENCY + or not isinstance(capabilities.get("capabilities"), dict) + ): + errors.append("Capabilities must be a schemaVersion 1 map for the dependency") + if errors: + raise ValueError("; ".join(errors)) + + setup_path = package_root / "setup.py" + if setup_path.is_file(): + requirement = declared_requirement(setup_path.read_text(encoding="utf-8")) + if manifest.get("declaredVersion") != str(requirement.specifier): + errors.append( + "Usage manifest declaredVersion must match setup.py " + f"({requirement.specifier})" + ) + + for document, outcome, name in ( + (manifest, USAGE_REVIEW_OUTCOME, "usage manifest"), + (capabilities, CAPABILITY_REVIEW_OUTCOME, "capabilities"), + ): + if "sourceReview" in document and not _valid_review( + document["sourceReview"], outcome + ): + errors.append( + f'{name} sourceReview must use outcome "{outcome}" and include ' + f"a non-empty reason; see {SOURCE_REVIEW_GUIDE}" + ) + + source_root = (package_root / manifest.get("sourceRoot", "")).resolve() + if not source_root.is_relative_to(package_root) or not source_root.is_dir(): + errors.append("Usage manifest sourceRoot is missing or unsafe") + source_files = [] + else: + source_files = _source_files(package_root, source_root) + source_set = set(source_files) + represented = set() + for usage in manifest["usages"]: + symbol = usage.get("upstreamSymbol") if isinstance(usage, dict) else None + files = usage.get("files") if isinstance(usage, dict) else None + if ( + not isinstance(symbol, str) + or not isinstance(usage.get("usage"), str) + or not isinstance(files, list) + ): + errors.append("Every usage must include upstreamSymbol, usage, and files") + continue + for file in files: + if not isinstance(file, str) or file not in source_set: + errors.append( + f"Usage {symbol} references missing or unsafe file {file!r}" + ) + else: + represented.add((symbol, file)) + imports = _collect_imports(package_root, source_files) + for imported in imports: + if (imported.symbol, imported.file) not in represented: + errors.append( + f"Direct Teams API import {imported.symbol} in {imported.file}:{imported.line} " + "is absent from the usage manifest" + ) + + for name, capability in capabilities["capabilities"].items(): + policy = ( + capability.get("adoptionPolicy") if isinstance(capability, dict) else None + ) + owners = capability.get("owners") if isinstance(capability, dict) else None + areas = ( + capability.get("upstreamAreas") if isinstance(capability, dict) else None + ) + if ( + policy not in ("strict-compatibility", "review-new-members", "advisory-only") + or not isinstance(owners, list) + or not isinstance(areas, list) + ): + errors.append(f"Capability {name} must include owners and upstreamAreas") + continue + for owner in owners: + if not isinstance(owner, str) or not any( + _matches_owner(owner, file) for file in source_files + ): + errors.append( + f"Capability {name} owner {owner!r} matches no source files" + ) + for imported in imports: + areas = _upstream_areas(imported.symbol) + if not areas: + continue + owners = _owners_for(capabilities, imported.file) + if not owners: + errors.append( + f"{imported.file} imports {imported.symbol} but has no capability owner" + ) + continue + owned_areas = [ + area + for owner in owners + for area in capabilities["capabilities"][owner]["upstreamAreas"] + ] + if not any( + candidate == area or candidate.startswith(area + ".") + for candidate in areas + for area in owned_areas + ): + errors.append( + f"{imported.symbol} maps to {', '.join(areas)}, absent from " + f"capability owners {', '.join(owners)} for {imported.file}" + ) + + _validate_static_usage(package_root, source_files, manifest, errors) + + if base_ref: + _validate_change_reviews( + repo_root, + package_root, + base_ref, + manifest, + capabilities, + imports, + errors, + manifest_path, + capabilities_path, + ) + if errors: + raise ValueError( + "Teams API metadata validation failed:\n- " + "\n- ".join(errors) + ) + return manifest diff --git a/tests/teams_api_drift/test_metadata.py b/tests/teams_api_drift/test_metadata.py new file mode 100644 index 000000000..0de4bffc5 --- /dev/null +++ b/tests/teams_api_drift/test_metadata.py @@ -0,0 +1,217 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +import json +from pathlib import Path +import subprocess + +import pytest +import yaml + +from teams_api_drift.metadata import validate_teams_api_metadata + + +def _git(root, *args): + subprocess.run(["git", *args], cwd=root, check=True, capture_output=True) + + +@pytest.fixture +def metadata_repo(tmp_path): + package = tmp_path / "libraries/microsoft-agents-hosting-msteams" + source = package / "microsoft_agents/hosting/msteams" + source.mkdir(parents=True) + (source / "activity.py").write_text( + "from microsoft_teams.api.models.channel_data import ChannelData\n" + "\n" + "def event_type(data: ChannelData):\n" + " return data.event_type\n", + encoding="utf-8", + ) + (package / "setup.py").write_text( + "from setuptools import setup\n" + "setup(install_requires=['microsoft-teams-api==2.0.16'])\n", + encoding="utf-8", + ) + manifest_path = package / "teams-api-usage-manifest.json" + manifest = { + "schemaVersion": 1, + "dependency": "microsoft-teams-api", + "declaredVersion": "==2.0.16", + "sourceRoot": "microsoft_agents/hosting/msteams", + "usages": [ + { + "upstreamSymbol": ( + "microsoft_teams.api.models.channel_data.ChannelData" + ), + "usage": "parsed-model", + "exposure": "publicly-exposed", + "propertiesRead": ["event_type"], + "files": ["microsoft_agents/hosting/msteams/activity.py"], + } + ], + } + manifest_path.write_text(json.dumps(manifest, indent=2) + "\n", encoding="utf-8") + capabilities_path = package / "config/teams-capabilities.yaml" + capabilities_path.parent.mkdir() + capabilities = { + "schemaVersion": 1, + "dependency": {"package": "microsoft-teams-api"}, + "capabilities": { + "activity-data": { + "owners": ["microsoft_agents/hosting/msteams"], + "upstreamAreas": ["microsoft_teams.api.models.channel_data"], + "adoptionPolicy": "strict-compatibility", + } + }, + } + capabilities_path.write_text(yaml.safe_dump(capabilities), encoding="utf-8") + _git(tmp_path, "init", "-b", "main") + _git(tmp_path, "config", "user.email", "metadata@example.test") + _git(tmp_path, "config", "user.name", "Metadata Test") + _git(tmp_path, "add", ".") + _git(tmp_path, "commit", "-m", "baseline") + return { + "root": tmp_path, + "package": package, + "source": source, + "manifest_path": manifest_path, + "manifest": manifest, + "capabilities_path": capabilities_path, + "capabilities": capabilities, + } + + +def _validate(repo, base_ref=None): + return validate_teams_api_metadata( + repo["manifest_path"], + repo["capabilities_path"], + repo["package"], + repo["root"], + base_ref, + ) + + +def _write_json(path, document): + path.write_text(json.dumps(document, indent=2) + "\n", encoding="utf-8") + + +def test_accepts_aligned_current_metadata(metadata_repo): + assert len(_validate(metadata_repo)["usages"]) == 1 + + +def test_rejects_unrecorded_import_and_stale_dependency(metadata_repo): + metadata_repo["manifest"]["declaredVersion"] = "==2.0.15" + _write_json(metadata_repo["manifest_path"], metadata_repo["manifest"]) + (metadata_repo["source"] / "extra.py").write_text( + "from microsoft_teams.api import ApiClient\n", encoding="utf-8" + ) + + with pytest.raises(ValueError) as error: + _validate(metadata_repo) + + assert "declaredVersion must match setup.py" in str(error.value) + assert "ApiClient" in str(error.value) + + +def test_rejects_unrecorded_static_member_and_public_exposure(metadata_repo): + usage = metadata_repo["manifest"]["usages"][0] + usage.pop("propertiesRead") + usage.pop("exposure") + _write_json(metadata_repo["manifest_path"], metadata_repo["manifest"]) + + with pytest.raises(ValueError) as error: + _validate(metadata_repo) + + assert "event_type is statically read" in str(error.value) + assert "not marked publicly exposed" in str(error.value) + + +def test_public_protocol_dunder_annotation_requires_public_exposure(metadata_repo): + activity = metadata_repo["source"] / "activity.py" + activity.write_text( + "from typing import Protocol\n" + "from microsoft_teams.api.models.channel_data import ChannelData\n" + "\n" + "class Handler(Protocol):\n" + " def __call__(self, data: ChannelData) -> None: ...\n", + encoding="utf-8", + ) + metadata_repo["manifest"]["usages"][0]["exposure"] = "internal-only" + metadata_repo["manifest"]["usages"][0]["propertiesRead"] = [] + _write_json(metadata_repo["manifest_path"], metadata_repo["manifest"]) + + with pytest.raises(ValueError, match="not marked publicly exposed"): + _validate(metadata_repo) + + +def test_rejects_unrecorded_static_method_call(metadata_repo): + activity = metadata_repo["source"] / "activity.py" + activity.write_text( + activity.read_text() + + "\ndef parse(payload):\n" + + " return ChannelData.model_validate(payload)\n", + encoding="utf-8", + ) + + with pytest.raises(ValueError, match="model_validate is statically called"): + _validate(metadata_repo) + + +def test_source_change_requires_usage_review(metadata_repo): + activity = metadata_repo["source"] / "activity.py" + activity.write_text(activity.read_text() + "# implementation refactor\n") + + with pytest.raises(ValueError, match="usage-related source file"): + _validate(metadata_repo, "main") + + metadata_repo["manifest"]["sourceReview"] = { + "outcome": "no-usage-metadata-change", + "reason": "The refactor does not change Teams API consumption.", + } + _write_json(metadata_repo["manifest_path"], metadata_repo["manifest"]) + _validate(metadata_repo, "main") + + +def test_previous_review_does_not_approve_later_source_change(metadata_repo): + activity = metadata_repo["source"] / "activity.py" + activity.write_text(activity.read_text() + "# first refactor\n") + metadata_repo["manifest"]["sourceReview"] = { + "outcome": "no-usage-metadata-change", + "reason": "The first refactor preserves Teams API usage.", + } + _write_json(metadata_repo["manifest_path"], metadata_repo["manifest"]) + _git(metadata_repo["root"], "add", ".") + _git(metadata_repo["root"], "commit", "-m", "review first change") + activity.write_text(activity.read_text() + "# later refactor\n") + + with pytest.raises(ValueError, match="fresh usage-manifest update"): + _validate(metadata_repo, "main") + + +def test_new_source_file_requires_capability_review(metadata_repo): + (metadata_repo["source"] / "new_feature.py").write_text( + "ENABLED = True\n", encoding="utf-8" + ) + + with pytest.raises(ValueError, match="capability ownership"): + _validate(metadata_repo, "main") + + metadata_repo["capabilities"]["sourceReview"] = { + "outcome": "no-capability-metadata-change", + "reason": "The helper remains owned by the existing activity capability.", + } + metadata_repo["capabilities_path"].write_text( + yaml.safe_dump(metadata_repo["capabilities"]), encoding="utf-8" + ) + _validate(metadata_repo, "main") + + +def test_invalid_review_reason_fails_current_validation(metadata_repo): + metadata_repo["manifest"]["sourceReview"] = { + "outcome": "no-usage-metadata-change", + "reason": "", + } + _write_json(metadata_repo["manifest_path"], metadata_repo["manifest"]) + + with pytest.raises(ValueError, match="non-empty reason"): + _validate(metadata_repo)