Add .github-private as preferred policy repo candidate

[sergio-sisternes-epam]

Why

Most organisations will want their apm-policy.yml to live in a private repository. GitHub offers .github-private as the convention for private org-wide configuration, but APM's policy discovery was only checking .github, .apm, and _apm -- forcing orgs to expose governance rules in a public repo.

Approach

Prepend .github-private to the candidate list so it is tried first, with .github as the natural fallback. The new precedence order is:

.github-private > .github > .apm > _apm

This is fully backwards-compatible: orgs already publishing policy in .github will see a 404 on .github-private and fall through transparently. ADO is unaffected (dot-prefixed repo names are invalid there, so the ADO tuple stays ("_apm",)).

Changes

• src/apm_cli/policy/discovery.py -- updated _DEFAULT_POLICY_REPOS tuple and all docstrings referencing the candidate order.
• tests/unit/policy/test_discovery.py -- updated cascade mock counts, first-call assertions, and candidate tuple expectations (all 92 tests pass).
• Docs (apm-policy.md, governance-guide.md, policy-reference.md, governance.md) -- updated to reflect the new precedence and recommend .github-private as the preferred location.

Non-obvious details

• The cache-hit test needed a small rework: since .github-private has no cached entry, it now makes one _fetch_github_contents call (returning 404) before falling through to the cached .github entry. The assertion was updated from assert_not_called() to call_count == 1.

[Copilot]

Pull request overview

Updates APM org-policy discovery to prefer GitHub’s private org-wide configuration repository (.github-private) before falling back to existing candidates, enabling organizations to keep apm-policy.yml private without changing the ADO behavior.

Changes:

• Prepended .github-private to the default policy repo candidate cascade: .github-private > .github > .apm > _apm.
• Updated unit tests to reflect the new candidate order and cache/fetch call behavior.
• Updated enterprise docs and the apm-guide governance skill docs to recommend .github-private and describe fallback behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/apm_cli/policy/discovery.py	Adds .github-private to _DEFAULT_POLICY_REPOS and updates discovery docstrings to match the new precedence.
tests/unit/policy/test_discovery.py	Updates expectations for source strings, candidate ordering, and fetch call counts under caching.
packages/apm-guide/.apm/skills/apm-usage/governance.md	Updates governance guidance to reflect .github-private being checked first on GitHub-compatible hosts.
docs/src/content/docs/enterprise/policy-reference.md	Updates the org inheritance source description to .github-private with fallback to .github.
docs/src/content/docs/enterprise/governance-guide.md	Updates narrative and trust-boundary references to prefer .github-private (or .github).
docs/src/content/docs/enterprise/apm-policy.md	Updates the precedence table and “get started” guidance to prefer .github-private and document fallback behavior.

[github-actions]

APM Review Panel: needs_rework

Hold for rework: GitHub 404-masking silently bypasses .github-private policy for scope-limited tokens -- the PR's stated fail-closed contract does not hold for the primary deployment scenario.

cc @sergio-sisternes-epam @danielmeppiel -- a fresh advisory pass is ready for your review.

The core code change is minimal, correct, and strategically valuable: prepending .github-private to _DEFAULT_POLICY_REPOS is a clean Chain-of-Responsibility extension that unlocks regulated enterprises refusing to expose policy in a public .github repo. The EPAM contributor signal is strong third-party validation of real demand. ADO is unaffected; the cascade is backwards-compatible; the code should ship. But not yet -- because the PR's stated security contract is unsound for the primary deployment scenario it enables.

Two blocking findings converge on the same root cause (supply-chain-security and auth-expert, corroborated at recommended tier by devx-ux): GitHub deliberately returns HTTP 404 -- not 401 or 403 -- when a token lacks private-repo read scope, to prevent repo-existence probing. _fetch_from_repo maps any 404 to outcome='absent'; the cascade silently continues to .github with no warning emitted. An org that publishes policy ONLY in .github-private -- the exact high-value scenario this PR targets -- with a default GITHUB_TOKEN (Actions default permissions, or any fine-grained PAT without repo scope) gets no policy enforcement and no indication anything is wrong. The fail-closed contract the PR documents does not hold for this misconfiguration. The fix is bounded and does not require changing the one-line constant: emit a diagnostic warning when outcome='absent' is returned for a .github-private candidate on a github.com host, add a unit test asserting 403 maps to cache_miss_fetch_fail (not absent), add a test covering the 404-fallthrough scenario, and document the required token scope.

The docs gap is material: doc-writer and oss-growth-hacker identified 15+ stale references in governance-guide sections 11, 12, 15, and 16, and in policy-reference.md at L470, L609, and L946. These sections contain verbatim operator instructions -- practitioners following the Phase 1 rollout guide or the CODEOWNERS/branch-protection setup in section 12 will create policy in <org>/.github, not <org>/.github-private, defeating the purpose of the PR for any org that follows the documentation. The performance cost -- one uncached 404 probe per apm install for the dominant .github cohort -- is a recommended fix with a clean, bounded solution (negative-cache sentinel for absent outcomes) and should accompany or follow the blocking fixes.

Aligned with: Portable by Manifest (unaffected, no schema change). Secure by Default (FAILS on current state: 404-masking bypass must be addressed before this principle is satisfied). Governed by Policy (partially: intent is correct but silent fallthrough on scope-limited tokens undermines enforcement for the exact cohort targeted). Multi-Harness/Multi-Host (ADO unaffected; minor nit: .github-private appears in the default candidate list for non-GitHub hosts via default fallback). OSS Community-Driven (strong positive: EPAM contributor validating real regulated-enterprise demand). Pragmatic as npm (marginally degraded: uncached 404 probe adds permanent per-run latency for the dominant .github cohort).

Growth signal. .github-private support is an enterprise unlock that removes the primary objection from security-conscious orgs that cannot expose governance policy in a public repository. Story angle for next release: "APM governance now works for security-conscious orgs." The EPAM contributor provenance adds third-party credibility. Once the diagnostic and docs fixes land, this feature should lead the CHANGELOG feature section and anchor the enterprise adoption narrative.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	2	Clean Chain-of-Responsibility extension; absent .github-private is never cached so every warm-path invocation pays one extra GitHub API RTT for most orgs.
CLI Logging Expert	0	2	1	Core change is clean; 403/401 path in _fetch_github_contents lacks build_error_context(), now a high-probability hit because .github-private is private by definition.
DevX UX Expert	0	3	1	Backwards-compatible, naming is fine; two self-debug gaps: 403 lacks remediation hint, GitHub silently 404s private repos with wrong token scope defeating the privacy guarantee.
Supply Chain Security Expert	1	2	1	Silent policy bypass when .github-private is private and token lacks read scope: GitHub returns 404 (not 403), APM falls through to .github without warning.
OSS Growth Hacker	0	2	2	Enterprise unlock for regulated orgs; governance-guide rollout playbook still teaches the old .github path -- practitioners will follow stale instructions, defeating the PR's goal.
Auth Expert	1	2	1	GitHub returns 404 (not 403) when token lacks private-repo scope -- .github-private policy silently treated as absent, contradicting stated fail-closed auth-error semantics.
Doc Writer	0	4	2	4 files updated but 15+ stale prose refs remain; critical misses are the discovery definition callout in policy-reference.md and Phase 1 rollout guidance in governance-guide.md.
Test Coverage Expert	0	3	1	Cascade ordering and 404-fallthrough covered; two security-relevant regression traps missing: no 403-is-not-absent assertion, no test for .github-private error with .github available.
Performance Expert	0	1	1	.github-private prepends an uncached HTTP 404 probe to every apm install for the majority of orgs; absent results need a negative cache or this cost is permanent.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

1. [Supply Chain Security Expert] (blocking-severity) Emit a diagnostic warning when .github-private returns outcome=absent on a github.com host -- the host+absent combination signals a likely token scope gap and must not be silent. -- GitHub returns HTTP 404 for private repos with insufficient token scope; APM silently cascades to .github; an org with policy ONLY in .github-private gets no enforcement and no warning.
2. [Auth Expert] (blocking-severity) Add unit tests: (1) 403 -> cache_miss_fetch_fail (not absent), (2) .github-private absent + .github available does NOT silently substitute -- explicit automated proof of fail-closed behavior on this secure-by-default surface. -- Both tests are confirmed missing (outcome=missing, tier=unit) on a secure-by-default surface; any future refactor can reintroduce the silent bypass with no guardrail.
3. [Doc Writer] Update governance-guide sections 11, 12, 15, 16 and policy-reference.md L470, L609, L946: replace <org>/.github with .github-private as primary, add migration callout for existing .github users, and document required token scope. -- Practitioners following the Phase 1 rollout guide and section 12 CODEOWNERS/branch-protection instructions will configure the wrong repo; the feature defeats itself for any org that follows the documentation.
4. [Performance Expert] Add negative-cache sentinel for absent outcomes to eliminate the permanent uncached 404 probe for the dominant .github cohort. -- Every apm install for orgs on .github pays one extra live GitHub API RTT indefinitely; a negative-cache sentinel is a clean, bounded fix requiring no cascade logic change.
5. [CLI Logging Expert] Apply build_error_context() to the GitHub 403/401 path in _fetch_github_contents -- the ADO path already has it; parity is now a high-probability UX hit because .github-private is private by definition. -- Without remediation hints on the 403/401 path, operators hitting token scope errors on .github-private will see a generic HTTP error with no actionable guidance.

Architecture

classDiagram
    direction TB
    class DiscoveryModule {
        <<Module>>
        _DEFAULT_POLICY_REPOS tuple
        _ADO_POLICY_REPOS tuple
        +discover_policy(root, policy_override, no_cache) PolicyFetchResult
        +_auto_discover(root, no_cache) PolicyFetchResult
        +_policy_repo_candidates(host) tuple
        +_fetch_from_repo(repo_ref, root, no_cache) PolicyFetchResult
    }
    class PolicyFetchResult {
        <<ValueObject>>
        +policy ApmPolicy or None
        +source str
        +cached bool
        +outcome str
        +error str or None
        +found bool
    }
    class ApmPolicy {
        <<ValueObject>>
        +extends str or None
    }
    class GithubHost {
        <<Pure>>
        +is_azure_devops_hostname(host) bool
        +is_visualstudio_legacy_hostname(host) bool
    }
    note for DiscoveryModule "Chain of Responsibility: .github-private > .github > .apm > _apm | ADO: _apm only | absent=pass-through, error=fail-closed"
    DiscoveryModule ..> PolicyFetchResult : produces
    DiscoveryModule ..> GithubHost : host-type dispatch
    PolicyFetchResult *-- ApmPolicy : contains
    class DiscoveryModule:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    A["discover_policy(root)"] --> B["_auto_discover(root)"]
    B --> C["[NET] _extract_org_from_git_remote\ngit remote get-url origin"]
    C -->|None| ERR1[outcome: no_git_remote]
    C -->|org and host| D["_policy_repo_candidates(host)"]
    D -->|ADO host| E["_ADO_POLICY_REPOS: _apm"]
    D -->|non-ADO host| F["_DEFAULT_POLICY_REPOS:\n.github-private .github .apm _apm"]
    E --> G{for candidate_repo in candidates}
    F --> G
    G --> H["_read_cache_entry(repo_ref, root)"]
    H -->|fresh cache hit| HIT["outcome: found, cached=True - return early"]
    H -->|miss or stale| I["[NET] _fetch_github_contents(repo_ref)"]
    I -->|"404 - no _write_cache call"| AC["outcome: absent\nnext candidate"]
    AC --> G
    I -->|auth or timeout error| CL["fail-closed\noutcome: cache_miss_fetch_fail"]
    I -->|200 OK| K["_verify_hash_pin(content)"]
    K -->|mismatch| HF[outcome: hash_mismatch]
    K -->|ok| L["load_policy(content)"]
    L --> M["[FS] _write_cache(repo_ref, policy, root)"]
    M --> FO[outcome: found]
    G -->|all candidates exhausted| AB[outcome: absent]

Loading

Recommendation

Do not merge as-is. The core code change is correct and strategically sound -- the one-line constant and docstring are right, and this feature should ship. Two blocking items must land first: (1) a diagnostic warning emitted when .github-private returns outcome='absent' on a github.com host, surfacing the likely token-scope gap to the operator rather than silently cascading to .github, and (2) unit tests asserting that 403 maps to cache_miss_fetch_fail (not absent) and that a .github-private absent result does not silently substitute .github policy -- both missing on a secure-by-default surface. The doc gap (15+ stale practitioner instructions in governance-guide and policy-reference) and the uncached 404 probe performance cost should be addressed in the same PR if scope permits, or as immediate follow-ups before the next release note amplifies this feature. Once the diagnostic warning and regression-trap tests land, this is a clean ship with a strong enterprise story and a genuine third-party contributor signal worth celebrating.

---

Full per-persona findings

Python Architect

• [recommended] Absent .github-private 404 is not cached, adding one live GitHub API call per discovery run for most orgs at src/apm_cli/policy/discovery.py:924
  _fetch_from_repo returns PolicyFetchResult(outcome='absent') on a 404 without calling _write_cache. Only found outcomes are cached. For every org using .github (the dominant cohort), every discover_policy invocation -- including warm path after .github is in cache -- always makes one extra live GitHub Contents API call that returns 404. This is per-invocation overhead, not just a one-time cold-start cost.
  Suggested: Cache absent outcomes with a short TTL (e.g., 15 min) so the 404 is not re-fetched on every run. Or document the accepted one-RTT warm-path cost in the module docstring.

• [nit] _policy_repo_candidates returns .github-private for non-GitHub hosts (GitLab, Gitea) via the default fallback branch at src/apm_cli/policy/discovery.py:87
  .github-private is a GitHub-specific naming convention. Silently adds one extra no-op API call per discovery for every non-GitHub, non-ADO deployment.
  Suggested: No action required now. If non-GitHub host support becomes first-class, split _DEFAULT_POLICY_REPOS into _GITHUB_POLICY_REPOS (includes .github-private) and _GENERIC_POLICY_REPOS (without).

• [nit] No cache-hit test for .github-private itself on the warm path at tests/unit/policy/test_discovery.py:633
  test_cache_hit_returns_cached covers .github-private 404 -> .github from cache but not the symmetric case: .github-private IS populated and its own cache entry is served.
  Suggested: Add: pre-populate cache for contoso/.github-private, call discover_policy(no_cache=False), assert result.cached==True and mock_fetch.assert_not_called().

CLI Logging Expert

• [recommended] _fetch_github_contents 403 emits no remediation hint -- now routinely triggered by .github-private token scope gaps at src/apm_cli/policy/discovery.py
  discovery.py:1005-1006 returns '403: Access denied to {repo_ref}' with no guidance. The ADO path already calls auth_resolver.build_error_context() on 403. .github-private is private by design, making 'token has public_repo scope only' a normal first-run mistake.
  Suggested: Apply build_error_context(host, 'fetch org policy') in _fetch_github_contents for 401/403, mirroring the ADO path.

• [recommended] 401 from .github-private falls to generic error with no token scope hint at src/apm_cli/policy/discovery.py
  _fetch_github_contents has no explicit 401 branch. Unlike .github (usually readable without auth), .github-private requires a repo-scoped token.
  Suggested: Add an explicit '401' branch alongside 403, both routing through auth_resolver.build_error_context().

• [nit] Verbose-mode per-candidate log format exposes module path; consider a cleaner prefix at src/apm_cli/policy/discovery.py
  logger.debug calls surface as 'DEBUG apm_cli.policy.discovery ...' which is noisy for users enabling --verbose.
  Suggested: Optionally emit STATUS_SYMBOLS-based lines, e.g. '[>] Checking .github-private for org policy' and '[i] .github-private absent -- trying .github'.

DevX UX Expert

• [recommended] GitHub 403 auth error on .github-private lacks remediation hint; ADO path has it at src/apm_cli/policy/discovery.py
  _fetch_github_contents (line 1005-1006) returns '403: Access denied' with no next-action. .github-private is the only candidate that is private by design.
  Suggested: Mirror the ADO pattern: call auth_resolver.build_error_context(host, 'fetch org policy', org=org) when status is 401 or 403.

• [recommended] GitHub returns 404 (not 403) for private repos when the token lacks repo scope -- silently falls through to .github without warning at docs/src/content/docs/enterprise/apm-policy.md
  When token lacks repo scope, GitHub Contents API returns 404 for any private repo. APM treats 404 as 'absent' and falls through to .github. The org believes private policy is active when it is not.
  Suggested: Add a caution callout documenting that GitHub returns 404 for private repos with insufficient scope, and listing the required token permissions.

• [recommended] No migration callout for orgs already using .github at docs/src/content/docs/enterprise/apm-policy.md
  The quickstart now leads with .github-private without explaining existing users need do nothing.
  Suggested: Add: 'Already using .github? No migration needed -- .github-private is an opt-in upgrade. APM checks .github-private first; if absent, it falls back to .github as before.'

• [nit] CI recipe in enforce-in-ci.md does not mention required token scope for .github-private at docs/src/content/docs/enterprise/enforce-in-ci.md

Supply Chain Security Expert

• [blocking] GitHub returns 404 (not 403) for private repos with insufficient token scope -- silent policy bypass at src/apm_cli/policy/discovery.py
  When org/.github-private is private and token lacks read scope, GitHub returns HTTP 404 (not 403) to obscure private repo existence. _fetch_from_repo maps 404 -> outcome='absent' -> cascade continues to .github. An org with policy ONLY in .github-private using a standard GITHUB_TOKEN gets no enforcement and no warning. The PR's stated fail-closed contract does NOT hold for the most common misconfiguration case.
  Suggested: Emit a structured warning (not debug-level) when .github-private returns 404 on a github.com host. Prominently document in apm-policy.md that GitHub returns 404 for private repos with insufficient scope, and the exact token scope required.

• [recommended] No test for 'private repo, insufficient token scope -> 404 -> falls through' scenario at tests/unit/policy/test_discovery.py
  A test documenting this behaviour would prevent future regressions and make the tradeoff explicit.
  Suggested: Add a test: mock _fetch_github_contents to return (None, '404: Policy file not found') for .github-private, assert cascade continues to .github, document the known limitation.

• [recommended] Token scope requirements for .github-private undocumented; insufficient scope produces no actionable diagnostic at docs/src/content/docs/enterprise/apm-policy.md
  Existing deployments using minimally-scoped tokens will silently fall through to .github with no warning.
  Suggested: Add a callout: repo scope (classic PAT) or Contents:Read (fine-grained PAT) required; without it GitHub returns 404 and APM falls back to .github.

• [nit] .github-private is not an official GitHub convention; PR body overstates its provenance at docs/src/content/docs/enterprise/apm-policy.md
  GitHub's official special repo names are .github and the profile README repo. .github-private is an informal community naming convention; naming a repo .github-private does not automatically make it private.
  Suggested: Clarify provenance and note that visibility settings must be configured separately.

OSS Growth Hacker

• [recommended] governance-guide.md VP-of-Engineering blurb still routes to <org>/.github with no mention of .github-private
  L30: 'until your org policy file lands in (org)/.github' -- this is the primary enterprise buyer reading this guide before approving a rollout. The PR teaches .github-private is preferred everywhere else but the decision-maker persona is still pointed at the old path.
  Suggested: Update to: '...until your org policy file lands in (org)/.github-private (or (org)/.github for public policy)'.

• [recommended] governance-guide.md sections 11/12/15/16 still instruct practitioners to create <org>/.github policy files in ~12 operational locations
  Phase 1 mermaid node, Phase 1/3/4 prose, section 12 CODEOWNERS/branch-protection/Rulesets, section 15 decision-tree, section 16 15-minute-path tip all reference .github without update.
  Suggested: Update section 11 Phase 1 text and mermaid node; section 12 security references (brief '(or .github-private)' parenthetical); section 16 '15-minute path' tip.

• [nit] apm-policy.md 'Get started in 20 minutes' forces a repo-structure decision before the first copy-paste
  Step 1 presents two parallel paths with a parenthetical bifurcation -- a conversion leak at step 1 is high-cost.
  Suggested: Rewrite step 1 to be opinionated (.github-private primary), with a tip callout below for .github as an opt-in alternative.

• [nit] apm-policy.md 'Where it lives' table does not link .github-private to GitHub's convention documentation
  .github-private is less well-known than .github; a citation removes ambiguity at the moment of decision.

Auth Expert

• [blocking] Token lacking private-repo scope yields GitHub 404, treated as 'absent', silently bypassing .github-private policy at src/apm_cli/policy/discovery.py
  GitHub API deliberately returns 404 (not 401/403) when token lacks repo scope for a private repo. _fetch_github_contents maps HTTP 404 to '404: Policy file not found'. _fetch_from_repo maps this to outcome='absent', causing cascade. GITHUB_TOKEN with default Actions permissions, fine-grained PATs missing 'repo' scope, or any public-only token will silently fall through. An org publishing policy ONLY in .github-private with a standard Actions workflow gets no enforcement and no indication anything is wrong.
  Suggested: Document the required token scope prominently (classic PAT: 'repo'; fine-grained PAT: 'contents:read' for .github-private; GITHUB_TOKEN from a workflow in a different repo is insufficient). Emit a visible warning when all candidates return absent AND the host is a GitHub-class host where .github-private would require auth.

• [recommended] No test covering the 404-masking / token-scope-missing scenario for .github-private at tests/unit/policy/test_discovery.py
  There is no test mocking _fetch_github_contents returning (None, '404: Policy file not found') for .github-private while a live .github policy exists. A future refactor could change 404-cascade logic without any test catching the regression.
  Suggested: Add a test documenting the 404-masking behavior, even if only to record the current limitation.

• [recommended] Docs do not state required token scope for .github-private access at docs/src/content/docs/enterprise/apm-policy.md
  apm-policy.md and governance-guide.md promote .github-private as preferred but nowhere state what token scope an operator must provision. Adopters will set up .github-private, run apm install in CI, and silently get no policy enforced.
  Suggested: Add a 'Required token scope' callout covering classic PAT, fine-grained PAT, and GITHUB_TOKEN limitations.

• [nit] Every warm-.github-cache invocation now pays one extra HTTP round-trip at src/apm_cli/policy/discovery.py
  test_cache_hit_returns_cached changed from mock_fetch.assert_not_called() to mock_fetch.call_count == 1, explicitly acknowledging the new warm-path cost.
  Suggested: Seed a negative cache entry for .github-private on first 404 so repeated invocations skip the round-trip within the TTL window.

Doc Writer

• [recommended] Two definitional discovery statements in policy-reference.md still name .github/apm-policy.yml as the only discovery target at docs/src/content/docs/enterprise/policy-reference.md
  L470 (:::note callout 'Discovery vs. extends:'): 'APM auto-discovers exactly one policy file: (org)/.github/apm-policy.yml'. L609 repeats the same claim. Both directly contradict the updated inheritance table on L103 and the priority table in apm-policy.md.
  Suggested: L470 and L609: change '(org)/.github/apm-policy.yml' to '(org)/.github-private/apm-policy.yml (falling back to .github)'.

• [recommended] governance-guide.md Phase 1 rollout and 15-minute path still tell adopters to land policy in <org>/.github at docs/src/content/docs/enterprise/governance-guide.md
  L30 (intro), L396 (Phase 1), L604 (Section 16 tip) are the copy-paste instructions most first-time adopters follow verbatim.
  Suggested: L30: '(org)/.github-private (or .github)'. L396 and L604: prefer .github-private with .github as opt-in fallback.

• [recommended] Section 12 trust boundary setup (CODEOWNERS, branch protection, Rulesets) is written exclusively for <org>/.github at docs/src/content/docs/enterprise/governance-guide.md
  L440-L446: CODEOWNERS, branch protection, and Rulesets all reference (org)/.github. An admin following the new recommendation who then follows Section 12 verbatim will set up access controls on the wrong repo.
  Suggested: Update L440-L446 to use .github-private as the primary example with '(or .github)' parenthetical.

• [recommended] policy-reference.md quickstart checklist item (L946) still says 'Create <org>/.github/apm-policy.yml' at docs/src/content/docs/enterprise/policy-reference.md
  First step of 'Checklist to publish a policy' should lead with .github-private as the preferred target.
  Suggested: Change to: 'Create (org)/.github-private/apm-policy.yml (or (org)/.github/apm-policy.yml) in the org policy repository.'

• [nit] apm-policy.md Step 2 drops the .github fallback one sentence after offering it at docs/src/content/docs/enterprise/apm-policy.md
  Step 1 offers both .github-private and .github. Step 2 says 'before it lands in .github-private' without mentioning .github.
  Suggested: Change to: 'before it lands in your policy repo (.github-private or .github):'.

• [nit] CLI output examples in policy-reference.md still show org:contoso/.github as the policy source at docs/src/content/docs/enterprise/policy-reference.md
  Simulated terminal output visually anchors readers to .github as the expected source string.
  Suggested: Update representative example output blocks to use org:contoso/.github-private.

Test Coverage Expert

• [recommended] TestFetchFromRepo has no test asserting 403 -> cache_miss_fetch_fail (not absent) -- silent bypass regression trap missing at tests/unit/policy/test_discovery.py
  The fail-closed guarantee depends on '_fetch_from_repo: if "404" in error: return absent; else: return cache_miss_fetch_fail'. The 403 path is untested at _fetch_from_repo level. If a developer accidentally extended the 404-absent branch to match '403', no test would catch it.
  Suggested: Add: mock_fetch.return_value = (None, '403: Access denied to contoso/.github-private'); assert result.outcome == 'cache_miss_fetch_fail'; assert result.outcome != 'absent'.
  Proof (missing at unit): tests/unit/policy/test_discovery.py::test_403_is_error_not_absent -- proves: When .github-private returns HTTP 403, APM classifies it as a fetch error (fail-closed), not a missing repo. [secure-by-default,governed-by-policy]

• [recommended] No TestAutoDiscover test for .github-private error with .github available -- explicit fail-closed proof missing at tests/unit/policy/test_discovery.py
  test_github_com_error_fail_closed uses return_value (all candidates return same error), not side_effect = [error, success]. It does not prove that .github is never tried when .github-private has an auth error.
  Suggested: Add: mock_fetch.side_effect = [PolicyFetchResult(error='403: Access denied', outcome='cache_miss_fetch_fail'), PolicyFetchResult(policy=ApmPolicy(), outcome='found')]; assert mock_fetch.call_count == 1; assert not result.found.
  Proof (missing at unit): tests/unit/policy/test_discovery.py::test_github_private_auth_error_does_not_fall_through_to_github -- proves: When .github-private returns an auth error (403/401), APM fails closed and does not attempt .github. [secure-by-default,governed-by-policy]

• [recommended] PR body lacks Scenario Evidence table; backward-compat promise has no _auto_discover-level regression trap at tests/unit/policy/test_discovery.py
  The backward-compat promise ('.github-private absent -> .github found') is only tested at discover_policy level via cache hit. No TestAutoDiscover test uses side_effect = [absent, found] to prove the cascade to a live .github fetch.
  Suggested: Add test_github_private_absent_falls_back_to_github with side_effect asserting call_count==2, .github-private tried first, .github tried second, result.found==True.
  Proof (missing at unit): tests/unit/policy/test_discovery.py::test_github_private_absent_falls_back_to_github -- proves: When .github-private does not exist (404), APM falls through to .github and serves the policy from there. [governed-by-policy]

• [nit] test_github_com_error_fail_closed does not assert which candidate triggered the error at tests/unit/policy/test_discovery.py
  The test asserts call_count==1 but not mock_fetch.call_args_list[0][0][0] == 'contoso/.github-private'. If candidate order regressed, this test would still pass.
  Suggested: Add self.assertEqual(mock_fetch.call_args_list[0][0][0], 'contoso/.github-private') to make it a standalone proof of the .github-private fail-closed contract.
  Proof (passed at unit): tests/unit/policy/test_discovery.py::test_github_com_error_fail_closed -- proves: An auth error on the first policy repo candidate stops the cascade. [secure-by-default,governed-by-policy]

Performance Expert

• [recommended] 404/absent outcomes are never written to cache; .github-private probe fires live on every apm install forever for orgs that stay on .github at src/apm_cli/policy/discovery.py
  _fetch_from_repo returns PolicyFetchResult(outcome='absent') on a 404 without calling _write_cache (line 952, only reached on successful fetch). The PR's own test_cache_hit_returns_cached proves: mock_fetch.call_count==1 even with a warm .github cache. At 100 repos x 10 CI builds/day, ~1,000 extra GitHub API calls per org per day indefinitely.
  Suggested: Add a negative-cache sentinel: when _fetch_from_repo gets a 404, write a short-TTL absent marker ({key}.absent sidecar, TTL 5-15 min). In _read_cache_entry, check for this sidecar and return synthetic _CacheEntry(absent=True) so _fetch_from_repo short-circuits before calling _fetch_github_contents.

• [nit] 10s timeout on requests.get applies to the new first-position probe; on degraded network each apm install stalls extra 10s at src/apm_cli/policy/discovery.py
  _fetch_github_contents uses timeout=10 (line 1002). Adding .github-private as the first probe widens the blast radius on network partitions or slow GHE hosts.
  Suggested: Optionally reduce the timeout to 5s or make it configurable per-callsite.

_{This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.}

Generated by PR Review Panel for issue #2058 · 683.2 AIC · ⌖ 20.4 AIC · ⊞ 5.5K · ◷