feat: Auth + Logging Architecture Overhaul (#393)

[danielmeppiel]

Auth + Logging Architecture Overhaul

Closes #393 and #327. Subsumes #389 (credential timeout fix — credit to @frblondin).

Problem

1. Auth was scattered: 4 different call sites reimplemented token lookup. EMU/enterprise packages silently failed validation because git ls-remote ran without auth.
2. Logging was ad-hoc: 766+ output calls across 27 files with no shared lifecycle. Misleading counts, contradictory symbols, MCP noise on partial install.
3. Global env var host-gating: Global tokens were restricted to the "default host" — framed as a security constraint but HTTPS is the real transport boundary. This forced users into per-org token setups unnecessarily.

Solution

AuthResolver (src/apm_cli/core/auth.py)

• Single source of truth for auth: per-(host, org) resolution, cached, thread-safe
• Resolution chain: per-org env var → global env vars → git credential fill
• Global env vars apply to all hosts (no host-gating). When a token doesn't work for a particular host, try_with_fallback() retries with git credential fill automatically
• try_with_fallback(): unauth-first for validation (saves rate limits), auth-first for downloads, credential-fill fallback on failure

CommandLogger (src/apm_cli/core/command_logger.py)

• Base class for all CLI command output — delegates to _rich_* helpers
• InstallLogger subclass with validation/resolution/download phases
• Short-circuits on total validation failure (no confusing "0 dependencies" messages)
• All 13 commands wired to use logger

Auth Flow

flowchart TD
    A[Dependency Reference] --> B{Per-org env var?}
    B -->|GITHUB_APM_PAT_ORG| C[Use per-org token]
    B -->|Not set| D{Global env var?}
    D -->|GITHUB_APM_PAT / GITHUB_TOKEN / GH_TOKEN| E[Use global token]
    D -->|Not set| F{Git credential fill?}
    F -->|Found| G[Use credential]
    F -->|Not found| H[No token]

    E --> I{try_with_fallback}
    C --> I
    G --> I
    H --> I

    I -->|Token works| J[Success]
    I -->|Token fails| K{Credential-fill fallback}
    K -->|Found credential| J
    K -->|No credential| L{Host has public repos?}
    L -->|Yes| M[Try unauthenticated]
    L -->|No| N[Auth error with actionable message]

Loading

Provenance Matrix

Package source	Host	Auth behavior	Fallback
org/repo (bare)	default_host()	Global env vars → credential fill	Unauth for public repos
github.com/org/repo	github.com	Global env vars → credential fill	Unauth for public repos
contoso.ghe.com/org/repo	*.ghe.com	Global env vars → credential fill fallback	Auth-only (no public repos)
GHES via GITHUB_HOST	ghes.company.com	Global env vars → credential fill	Unauth for public repos
dev.azure.com/org/proj/repo	ADO	ADO_APM_PAT only	Auth-only
Multi-org on any host	Per-org	GITHUB_APM_PAT_{ORG} → global → credential fill	Per host type

Token Prefix Classification

Prefix	Type	Origin
github_pat_	Fine-grained PAT	github.com settings
ghp_	Classic PAT	github.com settings
ghu_	OAuth user-to-server	gh auth login, OAuth apps
gho_	OAuth app	GitHub OAuth apps
ghs_	GitHub App installation	Server-to-server
ghr_	GitHub App refresh	Token refresh

EMU note: Enterprise Managed Users use standard PAT prefixes (ghp_, github_pat_). There is no EMU-specific prefix — APM cannot detect EMU from a token.

Key Design Decisions

1. HTTPS is the security boundary — all token-bearing requests use HTTPS. No host-gating needed.
2. Credential-fill fallback — when a global env-var token fails (e.g., github.com PAT on *.ghe.com), try_with_fallback() retries with git credential fill. This makes gh auth login for multiple hosts work seamlessly.
3. Per-org tokens take highest priority — GITHUB_APM_PAT_{ORG} always wins regardless of host.
4. Credential timeout: 5s → 60s (configurable via APM_GIT_CREDENTIAL_TIMEOUT, max 180s). Fixes Windows credential picker timeouts.

Commits

Commit	Description
6366f9b	AuthResolver + CommandLogger foundation with full auth wiring
fdc7d08	InstallLogger wiring, auth docs, CHANGELOG, test updates
6ef2511	CommandLogger wiring across all commands and support modules
c355d02	EMU docs accuracy, credential timeout (subsumes #389)
6ddaea4	Correct token prefix mapping — EMU uses standard PAT prefixes
37978ac	Remove host-gating on global env vars, add credential-fill fallback

Test Coverage

• 2851 tests passing (up from 2829 on main), 127 skipped, 0 failures
• 35 unit tests for AuthResolver (classify_host, detect_token_type, resolve, try_with_fallback, credential fallback)
• 44 unit tests for CommandLogger
• 27 integration tests for AuthResolver (end-to-end with env manipulation)
• 11 diagnostics tests

[Copilot]

Pull request overview

This PR introduces two cross-cutting foundations—AuthResolver (per-(host, org) auth resolution) and CommandLogger / InstallLogger (structured CLI lifecycle logging)—and wires them through CLI commands and auth touchpoints to fix EMU/private repo installs, reduce confusing output, and prevent token leakage across hosts.

Changes:

• Add AuthResolver + host classification + fallback strategies; integrate into clone/download/validation paths.
• Add CommandLogger/InstallLogger and thread diagnostics through commands and supporting modules.
• Expand diagnostics with CATEGORY_AUTH, update docs/changelog, and adjust/add unit & integration tests.

Reviewed changes

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

Show a summary per file

File	Description
tests/unit/test_unpacker.py	Updates dry-run output assertion to match new logging wording.
tests/unit/test_install_command.py	Updates invalid-format assertion to match updated validation messaging.
tests/unit/test_diagnostics.py	Adds coverage for new CATEGORY_AUTH diagnostics group rendering/counting.
tests/unit/test_dev_dependencies.py	Adapts to new (validated, outcome) return from validation helper.
tests/unit/test_command_logger.py	New unit tests for CommandLogger, InstallLogger, _ValidationOutcome.
tests/unit/test_canonicalization.py	Updates expectations for new validation helper return type.
tests/unit/test_auth_scoping.py	Adjusts scoping tests to align with per-dep auth resolution & cache.
tests/unit/test_auth.py	New unit tests for AuthResolver (classification, caching, fallback, errors).
tests/unit/test_audit_command.py	Updates audit helper calls to pass CommandLogger.
tests/test_token_manager.py	Adds tests for configurable credential-helper timeout behavior.
tests/test_github_downloader_token_precedence.py	Fixes test imports to use installed package paths.
tests/integration/test_generic_git_url_install.py	Updates expectations for new validation helper return type.
tests/integration/test_auth_resolver.py	New integration tests gated by APM_E2E_TESTS.
src/apm_cli/utils/diagnostics.py	Adds CATEGORY_AUTH + rendering and count helpers.
src/apm_cli/registry/operations.py	Uses token manager precedence when auto-filling env vars in CI/E2E mode.
src/apm_cli/integration/skill_integrator.py	Threads optional logger into skill integration warnings/diagnostics.
src/apm_cli/integration/prompt_integrator.py	Adds optional logger parameter for structured output compatibility.
src/apm_cli/integration/mcp_integrator.py	Threads optional logger through MCP install/remove flows; renames module logger to _log.
src/apm_cli/integration/instruction_integrator.py	Adds optional logger parameter for structured output compatibility.
src/apm_cli/drift.py	Threads optional logger through drift helpers.
src/apm_cli/deps/github_downloader.py	Switches to AuthResolver and per-dependency token resolution for clone/download/error context.
src/apm_cli/core/token_manager.py	Makes git credential fill timeout configurable; default 60s, max 180s.
src/apm_cli/core/command_logger.py	New structured logging base (CommandLogger) and install-specific logger (InstallLogger).
src/apm_cli/core/auth.py	New AuthResolver/AuthContext/HostInfo with caching + fallback strategies.
src/apm_cli/core/init.py	Exposes auth types from apm_cli.core.
src/apm_cli/compilation/agents_compiler.py	Adds optional logger plumbing for compilation output instead of raw print().
src/apm_cli/commands/update.py	Migrates update command output to CommandLogger.
src/apm_cli/commands/uninstall/cli.py	Migrates uninstall command output to CommandLogger.
src/apm_cli/commands/runtime.py	Migrates runtime command output to CommandLogger.
src/apm_cli/commands/run.py	Adds --verbose and migrates output to CommandLogger.
src/apm_cli/commands/prune.py	Migrates prune command output to CommandLogger.
src/apm_cli/commands/pack.py	Migrates pack/unpack output to CommandLogger (incl. dry-run messaging).
src/apm_cli/commands/mcp.py	Adds --verbose and migrates MCP commands output to CommandLogger.
src/apm_cli/commands/list_cmd.py	Migrates list command output to CommandLogger.
src/apm_cli/commands/install.py	Integrates InstallLogger, introduces validation outcome tracking, and auth-aware validation (try_with_fallback).
src/apm_cli/commands/init.py	Adds --verbose and migrates init output to CommandLogger.
src/apm_cli/commands/deps/cli.py	Migrates deps subcommands output to CommandLogger.
src/apm_cli/commands/deps/_utils.py	Threads optional logger into deps update helpers and removes direct console helper usage.
src/apm_cli/commands/config.py	Migrates config command output to CommandLogger.
src/apm_cli/commands/compile/cli.py	Migrates compile output to CommandLogger and passes logger into compiler.
src/apm_cli/commands/audit.py	Migrates audit output to CommandLogger; threads logger into helpers.
src/apm_cli/adapters/client/vscode.py	Threads optional logger through VS Code MCP config operations.
src/apm_cli/adapters/client/copilot.py	Uses token manager for Copilot header token selection.
docs/src/content/docs/guides/private-packages.md	Updates links to auth + git reference docs.
docs/src/content/docs/getting-started/authentication.md	Rewrites auth docs to describe per-(host, org) resolution and security constraint.
CHANGELOG.md	Adds Keep-a-Changelog entry for new auth/logging architecture and timeout fix.
.github/skills/python-architecture/SKILL.md	Adds skill trigger guidance for architecture changes.
.github/skills/cli-logging-ux/SKILL.md	Updates logging UX skill to include CommandLogger rules.
.github/skills/auth/SKILL.md	Adds skill trigger guidance and “AuthResolver-only” rule.
.github/agents/python-architect.agent.md	Adds agent persona for architecture review.
.github/agents/cli-logging-expert.agent.md	Adds agent persona for CLI logging UX review.
.github/agents/auth-expert.agent.md	Adds agent persona for authentication review.

[Copilot]

InstallLogger.resolution_start() is called with to_install_count=len(packages), which counts user-supplied arguments rather than new packages to install. If the package was already present in apm.yml, the logger will still print "Installing 1 new package...", which is misleading. Use the validation outcome (e.g., len(outcome.new_packages)) or the actual resolved install set for this count.

[danielmeppiel]

Fixed in efa13c1 — now uses len(validated_packages) instead of len(packages), so the count reflects packages that actually passed validation.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

src/apm_cli/core/command_logger.py:174

• InstallLogger.validation_pass()/validation_fail() embed Unicode check/cross characters directly in the output strings. Since the console helpers already support ASCII-safe symbols via symbol=... (see STATUS_SYMBOLS), consider switching these to use symbol instead of inline glyphs to keep output consistent and Windows-friendly.

    def validation_pass(self, canonical: str, already_present: bool):
        """Log a package that passed validation."""
        if already_present:
            _rich_echo(f"  ✓ {canonical} (already in apm.yml)", color="dim")
        else:
            _rich_success(f"  ✓ {canonical}")

    def validation_fail(self, package: str, reason: str):
        """Log a package that failed validation."""
        _rich_error(f"  ✗ {package} — {reason}")

[Copilot]

In CI/E2E mode, token/key variables are defaulted from GitHubTokenManager().get_token_for_purpose('modules') regardless of which env var is being requested. This can populate the wrong kind of token (e.g., a GitHub modules PAT into a variable that expects GITHUB_COPILOT_PAT, ADO_APM_PAT, or other service-specific keys). Consider mapping var_name to an appropriate token purpose (or leaving it empty unless it matches known GitHub module token vars) to avoid misconfiguring MCP server env.

[Copilot]

In AuthResolver._resolve_token(), the per-org env var GITHUB_APM_PAT_{ORG} is applied unconditionally for any host, including Azure DevOps. This can cause ADO dependencies to pick up a GitHub PAT (because ADO org is derived from the repo path) even though ADO auth should come only from ADO_APM_PAT, leading to guaranteed auth failures/confusing behavior. Gate the per-org GitHub env var to GitHub-like hosts only (github/ghe_cloud/ghes), or introduce an ADO-specific per-org mechanism if needed, but avoid using GITHUB_APM_PAT_* for host_info.kind == 'ado'.

[Copilot]

AuthResolver.resolve() caches results by the raw (host, org) tuple without normalizing case. Since host classification is case-insensitive, calling resolve('GitHub.COM') vs resolve('github.com') will create duplicate cache entries and can lead to inconsistent behavior in callers that pass differently-cased hosts. Consider normalizing host (and optionally org) for the cache key (e.g., host.lower() and a consistent org normalization) while still preserving the original host string in HostInfo if needed for display.

Suggested change

	key = (host, org)
	# Normalize the cache key to avoid case-sensitive duplication.
	key_host = host.lower() if host is not None else None
	key_org = org.lower() if org is not None else None
	key = (key_host, key_org)

[Copilot]

When logger is provided and dry_run is true, the not validated_packages branch suppresses all output (... if not logger else None). That makes apm install --dry-run <pkg> silent in the "nothing new to add" case. Use the logger to emit an info/warning (e.g., via logger.progress()/logger.warning()) so users still get feedback in dry-run mode.

Suggested change

	_rich_warning("No new packages to add") if not logger else None
	if logger:
	logger.progress(
	"Dry run: No new packages to add; all requested packages are already in apm.yml"
	)
	else:
	_rich_warning("No new packages to add")

[Copilot]

CommandLogger.auth_step() hardcodes Unicode status glyphs ("✓"/"✗") into the message string. This conflicts with the codebase’s ASCII-safe symbol approach (STATUS_SYMBOLS via the symbol= parameter) and can render poorly on Windows/codepages. Prefer using symbol="check"/symbol="error" (or similar) and keep the message text free of inline glyphs.

This issue also appears on line 164 of the same file.

Suggested change

	status = "✓" if success else "✗"
	msg = f" auth: {status} {step}"
	if detail:
	msg += f" ({detail})"
	_rich_echo(msg, color="dim")
	msg = f" auth: {step}"
	if detail:
	msg += f" ({detail})"
	symbol = "check" if success else "error"
	_rich_echo(msg, color="dim", symbol=symbol)