fix(ado): resolve az binary via shutil.which for Windows az.cmd (closes #1430)

[danielmeppiel]

TL;DR

apm update against private Azure DevOps deps fails on Windows with Authentication failed for dev.azure.com and the diagnostic "az present but not logged in" — even when the user IS signed in via az login. The root cause is that subprocess.run(["az", ...]) calls CreateProcessW, which does not honor PATHEXT for non-.exe executables, so the Windows az.cmd wrapper cannot be invoked despite shutil.which("az") resolving it. This PR resolves az via shutil.which once at construction and widens the ADO preflight carve-out so Git Credential Manager survives as a fallback. Closes #1430.

Problem (WHY)

Important

The user is shown a remediation message instructing them to run az login — the exact action they have already taken. Worst-possible-failure-mode for diagnostics.

The bug surfaces only on apm update, not apm install, because the preflight is gated on update_refs. The Windows-only cascade:

1. AzureCliBearerProvider.is_available() returns True (shutil.which honors PATHEXT).
2. Every subprocess.run([self._az_command, ...]) raises FileNotFoundError (CreateProcessW does not honor PATHEXT).
3. _run_get_access_token wraps the OSError as AzureCliBearerError(kind="subprocess_error").
4. _resolve_token in core/auth.py swallows it -> (None, "none", "basic").
5. _preflight_auth_check builds a probe env that strips GIT_CONFIG_GLOBAL / GIT_CONFIG_NOSYSTEM, killing Git Credential Manager — the only remaining channel that could have rescued the probe with an Entra-cached credential.
6. git ls-remote returns 401. build_error_context re-hits the same broken subprocess in get_current_tenant_id(), swallows the failure, and renders Case 3: "az present but not logged in".

PROSE anchor: "Grounding outputs in deterministic tool execution transforms probabilistic generation into verifiable action." The fix replaces a probabilistic PATH lookup (left to CreateProcessW) with a deterministic resolution (shutil.which).

Approach (WHAT)

Two layers, both required:

Layer	Where	What
Root cause	src/apm_cli/core/azure_cli.py	Resolve az via shutil.which once in __init__; pass the absolute path to every subprocess.run.
Defense in depth	src/apm_cli/install/pipeline.py	Widen the existing generic-host carve-out so ADO preflight probes also keep the user's ~/.gitconfig available, preserving Git Credential Manager as a fallback channel.

Implementation (HOW)

src/apm_cli/core/azure_cli.py — __init__ resolves az_command via shutil.which once and stores either the absolute path or None. Absolute paths supplied by callers (test injection) are trusted verbatim; everything else flows through shutil.which so a CWD-relative token like subdir/az cannot bypass resolution. is_available() becomes state-only (no per-call shutil.which), so it can no longer disagree with get_bearer_token()'s pre-check. get_current_tenant_id() gains an explicit early-return when _az_command is None to avoid passing None as argv[0] (previously masked by a broad except).

src/apm_cli/install/pipeline.py — the carve-out at line 160 changes from if is_generic: to if is_generic or is_azure_devops_hostname(host):. _dl.git_env (built by GitAuthEnvBuilder.setup_environment) still forces an empty global gitconfig for the actual clone path; only the single ls-remote probe leg gains GCM access. The bearer-fallback _bearer_op builds its own clean env via _build_git_env(scheme="bearer") and is unaffected.

Diagrams

Cascade before and after the fix — every step is a real call site in the diff.

flowchart LR
    subgraph Before["Before (Windows, az.cmd installed, user logged in)"]
        A1["is_available()<br/>shutil.which → True"] --> B1["subprocess.run(['az', ...])<br/>CreateProcessW: FileNotFoundError"]
        B1 --> C1["AzureCliBearerError<br/>kind=subprocess_error"]
        C1 --> D1["_resolve_token<br/>→ (None, 'none', 'basic')"]
        D1 --> E1["preflight strips<br/>GIT_CONFIG_GLOBAL"]
        E1 --> F1["GCM disabled<br/>ls-remote 401"]
        F1 --> G1["Case 3:<br/>'az present, not logged in'"]
    end
    subgraph After["After"]
        A2["__init__<br/>shutil.which → C:\\...\\az.cmd"] --> B2["subprocess.run(['C:\\...\\az.cmd', ...])<br/>CreateProcessW OK"]
        B2 --> C2["bearer token<br/>preflight passes"]
    end
    style G1 stroke:#d00,stroke-width:2px
    style C2 stroke:#070,stroke-width:2px

Loading

Trade-offs

• is_available() is now construction-time only. PATH changes mid-process won't be observed; restart the CLI if you install az after APM is running. Acceptable: tokens are cached anyway and the provider is a process-wide singleton.
• ADO preflight now consults ~/.gitconfig. A user's insteadOf rewrite could redirect the ls-remote probe to a different host. Mitigations: the probe is read-only; the actual clone path retains full gitconfig isolation; "attacker controls ~/.gitconfig" implies the attacker already owns the git client. Generic hosts already had this carve-out ([BUG] apm install --update fails for GHES/generic hosts due to pipeline probe blocking credential helpers #1082); the security argument applies equally to ADO.
• Unconditional carve-out vs feature flag. Considered an opt-in env var; rejected. The surface is narrow (one read-only probe), GCM-for-ADO is the expected corporate setup, and a flag adds maintenance for negligible upside.

Benefits

1. Windows ADO users on the documented az login path can apm update again.
2. The diagnostic no longer lies — is_available() and get_bearer_token() now agree.
3. GCM remains a viable fallback when bearer acquisition fails for any reason (sandbox, proxy, future PATH quirks).
4. Test injection contract preserved: callers can still pass an absolute path verbatim.

Validation

Note

CI lint mirror green; affected suites all pass locally.

$ uv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/ \
    && uv run --extra dev python -m pylint --disable=all --enable=R0801 --min-similarity-lines=10 \
       --fail-on=R0801 src/apm_cli/ && bash scripts/lint-auth-signals.sh
All checks passed!
1037 files already formatted
Your code has been rated at 10.00/10
[+] auth-signal lint clean

$ uv run --extra dev pytest tests/unit/test_azure_cli.py \
    tests/unit/install/test_pipeline_auth_preflight.py \
    tests/integration/test_ado_preflight_bearer_fallback_e2e.py -q
41 passed in 1.65s

Scenario evidence

User-promise scenario	Test	Principle
Windows az.cmd resolves to subprocess argv[0]	tests/unit/test_azure_cli.py::TestWindowsAzCmdResolution::test_get_bearer_token_invokes_resolved_az_cmd_path	Portability-by-manifest
Pre-fix FileNotFoundError shape no longer surfaces as subprocess_error	tests/unit/test_azure_cli.py::TestWindowsAzCmdResolution::test_bare_az_would_raise_filenotfound_but_resolved_path_succeeds	Determinism
is_available() is consistent with get_bearer_token() pre-check	tests/unit/test_azure_cli.py::TestIsAvailable::test_is_available_stable_after_construction	DevX (no misleading diagnostics)
ADO preflight no longer disables GCM	tests/unit/install/test_pipeline_auth_preflight.py::TestPreflightGenericHostAllowsCredentialHelpers::test_ado_host_omits_credential_blocking_env	Auth-fallback contract
Existing ADO PAT→bearer fallback still works	tests/integration/test_ado_preflight_bearer_fallback_e2e.py (unchanged, passes)	Regression-trap
get_current_tenant_id short-circuits cleanly when az missing	tests/unit/test_azure_cli.py::TestWindowsAzCmdResolution::test_get_current_tenant_id_returns_none_without_subprocess_when_az_missing	Explicit > swallowed exception

How to test

1. On Windows with az.cmd installed and az login completed: clone a repo with a private ADO dep in apm.yml, run apm update. Pre-fix: fails with "az present but not logged in". Post-fix: succeeds.
2. On Windows without az installed but with GCM configured for dev.azure.com: run apm update. Post-fix: GCM resolves the credential and the probe passes.
3. On Linux/macOS: run the focused suite — uv run --extra dev pytest tests/unit/test_azure_cli.py tests/unit/install/test_pipeline_auth_preflight.py tests/integration/test_ado_preflight_bearer_fallback_e2e.py.
4. Lint mirror — uv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/.

Closes #1430.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[Copilot]

Pull request overview

Fixes a Windows-specific Azure DevOps authentication failure during apm update by making Azure CLI invocation deterministic (resolving az via shutil.which so az.cmd works) and by relaxing the ADO preflight probe environment to preserve Git Credential Manager as a fallback channel.

Changes:

• Resolve and store the az executable path at AzureCliBearerProvider construction time and reuse it for all subprocess calls.
• Expand the preflight probe environment carve-out to include Azure DevOps hosts so credential helpers (notably GCM) can participate in the ls-remote probe.
• Add/adjust unit tests covering Windows az.cmd resolution behavior and the updated preflight environment expectations; add a changelog entry.

Show a summary per file

File	Description
src/apm_cli/core/azure_cli.py	Resolves az once at construction and uses the resolved path for subprocess calls; makes is_available() state-based and adds a guard in get_current_tenant_id().
src/apm_cli/install/pipeline.py	Widens the preflight env carve-out to also apply to Azure DevOps hosts to preserve credential helper behavior for the probe.
tests/unit/test_azure_cli.py	Adds regression tests for Windows az.cmd resolution and stability of is_available() after construction.
tests/unit/install/test_pipeline_auth_preflight.py	Updates/renames the ADO preflight env test to assert credential-blocking vars are omitted.
CHANGELOG.md	Adds an Unreleased Fixed entry documenting the behavior change.

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 3