← back to Cli Printing Press

test(cli): pin doctor authConfigured short-circuit for OAuth2 + EnvVarSpecs (#1034)

9e604a95195bcfd16d0a45474def930e38779f5f · 2026-05-11 00:19:53 -0700 · Trevin Chow

* test(cli): pin doctor authConfigured short-circuit for OAuth2 + EnvVarSpecs

Issue #879 reported that doctor flagged a missing per_call+Required env var
as `FAIL Env Vars: ERROR missing required: ...` even when AccessToken-based
auth was already configured. The fix landed in 1a8f68ee (#853, v4.2.2): the
template introduces `authConfigured := false`, sets it to true whenever
cfg.AuthHeader() != "", and the per_call+Required branch now routes a
missing env var to authEnvInfo with "credentials available from <source>",
which the switch elevates to "OK ..." via `case len(authEnvInfo) > 0 &&
authConfigured`.

Existing coverage exercised this on the legacy EnvVars path (bearer_token).
The YouTube-shape failure mode — oauth2 + kind-aware EnvVarSpecs with a
per_call+Required entry — was not directly pinned. Add a regression test
that generates that exact spec shape, asserts the authConfigured branch is
emitted, and asserts the missing-required append is positionally inside the
trailing else, so it cannot fire when config-auth is in use.

Refs #879

* test(cli): pin doctor authConfigured else chain as contiguous substring

The prior assertion checked the relative order of two substrings (the
`} else if authConfigured {` branch and the `authEnvRequiredMissing
= append(...)` line). That's too weak: if a refactor flattened the
missing-required append back outside the else chain (the exact shape
of the original #879 bug), both substrings would still exist and
`idxConfigured < idxMissing` would still hold, but the regression
would be back.

Replace with a single contiguous-substring assertion that pins the
full else-if-else chain end-to-end, so the missing-required append
can only ever be the trailing else, gated by authConfigured.

Files touched

• M internal/generator/doctor_auth_status_test.go

Diff

commit 9e604a95195bcfd16d0a45474def930e38779f5f
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Mon May 11 00:19:53 2026 -0700

    test(cli): pin doctor authConfigured short-circuit for OAuth2 + EnvVarSpecs (#1034)
    
    * test(cli): pin doctor authConfigured short-circuit for OAuth2 + EnvVarSpecs
    
    Issue #879 reported that doctor flagged a missing per_call+Required env var
    as `FAIL Env Vars: ERROR missing required: ...` even when AccessToken-based
    auth was already configured. The fix landed in 1a8f68ee (#853, v4.2.2): the
    template introduces `authConfigured := false`, sets it to true whenever
    cfg.AuthHeader() != "", and the per_call+Required branch now routes a
    missing env var to authEnvInfo with "credentials available from <source>",
    which the switch elevates to "OK ..." via `case len(authEnvInfo) > 0 &&
    authConfigured`.
    
    Existing coverage exercised this on the legacy EnvVars path (bearer_token).
    The YouTube-shape failure mode — oauth2 + kind-aware EnvVarSpecs with a
    per_call+Required entry — was not directly pinned. Add a regression test
    that generates that exact spec shape, asserts the authConfigured branch is
    emitted, and asserts the missing-required append is positionally inside the
    trailing else, so it cannot fire when config-auth is in use.
    
    Refs #879
    
    * test(cli): pin doctor authConfigured else chain as contiguous substring
    
    The prior assertion checked the relative order of two substrings (the
    `} else if authConfigured {` branch and the `authEnvRequiredMissing
    = append(...)` line). That's too weak: if a refactor flattened the
    missing-required append back outside the else chain (the exact shape
    of the original #879 bug), both substrings would still exist and
    `idxConfigured < idxMissing` would still hold, but the regression
    would be back.
    
    Replace with a single contiguous-substring assertion that pins the
    full else-if-else chain end-to-end, so the missing-required append
    can only ever be the trailing else, gated by authConfigured.
---
 internal/generator/doctor_auth_status_test.go | 54 +++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/internal/generator/doctor_auth_status_test.go b/internal/generator/doctor_auth_status_test.go
index b885a20a..31da8585 100644
--- a/internal/generator/doctor_auth_status_test.go
+++ b/internal/generator/doctor_auth_status_test.go
@@ -35,3 +35,57 @@ func TestDoctorReportsConfigAuthAsEnvVarsSatisfied(t *testing.T) {
 
 			if authEnvSet == 0 {`, "legacy EnvVars branch must not report zero env vars when config auth is already valid")
 }
+
+// TestDoctorOAuth2PerCallRequiredEnvVarDefersToConfigAuth pins the
+// authConfigured short-circuit on the kind-aware EnvVarSpecs path for
+// oauth2 specs (issue #879). When a user authenticates via `auth login`,
+// AccessToken populates the config and AuthHeader() returns a Bearer; a
+// missing per_call+Required env var must surface as "credentials available
+// from" and route through the "OK" arm of the env_vars switch, never as
+// "ERROR missing required".
+func TestDoctorOAuth2PerCallRequiredEnvVarDefersToConfigAuth(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("doctor-oauth2-envspec")
+	apiSpec.Auth = spec.AuthConfig{
+		Type:             "oauth2",
+		Header:           "Authorization",
+		OAuth2Grant:      spec.OAuth2GrantAuthorizationCode,
+		AuthorizationURL: "https://example.com/oauth/authorize",
+		TokenURL:         "https://example.com/oauth/token",
+		EnvVarSpecs: []spec.AuthEnvVar{
+			{Name: "DOCTOR_OAUTH2_ENVSPEC_TOKEN", Kind: spec.AuthEnvVarKindPerCall, Required: true, Sensitive: true},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "doctor-oauth2-envspec-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	doctorSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+	doctor := string(doctorSrc)
+
+	require.Contains(t, doctor, "authConfigured := false")
+	require.Contains(t, doctor, "authConfigured = true")
+	require.Contains(t, doctor, `case len(authEnvInfo) > 0 && authConfigured:`,
+		"env_vars switch needs the authConfigured arm to elevate INFO to OK")
+
+	// Pin the full else-if-else chain as a contiguous substring. A weaker
+	// "both substrings exist" check would pass even if a refactor flattened
+	// authEnvRequiredMissing back to an unconditional append (the exact
+	// shape of the original #879 bug). Asserting the contiguous block
+	// guarantees the missing-required append is the trailing else, gated
+	// by authConfigured.
+	require.Contains(t, doctor, `if os.Getenv("DOCTOR_OAUTH2_ENVSPEC_TOKEN") != "" {
+				authEnvSet = append(authEnvSet, "DOCTOR_OAUTH2_ENVSPEC_TOKEN")
+			} else if authConfigured {
+				authSource, _ := report["auth_source"].(string)
+				if authSource == "" {
+					authSource = "config"
+				}
+				authEnvInfo = append(authEnvInfo, "credentials available from "+authSource)
+			} else {
+				authEnvRequiredMissing = append(authEnvRequiredMissing, "DOCTOR_OAUTH2_ENVSPEC_TOKEN")
+			}`,
+		"per_call+Required env-var check must route missing-required through the authConfigured else chain, not as an unconditional append")
+}

← b1dded7d fix(cli): classify doctor HTTP 403 as scope-limited WARN, no  ·  back to Cli Printing Press  ·  fix(cli): Store.Get propagates sql.ErrNoRows so callers can 66fd401b →