Add cascading policy repo discovery with ADO support#1830
Conversation
Introduce cascading policy repo discovery (.github > .apm > _apm) with host profiles so Azure DevOps organisations can use APM governance. ADO forbids repo names starting/ending with a period, so a dedicated ADO profile tries only _apm (project + repo). - Add _policy_repo_candidates() with default and ado host profiles - Rewrite _auto_discover() to loop over profile candidates - Add _fetch_from_ado_repo() and _fetch_ado_contents() for ADO Items API - Fail-closed on errors (only 404/absent continues to next candidate) - Update help text, inheritance docstring, and governance docs - Add tests for cascading precedence, ADO profile, and ADO fetch Closes #1813 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
⚠️ Not ready to approve
The new ADO fetch path uses a Bearer auth header and doesn’t reliably pick up ADO_APM_PAT, which will break policy discovery for private Azure DevOps org policy repos.
Pull request overview
Adds host-aware, cascading org policy discovery so Azure DevOps orgs (which cannot create .github) can still use APM governance policies by discovering apm-policy.yml from _apm, while preserving the existing .github convention on GitHub-family hosts.
Changes:
- Implement cascading policy repo discovery (
.github->.apm->_apm) with an ADO-specific host profile that only tries_apm. - Add Azure DevOps Items API fetch path for org policy discovery and wire it into
_auto_discover(). - Update tests and documentation to cover/describe the new discovery conventions.
File summaries
| File | Description |
|---|---|
src/apm_cli/policy/discovery.py |
Adds host-profile candidate selection + ADO fetch implementation and cascaded _auto_discover() logic. |
src/apm_cli/policy/_help_text.py |
Updates --policy help text to mention cascading discovery behavior. |
src/apm_cli/policy/inheritance.py |
Updates docstring to reflect that "org" extends resolves via the policy repo convention, not only .github. |
tests/unit/policy/test_discovery.py |
Adds unit tests for cascade precedence, ADO host profile behavior, and ADO fetch behaviors. |
docs/src/content/docs/enterprise/apm-policy.md |
Documents cascading repo convention and ADO-specific _apm project/repo layout. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 2
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
Address review feedback: - Use ADO_APM_PAT env var with Basic auth (base64 of :<pat>) matching the existing download_ado_file pattern, falling back to Bearer via _get_token_for_host() only when the env var is absent - Clarify help text that ADO hosts only try _apm Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Route ADO policy fetch credentials through AuthResolver so GitHub tokens are never sent to Azure DevOps, repair legacy visualstudio.com and ssh.dev.azure.com URL handling, and fold panel-requested docs/test coverage for the new discovery cascade. Addresses Copilot inline review and panel follow-ups for PR #1830. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Route ADO policy fetch credentials through AuthResolver so GitHub tokens are never sent to Azure DevOps, repair legacy visualstudio.com and ssh.dev.azure.com URL handling, and fold panel-requested docs/test coverage for the new discovery cascade. Addresses Copilot inline review and panel follow-ups for PR #1830. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
a790d2a to
40d84c6
Compare
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 1 | 2 | ADO policy discovery cleanly extends the existing fetch/cache architecture; remaining extraction is future cleanup. |
| CLI Logging Expert | 0 | 0 | 1 | Debug breadcrumbs and AuthResolver-backed ADO error context are now in place. |
| DevX UX Expert | 0 | 0 | 1 | Help text, error wording, and docs are consistent for the ADO cascade. |
| Supply Chain Security Expert | 0 | 0 | 1 | ADO fetch mirrors GitHub security gates: redirect rejection, AuthResolver routing, fail-closed cascade, hash verification, URL encoding. |
| OSS Growth Hacker | 0 | 1 | 2 | ADO support widens the enterprise funnel; quickstart/release story can follow. |
| Auth Expert | 0 | 0 | 1 | ADO auth routes through AuthResolver; remaining resolver reuse note is low-impact. |
| Doc Writer | 0 | 0 | 0 | Docs and apm-usage governance resource now track the cascade truthfully. |
| Test Coverage Expert | 0 | 0 | 0 | New ADO discovery, auth-header, cache/hash, and URL behavior has unit regression coverage. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top follow-ups
- [Python Architect] Extract shared fetch-from-repo orchestration skeleton to reduce duplication between GitHub and ADO paths -- future host additions become lower-risk.
- [Supply Chain Security Expert] Audit and harden the 404-substring-match pattern across all host fetchers -- pre-existing pattern debt, not introduced here.
- [OSS Growth Hacker] Add an ADO quickstart snippet and release-note story -- adoption polish for release prep.
- [DevX UX Expert] Simplify the dense policy help parenthetical -- readability nit.
- [Auth Expert] Consider reusing one AuthResolver instance per discovery run -- negligible today, cleaner in a host-cleanup pass.
Folded in this run
- (copilot) ADO policy fetch now routes credentials through AuthResolver instead of falling back to a GitHub-oriented token path -- resolved in
40d84c62. - (panel) ADO 401/403 diagnostics now use AuthResolver
build_error_contextinstead of hardcoded token guidance -- resolved in40d84c62. - (panel)
build_ado_api_urlnow URL-encodes org, repo, and ref values and handles legacy*.visualstudio.complusssh.dev.azure.comcorrectly -- resolved in40d84c62. - (panel) Policy docs and the packaged apm-usage governance resource now describe the
.github/.apm/_apmcascade and ADO_apmconvention truthfully -- resolved in40d84c62. - (panel) Added direct
_fetch_from_ado_repotests for valid content, 404 absent, stale-cache fallback, invalid YAML, and hash-pin mismatch -- resolved in40d84c62. - (panel) Added regression tests for AuthResolver header selection, visualstudio.com org parsing,
ssh.dev.azure.comADO detection, and ADO URL encoding -- resolved in40d84c62. - (panel) Added debug breadcrumbs for policy repo candidate cascade -- resolved in
40d84c62.
Copilot signals reviewed
review:4519414788-- LEGIT: Copilot identified that the ADO fetch path used an unreliable Bearer fallback and did not reliably pick upADO_APM_PAT; the PR now delegates ADO auth to AuthResolver and preserves Basic vs Bearer scheme selection (resolved in40d84c62).
Deferred (out-of-scope follow-ups)
- (panel) Extract a shared fetch/cache/parse helper across GitHub and ADO policy fetchers -- scope boundary: this PR fixes ADO policy discovery; cross-fetcher refactoring is a host-cleanup task affecting existing GitHub code.
- (panel) Replace string-based 404 detection with structured fetch outcomes across policy fetchers -- scope boundary: this is pre-existing pattern debt in both GitHub and ADO paths, not required to land the ADO discovery fix.
- (panel) Add an ADO quickstart snippet and release-note story -- scope boundary: this is adoption/release-prep polish beyond the bug fix and docs truthfulness updates.
- (panel) Simplify the policy help parenthetical -- scope boundary: current help is accurate; readability polish can happen in a CLI help pass.
Regression-trap evidence (mutation-break gate)
tests/unit/test_github_host.py::test_build_ado_api_url-- deleted legacy visualstudio.com URL guard; test FAILED as expected; guard restored.tests/unit/policy/test_discovery.py::TestFetchAdoContents::test_success-- changed AuthResolver host resolution togithub.com; test FAILED as expected; guard restored.tests/unit/policy/test_discovery.py::TestFetchFromAdoRepo::test_api_error_uses_stale_cache-- changed ADO cache key shape; test FAILED as expected; guard restored.tests/unit/policy/test_discovery.py::TestFetchAdoContents::test_401_returns_error-- removedbuild_error_contextfrom ADO 401 path; test FAILED as expected; guard restored.tests/unit/test_github_host.py::test_build_ado_api_url-- removed ADO ref URL encoding; test FAILED as expected; guard restored.
Lint contract
uv run --extra dev ruff check src/ tests/, uv run --extra dev ruff format --check src/ tests/, pylint R0801, and scripts/lint-auth-signals.sh are clean locally.
CI
gh pr checks 1830 --repo microsoft/apm is green on head 40d84c62a11359a0f0f8d9367889a01529c8ad4f: CI, CodeQL, docs build, merge gate, NOTICE drift, spec conformance, PR binary smoke, coverage combine, and CLA all pass (after 0 CI fix iterations).
Mergeability status
Captured from gh pr view 1830 --json mergeable,mergeStateStatus,statusCheckRollup immediately after the last push of this run.
| PR | head SHA | CEO stance | iters | folds | defers | Copilot rounds | CI | mergeable | mergeStateStatus | notes |
|---|---|---|---|---|---|---|---|---|---|---|
| #1830 | 40d84c6 |
ship_now | 1 | 7 | 4 | 2 | green | MERGEABLE | BLOCKED | pending required review |
Convergence
1 outer iteration; 2 Copilot rounds. Final panel stance: ship_now.
Ready for maintainer review.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
Resolve strangler-fig conflict in policy/discovery.py against main #1830 (cascading policy repo discovery with ADO support): - Import block: keep #1078 sibling-module imports + main's github_host imports; drop the stale path_security import (its sole consumer _get_cache_dir was relocated to _discovery_cache.py by #1078). - _auto_discover docstring: take main's cascading description (body auto-merged). - _parse_remote_url: main's visualstudio branch pushed it to 9 returns (>8); extract the scheme-URL branch into _parse_scheme_remote_url. - File-length: main's +226 ADO lines pushed discovery.py to 894 (>800 gate). Relocate the ADO transport (_fetch_from_ado_repo, _fetch_ado_contents) into new _discovery_ado.py, re-exported from discovery; sibling routes requests/cache helpers/_fetch_ado_contents through the discovery module so existing patch seams still intercept. discovery.py now 762 lines. Verified: 932 policy tests pass, merge-touched github_host/cleanup tests pass, full CI-mirror lint chain green (ruff check/format, R0801 10/10, auth-signals, file-length<=800). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#1871) Add the user-facing PRs merged after the v0.21.0 tag that were missing from the changelog (#1830 Added; #1854, #1856 Fixed), add the external- contributor credit on #1855, and move #1853 out of the released [0.21.0] section into [Unreleased] since it merged after the v0.21.0 tag. No version bump -- everything stays under [Unreleased]. Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* test(policy): align EMU/ADO discovery E2E tests with cascade routing The merge-queue Integration Tests job (merge_group event) failed on two stale assertions in test_dep_url_parsing_e2e.py that predate the cascading policy-repo discovery added in #1830: - EMU test asserted `result is sentinel`, but cascade discovery (.github -> .apm -> _apm) returns a fresh terminal "absent" result rather than propagating the per-candidate sentinel object. The #1159 regression guard is the ROUTING (first candidate == contoso/.github), which is preserved; the identity assertion is replaced with an outcome assertion. - ADO test mocked `_fetch_from_repo`, but ADO remotes now route through `_fetch_from_ado_repo` (#1830), so the mock was never called (call_count == 0). Re-point the patch at `_fetch_from_ado_repo` and assert the real org (`realorg`, not `v3`) is passed as a kwarg. Production discovery is correct; these are test-only fixes that keep the #1159 SCP-regex routing regression guard intact while being cascade-compatible. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(plugin): harden live sequential uninstall assertion Offline local plugin repro shows sequential installs preserve deployed_files and uninstall cleans deployed agent, prompt, and skill files. The live SAML-protected plugin can change its primitive mix, so the network E2E now validates cleanup against files actually deployed instead of requiring an agent cleanup line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Problem
Azure DevOps does not allow repository names starting or ending with a period (
.). APM's org-wide policy discovery hardcodes.githubas the policy repo name, which means ADO organisations cannot use APM's governance policy feature at all.Additionally,
_fetch_github_contents()only speaks the GitHub Contents API -- even if the repo name were changed, ADO needs a different REST API call (Items API).Closes #1813
Approach
Introduce cascading policy repo discovery with three candidate repo names tried in precedence order, and host profiles that determine which candidates are valid per git platform.
Candidate repos (precedence order)
.github.apm_apmHost profiles
.github,.apm,_apm_apmonlyFirst valid policy found wins. Errors (auth, timeout, malformed YAML) fail-closed immediately -- only 404/absent continues to the next candidate.
Implementation
src/apm_cli/policy/discovery.py_policy_repo_candidates), cascading_auto_discover(), new_fetch_from_ado_repo()and_fetch_ado_contents()src/apm_cli/policy/_help_text.pysrc/apm_cli/policy/inheritance.pytests/unit/policy/test_discovery.pydocs/src/content/docs/enterprise/apm-policy.mdHow to test
All 84 discovery tests pass (923 total policy tests pass).
apm-spec-waiver: Bug fix -- cascading policy repo discovery for ADO platform support, no new normative behaviour