fix(auth): thread dep_ref.port into credential resolution (#785)

[edenfunf]

Description

Closes #785. Follow-up from the #665 review panel's auth-expert finding and the design discussion on the issue thread.

DependencyReference.port was parsed and stored by #665 but never reached the authentication path. Same-host multi-port setups (e.g. Bitbucket Datacenter with distinct PATs on 7990 and 7991) therefore collapsed onto a single credential entry via the AuthResolver cache, causing APM's pre-resolved token path (Method 1: authenticated HTTPS) to return the wrong credential.

The fix threads port through every layer that identifies a credential target: HostInfo, both in-process caches, git credential fill stdin, and user-facing error output.

What changed

• HostInfo gains a port: Optional[int] = None field and a display_name property ("host:port" if port else host). Classification stays transport-agnostic -- classify_host carries the port through but never uses it to decide kind.
• AuthResolver._cache key widens to (host.lower(), port, org.lower() if org else ""). port=None preserves the pre-fix key for the >99% common case; multi-port setups get per-port entries. Lock semantics unchanged.
• TokenManager._credential_cache is keyed by (host, port) tuple. Both caches widen together -- a half-widened cache would return the wrong cached credential at the collapsed layer.
• git credential fill receives host=host:port per gitcredentials(7). The credential protocol has no standalone port= attribute; sending one would be silently ignored by every helper. The _format_credential_host helper and an anti-pattern test guard against this regression.
• resolve_for_dep threads dep_ref.port; github_downloader, validation, and sources call sites forward the port argument.
• build_error_context uses host_info.display_name in the failure summary and, when a port is set, appends one [i] hint pointing users at credential helpers that key by hostname only.
• Docs -- docs/getting-started/authentication.md gains a per-helper port-awareness table under the "Git credential helper not found" section.
• CHANGELOG entry under [Unreleased] / Fixed.

Lockfile identity is unaffected -- DependencyReference.get_unique_key() still excludes port by design; the widened cache keys are purely in-process memoisation.

Type of change

• Bug fix
• New feature
• Documentation
• Maintenance / refactor

Testing

• Tested locally
• All existing tests pass (4315 unit tests; 4 pre-existing failures unrelated to this change)
• Added tests for new functionality

New test coverage (23 cases)

• TestHostInfoPort -- field, display_name, classify_host port attachment + transport-agnostic invariant.
• TestResolvePortDiscrimination -- same host / different port → separate entries; same (host, port, org) hits cache; port=None vs port=<int> are distinct; resolve_for_dep threads port end-to-end for both ssh:// and https:// URLs; HostInfo carries port on resolve.
• TestBuildErrorContextWithPort -- display_name in error; [i] hint appears when port is set; no hint when port is absent.
• TestTryWithFallbackWithPort -- port flows into the credential-fill fallback path.
• TestCredentialFillPortEmbedding -- host=host:port stdin format; no standalone port= line (anti-pattern guard); backward-compat with bare host when port=None.
• TestGetTokenWithCredentialFallback gains same-host-different-port + same-host-same-port cache tests.

Manual end-to-end verification

Ran a scripted check covering: ssh/https port capture, resolve_for_dep propagation, cache-hit behaviour, git credential fill stdin format, TokenManager cache segmentation, and error-context hint presence/absence. All six scenarios pass.

Notes for reviewers

• Per the panel feedback, proposal item Add ARM64 Linux support to CI/CD pipeline #4 in the original issue (a standalone port= line in git credential fill stdin) is not implemented -- the port is embedded into the host field instead, per gitcredentials(7). An anti-pattern regression test asserts no \nport= ever appears in the stdin.
• classify_host accepts port as a kwarg and attaches it to the returned HostInfo. Classification logic itself remains transport-agnostic as per the architect's note; the kwarg is purely metadata.
• Port-unaware call sites elsewhere in the codebase (policy/discovery.py, marketplace/client.py, registry_proxy.py) are intentionally left alone -- they either do not parse a dep_ref (policy discovery) or only use host_info.kind/api_base for classification, where port is irrelevant.

Refs: #661, #665, #784

[edenfunf]

@danielmeppiel — ready for review when you have a cycle. Scope is the 9-item PR spec from your review-panel comment on #785 (standalone PR, both caches widened, host=host:port per gitcredentials(7), display_name used at the three call sites, [i] hint, docs table, CHANGELOG). Manual e2e + 23 new unit tests all green. Thanks!

[danielmeppiel]

APM Review Panel — PR #788

[+] Verdict: APPROVE — merge as-is, file two follow-ups

@edenfunf delivered a faithful 9-for-9 execution on the panel ask from #785. Four specialists (python-architect, auth-expert, cli-logging-expert, devx-ux-expert) converge on mergeable. The only "blocker" surfaced (architect flag on validation.py:326/335) was verified as a false positive — those line numbers fall in the parse-failure fallback path where port is genuinely unavailable and the assumed host is github.com (which doesn't use custom ports).

What's excellent

• Faithful scope discipline. All 9 panel asks landed; zero scope creep, zero drive-by refactors.
• Test quality. 23 behavior-targeted tests with the anti-pattern guard assert "\nport=" not in sent — exactly the right shape for a gitcredentials(7) protocol contract.
• Surface design. display_name as a @property with truthy-port guard keeps the happy path invisible and the error path materially better for Bitbucket DC users.

Findings

1. [i] Style nit — auth.py:309 (auth-expert). _try_credential_fallback uses closure host instead of host_info.host. Behaviorally identical today, drift risk tomorrow. Prefer host=host_info.host, port=host_info.port for symmetry with _resolve_token at line 462. Non-blocking — fix in this PR if trivial, else follow-up.
2. [!] Default-port normalization risk (devx-ux-expert). display_name truthy-checks self.port, so HostInfo(host="github.com", port=443) would render github.com:443 and incorrectly trigger the [i] hint. Confirm the dep_ref parser stores None for default-scheme ports (443/80/22), or normalize in HostInfo.__post_init__. Add a unit test pinning the contract. Follow-up issue — low likelihood in practice given current parser, but cheap to lock down.
3. [i] Pre-existing gap — github_downloader.py:808-811 (cli-logging-expert). is_generic error path renders bare host, losing port for Bitbucket DC users. Not introduced by this PR — file as follow-up.
4. [i] [i] hint actionability ~70% (devx-ux-expert). "Verify your credential helper stores per-port entries" names the suspect but not the next action. Follow-up — add a concrete git credential-manager get snippet or doc anchor in a polish pass.
5. [i] Cosmetic — cache-key None → "" (auth-expert). Behaviorally identical. One-line normalization comment would help future readers. Optional, non-blocking.

Merge call

Ship it. Open follow-ups for #2, #3, #4. @edenfunf — this is the standard for contributor work on a multi-part ask: read the full thread, execute every item, write the tests that prove the contract. Thank you for the precision. [+]

---

Growth angle

• Enterprise/Bitbucket-DC segment is self-organizing. @edenfunf's chain (review fix: preserve protocol (e.g. ssh:// or https://) and port in dependency URL #665 → PRs [BUG] SCP shorthand with numeric port silently misparsed as repo path #784/fix: detect port-like segment in SCP shorthand and suggest ssh:// form #787 → issue [FEATURE] Thread dep_ref.port into credential resolution (git credential fill) #785 → PR fix(auth): thread dep_ref.port into credential resolution (#785) #788) is the signature of a real on-prem user with sustained pain, not a drive-by. A repeat external contributor specifically hardening Bitbucket Data Center auth means APM is being adopted inside firewalled enterprises faster than our docs acknowledge — worth a dedicated "APM on Bitbucket DC" guide and probing whether a repeatable persona exists behind them.
• Story-shaped CHANGELOG line. "External contributor diagnosed and fixed an enterprise auth bug end-to-end — issue, fix, anti-regression tests, docs, self-credited" is exactly the narrative that recruits the next contributor. Frame it in release notes as "community-shipped" with @edenfunf credited in the headline, not just the trailer.
• Flag for next maintainer comms. Three+ merged PRs and a faithful 9-item panel execution clears the bar for a public shoutout (release notes highlight + contributor-spotlight post) and an internal nudge: invite to the contributors channel, consider triage permissions, watch for the fourth PR.

_{Reviewed via the apm-review-panel skill: python-architect, auth-expert, cli-logging-expert, devx-ux-expert; synthesized by apm-ceo with oss-growth-hacker side-channel.}

[Copilot]

Pull request overview

Threads DependencyReference.port through APM’s auth/credential resolution so same-host multi-port setups don’t collide in in-process caches or git credential fill lookups.

Changes:

• Add port to HostInfo, widen AuthResolver cache keys, and propagate port through resolve/fallback/error-context paths.
• Embed port into git credential fill as host=host:port and segregate GitHubTokenManager credential cache by (host, port).
• Add/adjust unit tests, update auth docs, and add a changelog entry.

Show a summary per file

File	Description
src/apm_cli/core/auth.py	Adds port-aware HostInfo, port-threading through auth resolution, cache key widening, and port-aware error messaging.
src/apm_cli/core/token_manager.py	Embeds port into git-credential host field and keys credential cache by (host, port).
src/apm_cli/deps/github_downloader.py	Threads dep_ref.port into auth error-context generation and GitHub file auth resolution.
src/apm_cli/install/validation.py	Threads dep_ref.port into auth classification, resolution, fallback, and verbose diagnostics.
src/apm_cli/install/sources.py	Threads dep_ref.port into auth resolution used for package auth logging.
tests/unit/test_auth.py	Adds coverage for port propagation, cache discrimination, error context display/hints, and fallback behavior.
tests/test_token_manager.py	Updates existing assertions and adds tests for (host, port) credential cache + credential-fill stdin format.
tests/test_github_downloader.py	Adjusts mocks for new resolve_credential_from_git(host, port=...) signature.
docs/src/content/docs/getting-started/authentication.md	Documents port-in-host behavior and helper port-awareness caveats.
CHANGELOG.md	Adds an Unreleased “Fixed” entry for port-discriminated token resolution.

Copilot's findings

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

[Copilot]

AuthResolver cache key changed for org=None: the old code used org=None in the tuple, but this now normalizes to an empty string. That means port=None does not actually preserve the pre-existing key shape and also collapses org=None with org="". Consider keeping None for missing org (and only lowercasing when org is not None), or update the PR description/comments to match the new behavior.

[Copilot]

HostInfo.display_name uses truthiness (if self.port) to decide whether to include the port. For correctness and consistency with other checks in this file (host_info.port is not None), consider using an explicit is not None check so port values like 0 (or other falsey ints) are not silently treated as absent.

[Copilot]

_format_credential_host() uses if port which treats falsey integers as absent. Using port is not None is safer and matches the rest of the port-handling logic (where None is the sentinel for "no port").

Suggested change

	return f"{host}:{port}" if port else host
	return f"{host}:{port}" if port is not None else host

[Copilot]

This doc update changes user guidance around git credential helpers for custom-port hosts. Per the repo docs workflow, please mirror this information (at least the host:port behavior / helper caveat) into packages/apm-guide/.apm/skills/apm-usage/authentication.md so the in-repo APM usage skill stays consistent with the public docs.

[danielmeppiel]

@edenfunf check Copilot Review and CodeQL scan alert and I will approve after that's done, thanks!