[object Object]

← back to Cli Printing Press

fix(cli): derive doctor probe path from auth.verify_path or me-shaped endpoint (#1406)

d0226602f7c6d68d59e4b25e87e6940ea48306e6 · 2026-05-14 13:07:14 -0700 · Trevin Chow

Subdomain-customer APIs (Zendesk, Notion, Linear) front `/` with the
marketing or help-center page, which Cloudflare often answers with a 403
interstitial. The generated `doctor` previously probed `/` unconditionally
and reported "blocked by Cloudflare interstitial" while the actual API at
`/api/...` was reachable, contradicting the auth signal.

Derive `spec.HealthCheckPath` in `Generator.Generate()` when unset:
  1. spec.HealthCheckPath (explicit override, never clobbered)
  2. Auth.VerifyPath (already vetted as a known-good GET; unauth probe
     returns 401, which the doctor template classifies as reachable)
  3. Heuristic me-shaped GET endpoint discovered in the spec
  4. Empty -> template falls back to `/` (existing behavior)

The doctor template already had a `{{- if .HealthCheckPath}}` branch;
this change just feeds it a real path for specs that didn't declare one.
findEndpointMatch is added as a sibling to anyEndpointMatches with sorted
key iteration so the derived path stays deterministic across runs.

Closes #1118

Files touched

Diff

commit d0226602f7c6d68d59e4b25e87e6940ea48306e6
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Thu May 14 13:07:14 2026 -0700

    fix(cli): derive doctor probe path from auth.verify_path or me-shaped endpoint (#1406)
    
    Subdomain-customer APIs (Zendesk, Notion, Linear) front `/` with the
    marketing or help-center page, which Cloudflare often answers with a 403
    interstitial. The generated `doctor` previously probed `/` unconditionally
    and reported "blocked by Cloudflare interstitial" while the actual API at
    `/api/...` was reachable, contradicting the auth signal.
    
    Derive `spec.HealthCheckPath` in `Generator.Generate()` when unset:
      1. spec.HealthCheckPath (explicit override, never clobbered)
      2. Auth.VerifyPath (already vetted as a known-good GET; unauth probe
         returns 401, which the doctor template classifies as reachable)
      3. Heuristic me-shaped GET endpoint discovered in the spec
      4. Empty -> template falls back to `/` (existing behavior)
    
    The doctor template already had a `{{- if .HealthCheckPath}}` branch;
    this change just feeds it a real path for specs that didn't declare one.
    findEndpointMatch is added as a sibling to anyEndpointMatches with sorted
    key iteration so the derived path stays deterministic across runs.
    
    Closes #1118
---
 internal/generator/doctor_health_path.go      |  81 ++++++
 internal/generator/doctor_health_path_test.go | 366 ++++++++++++++++++++++++++
 internal/generator/generator.go               |  39 +++
 3 files changed, 486 insertions(+)

diff --git a/internal/generator/doctor_health_path.go b/internal/generator/doctor_health_path.go
new file mode 100644
index 00000000..ed04f774
--- /dev/null
+++ b/internal/generator/doctor_health_path.go
@@ -0,0 +1,81 @@
+package generator
+
+import (
+	"strings"
+
+	"github.com/mvanhorn/cli-printing-press/v4/internal/spec"
+)
+
+// meShapedPathTails enumerates path suffixes that conventionally identify a
+// small auth-only GET endpoint suitable as a reachability probe (the same
+// shape Auth.VerifyPath calls out in spec.go). Order is meaningful only when
+// multiple endpoints qualify in the same spec — earliest match wins so that
+// the more specific `users/me` beats a bare `me` on APIs that ship both.
+var meShapedPathTails = []string{
+	"users/me.json",
+	"users/me",
+	"users/@me",
+	"current_user",
+	"me.json",
+	"whoami",
+	"viewer",
+	"account",
+	"self",
+	"user",
+	"me",
+}
+
+// deriveHealthCheckPath chooses the path the generated `doctor` command should
+// hit for its unauthenticated reachability probe.
+//
+// Priority:
+//  1. spec.HealthCheckPath when set (explicit user override, never clobbered).
+//  2. Auth.VerifyPath when set (already vetted by the operator as a known-good
+//     authenticated GET; an unauthenticated probe against it returns 401 from
+//     the real API surface, which the doctor classifies as "reachable").
+//  3. A heuristic me-shaped GET endpoint discovered in the spec (no required
+//     path/query params and a tail matching meShapedPathTails).
+//  4. Empty string. The template falls back to probing `/`, preserving the
+//     pre-derivation behavior for specs with nothing better to offer.
+//
+// Returning empty in case 4 (rather than `"/"`) keeps the existing template
+// branch in doctor.go.tmpl authoritative for the fallback — one source of
+// truth for the probe path the runtime sends.
+func deriveHealthCheckPath(s *spec.APISpec) string {
+	if s == nil {
+		return ""
+	}
+	if s.HealthCheckPath != "" {
+		return s.HealthCheckPath
+	}
+	if s.Auth.VerifyPath != "" {
+		return s.Auth.VerifyPath
+	}
+	for _, tail := range meShapedPathTails {
+		if e, ok := findEndpointMatch(s, func(e spec.Endpoint) bool {
+			return isMeShapedEndpoint(e, tail)
+		}); ok {
+			return e.Path
+		}
+	}
+	return ""
+}
+
+func isMeShapedEndpoint(e spec.Endpoint, tail string) bool {
+	if !strings.EqualFold(e.Method, "GET") {
+		return false
+	}
+	if e.Path == "" || strings.Contains(e.Path, "{") {
+		return false
+	}
+	for _, p := range e.Params {
+		if p.Required {
+			return false
+		}
+	}
+	// Match the bare tail or any "/<tail>" suffix so prefixed paths like
+	// `/api/v2/users/me.json` resolve cleanly against `users/me.json`.
+	lower := strings.ToLower(strings.TrimSuffix(e.Path, "/"))
+	target := strings.ToLower(tail)
+	return lower == target || strings.HasSuffix(lower, "/"+target)
+}
diff --git a/internal/generator/doctor_health_path_test.go b/internal/generator/doctor_health_path_test.go
new file mode 100644
index 00000000..2e652698
--- /dev/null
+++ b/internal/generator/doctor_health_path_test.go
@@ -0,0 +1,366 @@
+package generator
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/mvanhorn/cli-printing-press/v4/internal/spec"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDeriveHealthCheckPath_PrioritizesExplicitOverride(t *testing.T) {
+	t.Parallel()
+
+	s := minimalSpec("explicit")
+	s.HealthCheckPath = "/api/health"
+	s.Auth.VerifyPath = "/me"
+	s.Resources["users"] = spec.Resource{
+		Endpoints: map[string]spec.Endpoint{
+			"me": {Method: "GET", Path: "/users/me"},
+		},
+	}
+
+	assert.Equal(t, "/api/health", deriveHealthCheckPath(s),
+		"explicit HealthCheckPath must not be overwritten by fallbacks")
+}
+
+func TestDeriveHealthCheckPath_PrefersAuthVerifyPath(t *testing.T) {
+	t.Parallel()
+
+	s := minimalSpec("verify-path")
+	s.Auth.VerifyPath = "/v1/account"
+	s.Resources["users"] = spec.Resource{
+		Endpoints: map[string]spec.Endpoint{
+			"me": {Method: "GET", Path: "/users/me"},
+		},
+	}
+
+	assert.Equal(t, "/v1/account", deriveHealthCheckPath(s),
+		"Auth.VerifyPath should win over a me-shaped heuristic match")
+}
+
+func TestDeriveHealthCheckPath_HeuristicMeShapedTails(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name string
+		path string
+		want string
+	}{
+		{"bare me", "/me", "/me"},
+		{"me.json", "/me.json", "/me.json"},
+		{"users/me", "/users/me", "/users/me"},
+		{"users/me.json (Zendesk)", "/api/v2/users/me.json", "/api/v2/users/me.json"},
+		{"user (GitHub)", "/user", "/user"},
+		{"viewer", "/viewer", "/viewer"},
+		{"whoami", "/api/whoami", "/api/whoami"},
+		{"self", "/v1/self", "/v1/self"},
+		{"account", "/v1/account", "/v1/account"},
+		{"users/@me (Discord)", "/api/users/@me", "/api/users/@me"},
+		{"current_user", "/api/current_user", "/api/current_user"},
+		{"mixed case path", "/Users/Me", "/Users/Me"},
+		{"trailing slash", "/users/me/", "/users/me/"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			s := &spec.APISpec{
+				Name:    "heuristic",
+				BaseURL: "https://api.example.com",
+				Resources: map[string]spec.Resource{
+					"probe": {Endpoints: map[string]spec.Endpoint{
+						"probe": {Method: "GET", Path: tc.path},
+					}},
+				},
+			}
+			assert.Equal(t, tc.want, deriveHealthCheckPath(s))
+		})
+	}
+}
+
+func TestDeriveHealthCheckPath_SkipsPathsWithPlaceholders(t *testing.T) {
+	t.Parallel()
+
+	// `/{tenant}/me` would match the bare `me` tail if the placeholder
+	// guard were removed — that makes the guard load-bearing on this test.
+	s := &spec.APISpec{
+		Name: "placeholder",
+		Resources: map[string]spec.Resource{
+			"users": {Endpoints: map[string]spec.Endpoint{
+				"me": {Method: "GET", Path: "/{tenant}/me"},
+			}},
+		},
+	}
+	assert.Equal(t, "", deriveHealthCheckPath(s),
+		"paths with {placeholders} cannot be probed without inputs")
+}
+
+func TestDeriveHealthCheckPath_RejectsSubstringSegmentMatches(t *testing.T) {
+	t.Parallel()
+
+	// `/some_account` ends with the string "account" but not the segment
+	// "account"; the boundary anchor `"/"+target` should keep it out.
+	cases := []string{"/some_account", "/admin/notme", "/account-management"}
+	for _, path := range cases {
+		t.Run(path, func(t *testing.T) {
+			t.Parallel()
+			s := &spec.APISpec{
+				Resources: map[string]spec.Resource{
+					"r": {Endpoints: map[string]spec.Endpoint{
+						"e": {Method: "GET", Path: path},
+					}},
+				},
+			}
+			assert.Equal(t, "", deriveHealthCheckPath(s),
+				"%s shares only a substring with a tail; must not match", path)
+		})
+	}
+}
+
+func TestDeriveHealthCheckPath_SkipsRequiredQueryParams(t *testing.T) {
+	t.Parallel()
+
+	s := &spec.APISpec{
+		Name: "required-params",
+		Resources: map[string]spec.Resource{
+			"users": {Endpoints: map[string]spec.Endpoint{
+				"me": {
+					Method: "GET",
+					Path:   "/users/me",
+					Params: []spec.Param{{Name: "fields", Required: true}},
+				},
+			}},
+		},
+	}
+	assert.Equal(t, "", deriveHealthCheckPath(s),
+		"endpoints with required params can't be safely probed unauthenticated")
+}
+
+func TestDeriveHealthCheckPath_SkipsNonGet(t *testing.T) {
+	t.Parallel()
+
+	s := &spec.APISpec{
+		Name: "non-get",
+		Resources: map[string]spec.Resource{
+			"users": {Endpoints: map[string]spec.Endpoint{
+				"create-me": {Method: "POST", Path: "/users/me"},
+			}},
+		},
+	}
+	assert.Equal(t, "", deriveHealthCheckPath(s))
+}
+
+func TestDeriveHealthCheckPath_WalksSubResources(t *testing.T) {
+	t.Parallel()
+
+	s := &spec.APISpec{
+		Name: "nested",
+		Resources: map[string]spec.Resource{
+			"top": {
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/top"},
+				},
+				SubResources: map[string]spec.Resource{
+					"users": {Endpoints: map[string]spec.Endpoint{
+						"me": {Method: "GET", Path: "/users/me"},
+					}},
+				},
+			},
+		},
+	}
+	assert.Equal(t, "/users/me", deriveHealthCheckPath(s))
+}
+
+func TestDeriveHealthCheckPath_PrefersSpecificTailOverBare(t *testing.T) {
+	t.Parallel()
+
+	// A spec that ships both `/me` and `/users/me` should land on
+	// `/users/me` — meShapedPathTails orders the more specific tail first.
+	s := &spec.APISpec{
+		Name: "both",
+		Resources: map[string]spec.Resource{
+			"r": {Endpoints: map[string]spec.Endpoint{
+				"a": {Method: "GET", Path: "/me"},
+				"b": {Method: "GET", Path: "/users/me"},
+			}},
+		},
+	}
+	assert.Equal(t, "/users/me", deriveHealthCheckPath(s))
+}
+
+func TestDeriveHealthCheckPath_FallsBackToEmpty(t *testing.T) {
+	t.Parallel()
+
+	s := &spec.APISpec{
+		Name: "nothing-suitable",
+		Resources: map[string]spec.Resource{
+			"items": {Endpoints: map[string]spec.Endpoint{
+				"list":   {Method: "GET", Path: "/items"},
+				"create": {Method: "POST", Path: "/items"},
+			}},
+		},
+	}
+	assert.Equal(t, "", deriveHealthCheckPath(s),
+		"no me-shaped tail matches → template falls back to `/`")
+}
+
+func TestDeriveHealthCheckPath_NilSpec(t *testing.T) {
+	t.Parallel()
+	assert.Equal(t, "", deriveHealthCheckPath(nil))
+}
+
+func TestDeriveHealthCheckPath_DeterministicSameTailCollision(t *testing.T) {
+	t.Parallel()
+
+	// Two GET endpoints in different resources both match the bare `me`
+	// tail. findEndpointMatch iterates sorted resource keys, so `a-resource`
+	// wins regardless of map iteration order. Re-run the helper 50 times to
+	// catch any non-determinism that slipped past a single happy-path run.
+	build := func() *spec.APISpec {
+		return &spec.APISpec{
+			Name: "collide",
+			Resources: map[string]spec.Resource{
+				"z-resource": {Endpoints: map[string]spec.Endpoint{
+					"me": {Method: "GET", Path: "/z/me"},
+				}},
+				"a-resource": {Endpoints: map[string]spec.Endpoint{
+					"me": {Method: "GET", Path: "/a/me"},
+				}},
+			},
+		}
+	}
+	for range 50 {
+		assert.Equal(t, "/a/me", deriveHealthCheckPath(build()))
+	}
+}
+
+func TestGenerate_HealthCheckPathDerivationIsIdempotent(t *testing.T) {
+	t.Parallel()
+
+	// Generate() mutates g.Spec.HealthCheckPath when empty. A second call
+	// on the same spec must re-emit byte-identical output — regen-merge and
+	// mcp-sync re-enter Generate() on a spec that already saw the
+	// derivation pass.
+	apiSpec := minimalSpec("idempotent")
+	apiSpec.Auth.VerifyPath = "/v1/account"
+
+	firstDir := filepath.Join(t.TempDir(), "first")
+	require.NoError(t, New(apiSpec, firstDir).Generate())
+	first, err := os.ReadFile(filepath.Join(firstDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+
+	secondDir := filepath.Join(t.TempDir(), "second")
+	require.NoError(t, New(apiSpec, secondDir).Generate())
+	second, err := os.ReadFile(filepath.Join(secondDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+
+	assert.Equal(t, string(first), string(second),
+		"second Generate() must emit byte-identical doctor.go")
+	assert.Equal(t, "/v1/account", apiSpec.HealthCheckPath,
+		"derived value should be visible on spec after Generate()")
+}
+
+// TestGeneratedDoctor_DerivesHealthCheckPathFromVerifyPath wires the helper
+// through Generate(): a spec with only Auth.VerifyPath set and no explicit
+// HealthCheckPath must emit a doctor.go that probes the verify path, not `/`.
+func TestGeneratedDoctor_DerivesHealthCheckPathFromVerifyPath(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("derive-verify")
+	apiSpec.Auth.VerifyPath = "/v1/account"
+
+	outputDir := filepath.Join(t.TempDir(), "derive-verify-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	doctorGo, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+	content := string(doctorGo)
+
+	assert.Contains(t, content, `healthPath := "/v1/account"`,
+		"doctor should probe Auth.VerifyPath when HealthCheckPath is unset")
+	assert.NotContains(t, content, `reachBody, reachErr := c.Get("/", nil)`,
+		"the bare-root fallback branch should not be rendered when a derived path exists")
+}
+
+// TestGeneratedDoctor_DerivesHealthCheckPathFromMeEndpoint mirrors the above
+// for the heuristic case — no Auth.VerifyPath, but a me-shaped GET endpoint
+// in the spec should be picked up.
+func TestGeneratedDoctor_DerivesHealthCheckPathFromMeEndpoint(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("derive-me")
+	apiSpec.Resources["users"] = spec.Resource{
+		Description: "Users",
+		Endpoints: map[string]spec.Endpoint{
+			"me": {Method: "GET", Path: "/users/me", Description: "Get current user"},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "derive-me-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	doctorGo, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+	content := string(doctorGo)
+
+	assert.Contains(t, content, `healthPath := "/users/me"`)
+}
+
+// TestGeneratedDoctor_KeepsExplicitHealthCheckPath guards against the helper
+// overwriting an explicit override. Pairs with the priority logic in
+// deriveHealthCheckPath.
+func TestGeneratedDoctor_KeepsExplicitHealthCheckPath(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("explicit-override")
+	apiSpec.HealthCheckPath = "api/marketStatus"
+	apiSpec.Auth.VerifyPath = "/me" // would otherwise win over the heuristic
+
+	outputDir := filepath.Join(t.TempDir(), "explicit-override-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	doctorGo, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+	assert.Contains(t, string(doctorGo), `healthPath := "api/marketStatus"`)
+}
+
+// TestGeneratedDoctor_NoCandidateFallsBackToRoot keeps the negative case
+// stable: a spec with nothing me-shaped renders the `/`-probe branch the
+// pre-derivation template has always emitted.
+func TestGeneratedDoctor_NoCandidateFallsBackToRoot(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := &spec.APISpec{
+		Name:    "fallback",
+		Version: "0.1.0",
+		BaseURL: "https://api.example.com",
+		Auth:    spec.AuthConfig{Type: "none"},
+		Config: spec.ConfigSpec{
+			Format: "toml",
+			Path:   "~/.config/fallback-pp-cli/config.toml",
+		},
+		Resources: map[string]spec.Resource{
+			"items": {
+				Description: "Manage items",
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/items", Description: "List items"},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "fallback-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	doctorGo, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+	content := string(doctorGo)
+
+	assert.Contains(t, content, `reachBody, reachErr := c.Get("/", nil)`,
+		"specs with no derivable probe path should keep the bare-root fallback")
+	assert.NotContains(t, content, `healthPath := "`,
+		"no healthPath variable should be declared when the spec has nothing to derive")
+}
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index 7e53a6c5..0450e8e8 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -1637,6 +1637,9 @@ func (g *Generator) Generate() error {
 				"Set `git config github.user` (your GitHub @handle) to populate this correctly before publishing.\n",
 		)
 	}
+	if g.Spec.HealthCheckPath == "" {
+		g.Spec.HealthCheckPath = deriveHealthCheckPath(g.Spec)
+	}
 	if err := g.prepareOutput(); err != nil {
 		return err
 	}
@@ -3639,6 +3642,42 @@ func anyEndpointMatches(apiSpec *spec.APISpec, predicate func(spec.Endpoint) boo
 	return walk(apiSpec.Resources)
 }
 
+// findEndpointMatch returns the first endpoint for which predicate is true,
+// walking resources and sub-resources depth-first. Resource and endpoint
+// names are iterated in sorted order so callers that bake the returned
+// endpoint's path into generated output stay deterministic across runs.
+func findEndpointMatch(apiSpec *spec.APISpec, predicate func(spec.Endpoint) bool) (spec.Endpoint, bool) {
+	if apiSpec == nil {
+		return spec.Endpoint{}, false
+	}
+	var walk func(resources map[string]spec.Resource) (spec.Endpoint, bool)
+	walk = func(resources map[string]spec.Resource) (spec.Endpoint, bool) {
+		for _, rName := range sortedKeys(resources) {
+			resource := resources[rName]
+			for _, eName := range sortedKeys(resource.Endpoints) {
+				endpoint := resource.Endpoints[eName]
+				if predicate(endpoint) {
+					return endpoint, true
+				}
+			}
+			if e, ok := walk(resource.SubResources); ok {
+				return e, ok
+			}
+		}
+		return spec.Endpoint{}, false
+	}
+	return walk(apiSpec.Resources)
+}
+
+func sortedKeys[V any](m map[string]V) []string {
+	keys := make([]string, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return keys
+}
+
 // formBodyMaps renders per-flag form-field assignments for endpoints that send
 // application/x-www-form-urlencoded request bodies. Object/array/JSON-string
 // fields are validated as JSON then sent as a single string field (matching

← 15964974 fix(cli): tighten phone-us PII regex to NANP-valid first dig  ·  back to Cli Printing Press  ·  fix(cli): stop generated main.go from printing every error t 46ad86b2 →