refactor(api): extract ResolveActiveAuth helper, eliminate auth-precedence duplication (KLA-447)

[jklaassenjc]

Summary

KLA-447 — the architectural cleanup motivated by KLA-439 / PR #42's Bugbot review trail.

KLA-439 accumulated 13 Bugbot findings, all of one structural class: `collectAuth` in `cmd/doctor.go` was a hand-rolled mirror of `api.NewClient()`'s auth-resolution precedence, and the two implementations drifted at every edge. Each fix exposed the next narrower one because the underlying duplication remained.

This change pulls auth resolution into the api package as the single source of truth. Future maintainers cannot reintroduce a drift here because the precedence walking lives in one place.

Changes

File	Change
internal/api/auth_resolve.go (new, 267 lines)	`ResolveActiveAuth(Hint) ResolvedAuth` with the canonical decision tree. `Hint` lets cmd-package callers pass cobra pflag `.Changed` bits through without the api package importing pflag.
internal/api/client.go	`NewClient` becomes a 7-line consumer of `ResolveActiveAuth` — same decision tree, no duplicate walking. api package drops the `config` import.
internal/cmd/doctor.go	`collectAuth` shrinks from 110 lines to 9 — pure presentation adapter.
internal/api/auth_resolve_test.go (new, 310 lines)	14 tests covering every Bugbot-found edge against `ResolveActiveAuth`, plus the cross-package parity check.

Diff: 4 files, +614 / -160. Net ~130 lines removed from doctor; gained a focused single-source-of-truth resolver with exhaustive tests.

Behavior preservation

All 16 existing doctor tests pass unchanged. That's the bar — the refactor must not regress any of the 13 Bugbot edges:

```
=== RUN TestCollectAuth_JCAPIKeyEnvWins PASS
=== RUN TestCollectAuth_FlagBeatsEnvEvenWhenEqual PASS
=== RUN TestCollectAuth_ServiceAccountReportsOAuth PASS
=== RUN TestCollectAuth_ServiceAccountWinsOverStrayEnvKey PASS
=== RUN TestCollectAuth_ServiceAccountMissingClientCreds PASS
=== RUN TestCollectAuth_ServiceAccountKeychainSecretMissing PASS
=== RUN TestCollectAuth_ServiceAccountKeychainFailureFallsBackToAPIKey PASS
=== RUN TestCollectAuth_ServiceAccountFallsBackToAPIKey PASS
=== RUN TestCollectAuth_KeychainReference PASS
=== RUN TestCollectAuth_PlaintextProfileConfig PASS
=== RUN TestCollectAuth_Unset PASS
=== RUN TestCollectProfile_* PASS (3)
=== RUN TestCollectOrgID_* PASS (2)
```

Bugbot #13 proof-point

The acceptance criteria called for a cross-package parity test. Bugbot finding #13 (`BUGBOT_BUG_ID fef17827`) was the exact case where the duplicated precedence was structurally wrong: doctor checked raw viper values, NewClient checked resolved values.

Two new paired tests in `auth_resolve_test.go`:

• `TestResolveActiveAuth_KeychainRefAPIKeyDoesntCountAsFallback` — service_account profile + unresolvable `api_key: keychain://...` → ResolveActiveAuth does NOT claim "api_key fallback" (because the keychain ref doesn't resolve to a real key).
• `TestNewClient_KeychainRefAPIKeyReturnsErrNoAPIKey` — same fixture, NewClient returns `ErrNoAPIKey`.

The two layers now agree by construction. Pre-refactor, doctor said "fallback" while NewClient said "no auth."

Test plan

• `go test ./... -count=1` full suite green
• `go vet ./...` clean
• 16 doctor tests pass unchanged (behavior preservation)
• 14 new api-package tests cover every precedence branch
• Live: `jc doctor --no-probe --json` against real config produces honest output
• On merge: tag `1.18.1` via the auto-release workflow (patch label)

🤖 Generated with Claude Code

---

Note

Medium Risk
Touches authentication resolution used by every API call and doctor diagnostics; behavior is intended to be preserved but edge cases (OAuth fallback, keychain refs, flag/env precedence) are security-sensitive.

Overview
Centralizes JumpCloud credential resolution in api.ResolveActiveAuth, so NewClient and jc doctor share one decision tree instead of duplicated precedence logic.

auth_resolve.go adds ResolvedAuth (method, source, fingerprint, org ID + provenance) and optional Hint.APIKeyFlagChanged for --api-key vs JC_API_KEY attribution without importing cobra. The resolver covers service-account OAuth, keychain failures, silent api_key fallback labeling, and org ID precedence.

NewClient now delegates to ResolveActiveAuth (~7 lines). collectAuth in doctor shrinks to mapping that result into the report struct; inline collectOrgID is removed.

auth_resolve_test.go adds branch coverage plus a paired test that unresolvable keychain:// api_key refs do not count as fallback and NewClient returns ErrNoAPIKey. Doctor org-ID tests call collectAuth instead of collectOrgID.

^{Reviewed by Cursor Bugbot for commit 1ad377c. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Reviewed by Cursor Bugbot for commit 0c604ea. Configure here.}