[FEATURE] Thread dep_ref.port into credential resolution (git credential fill)

[edenfunf]

Problem

DependencyReference.port is parsed and threaded through URL construction (#665), but is never propagated into the credential resolution path. git credential fill supports a port= field, but APM always sends bare hostname only:

# token_manager.py:111
input=f"protocol=https\nhost={host}\n\n"   # no port= line

Affected code path:

Layer	File	Gap
Auth resolver	core/auth.py:210	resolve_for_dep() reads dep_ref.host, ignores dep_ref.port
Cache key	core/auth.py:183	(host.lower(), org.lower()) — no port dimension
Host model	core/auth.py:56	HostInfo has no port field
Credential fill	core/token_manager.py:111	git credential fill input omits port=

Impact

When a user has the same hostname serving git on different ports with different credentials (e.g. bitbucket.corp.com:7999 for SSH and bitbucket.corp.com:7990 for HTTPS, each requiring distinct PATs), APM's pre-resolved token path (Method 1: authenticated HTTPS) may resolve the wrong credential.

Mitigating factor: for non-GitHub hosts, APM's token resolution typically returns None, and git's own clone process queries credential helpers with the full URL including port — so the direct clone path handles this correctly. The gap only affects the pre-resolved token path in _resolve_token.

Suggested fix

1. HostInfo — add port

@dataclass(frozen=True)
class HostInfo:
    host: str
    port: Optional[int] = None  # Non-standard git port
    kind: str
    has_public_repos: bool
    api_base: str

2. AuthResolver — port-aware cache key

key = (host.lower(), port, org.lower() if org else "")

3. resolve_for_dep — thread port

def resolve_for_dep(self, dep_ref):
    host = dep_ref.host or default_host()
    port = dep_ref.port
    org = dep_ref.repo_url.split("/")[0] if dep_ref.repo_url else None
    return self.resolve(host, org, port=port)

4. resolve_credential_from_git — pass port to git credential protocol

input_str = f"protocol=https\nhost={host}\n"
if port:
    input_str += f"port={port}\n"
input_str += "\n"

All new parameters default to None — fully backward compatible.

Limitation worth documenting

Not all credential helpers honor the port field. OS keychains and gh auth typically store credentials per-hostname. APM can pass the information correctly, but per-port discrimination ultimately depends on the helper implementation. A one-line note in docs/getting-started/authentication.md under "Git credential helper not found" would set expectations.

Context

Follow-up from PR #665 review (auth-expert panel finding). The port field was introduced on DependencyReference and LockedDependency but was only wired into URL construction, not credential resolution.

Refs: #661, #665

[edenfunf]

@danielmeppiel This is a follow-up from the auth-expert finding in the #665 review panel. Wanted to get your take before starting work.

The gap is narrow but real: DependencyReference.port is parsed and threaded through URL construction, but never reaches resolve_credential_from_git or the AuthResolver cache key. The git credential protocol supports a port= field — we're just not sending it.

That said, the practical impact is limited. For non-GitHub hosts, _resolve_token typically returns None and git's own clone process queries credential helpers with the full URL (including port) — so the direct clone path already handles this correctly. The gap only affects the pre-resolved token path (Method 1: authenticated HTTPS with a token APM resolved in advance).

My read is this is a low-risk, backward-compatible fix (all new params default to None), but it touches HostInfo, the AuthResolver cache key, and token_manager — which are core auth surfaces you may prefer to batch with other auth work rather than change piecemeal.

Two questions:

1. Is this worth fixing standalone, or would you rather fold it into a broader auth refactor?
2. Any concern about changing the cache key from (host, org) to (host, port, org)? The only observable difference is that resolve() would cache separately for the same host on different ports — which is the correct behavior but technically a cache-miss increase for the (extremely rare) multi-port scenario.

Happy to either submit a PR or hold off based on your preference.

[danielmeppiel]

Thanks @edenfunf — ran this through the full review panel (architect + auth + CLI logging + DevX UX + CEO + growth). Direct answers and PR scope below.

---

Direct answers

Q1 — Standalone PR or fold into broader refactor?
Standalone. No pending auth refactor to batch with; the gap is a real, user-visible correctness bug. Delaying it helps nobody.

Q2 — Concern about cache key (host, org) → (host, port, org)?
No concern — the current key is the bug, the widened key is the fix.

• Semantically correct: git credential helpers genuinely distinguish by port.
• Zero cache-miss delta in the >99% case (port=None → same key as today).
• Extra miss in multi-port case is required for correctness, not a regression.
• Lockfile identity is unaffected — DependencyReference.get_unique_key() returns bare repo_url (port excluded by design). The cache key is purely in-process memoization for AuthResolver.resolve().
• Hold-lock-during-resolution pattern stays safe (single global self._lock; widening the key reduces collisions, doesn't change lock semantics).

---

PR scope

One correction to the proposal — please read before coding

Proposed item #4 (adding a standalone port={n} line to git credential fill stdin) is not correct per gitcredentials(7). There is no standalone port= attribute in the credential protocol — port must be embedded in the host field:

# In token_manager.py:111 — correct form:
host_str = f"{host}:{port}" if port else host
input_str = f"protocol=https\nhost={host_str}\n\n"

Shipping the standalone-port= line produces a silently broken helper integration on every platform. Easy to miss — one of those gitcredentials(7) gotchas.

Required changes (all backward-compatible)

1. HostInfo — add port: Optional[int] = None to the frozen dataclass.

2. AuthResolver._cache key — widen to (host.lower(), port, org.lower() if org else "") at auth.py:183.

3. TokenManager._credential_cache key — widen at token_manager.py:53 as well. A half-widened cache is worse than none — one layer would discriminate, the other would collapse and return the wrong cached result. Both go together.

4. resolve_for_dep — thread dep_ref.port to resolve() via new kwarg with default None.

5. git credential fill — embed port in the host field (see correction above).

6. HostInfo.display_name property — f"{self.host}:{self.port}" if self.port else self.host. Use it at auth.py:264, auth.py:272, and especially auth.py:316 (build_error_context, user-facing error text) so errors actually mention the port when port is what failed.

7. Error-path hint — on auth failure when dep_ref.port is not None, append one line:

   [i] Host '{host}:{port}' -- verify your credential helper stores per-port entries (some helpers key by host only).
   Gives users a concrete next action when the helper itself drops the port.

8. Doc note in docs/getting-started/authentication.md under "Git credential helper not found":

   Helper	Honors port-in-host?
   git-credential-manager (GCM)	Yes
   macOS Keychain (osxkeychain)	Yes (stores full host:port as key)
   libsecret (Linux)	Yes (port in URI)
   gh auth git-credential	No, but only applies to GitHub hosts which don't use custom ports

9. CHANGELOG [Unreleased] → Fixed:

   - Token resolution now discriminates by port, fixing credential collisions across multiple self-hosted Git instances on the same host. Thanks @edenfunf! (#785)

Optional (do not block PR): verbose-only log line inside resolve() when port discriminates the cache key.

---

Additional panel notes

• Architect: pass port via kwarg with default None; don't thread through classify_host — host-kind classification (github / ghe / ado / generic) is transport-agnostic.
• Auth: your mitigating-factor reasoning is sound — pre-resolved token path is only hit when env PATs are set, which is rare for custom-port Bitbucket/Gitea. Real bug, low frequency, correct fix.
• DevX: prior art — npm (full registry URL in _auth scope), pip (keyring per-URL), cargo (full index URL) are all port-aware by construction. APM being host-only was the outlier; this fix aligns us.

---

Strategic fit

Reinforces "strict-by-default, explicit beats magic" — the package manager correctly distinguishes host:8080 from host:9090 at every layer. Being the outlier here was a defect, not a design choice. Ship it.

If items #6–#7 feel like scope creep, happy to split into a follow-up and merge the core 5-file fix first. Your call.

Thanks for driving the auth layer forward — #665 review → #784 → #787 → #785 is genuinely exceptional contributor velocity and appreciated.