cli: reclassify non-envelope errors and split 401/403 auth codes (#205)

Route error responses with no v3 envelope through FromAPIError by HTTP status
instead of the opaque `error` code: 4xx map to client codes, 5xx to
unclassified_server_error, unmapped to unclassified_client_error, always with a
non-empty message. Preserve code-only envelopes (relaxed the message-only gate).
Split 401/403 into unauthorized/forbidden so a 403 no longer suggests logging in;
enrichAuthHint is gated to not-authenticated codes. Help footer + README exit-code
table now read '3 auth/permission'.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: somanshreddy

README.md

@@ -165,7 +165,7 @@ Every command supports `--help`.
 |--------|----------|
 | **stdout** | Always JSON. Even `video download` — binary writes to disk; stdout emits `{"asset", "message", "path"}` so you can chain on `.path`. |
 | **stderr** | Structured envelope on error: `{"error": {"code", "message", "hint"}}`. Stable `code` values for programmatic branching. |
-| **Exit codes** | `0` ok · `1` API or network · `2` usage · `3` auth · `4` timeout under `--wait` (stdout contains partial resource for resume) |
+| **Exit codes** | `0` ok · `1` API or network · `2` usage · `3` auth / not permitted · `4` timeout under `--wait` (stdout contains partial resource for resume) |
 | **Request bodies** | Flags for simple inputs; `-d` for nested JSON (inline, file path, or `-` for stdin). Flags override matching fields. |
 | **Async jobs** | `--wait` blocks with exponential backoff; `--timeout` sets max (default 20m). 429s and 5xx retry automatically. |
 

cmd/heygen/auth_status_test.go

@@ -53,8 +53,8 @@ func TestAuthStatus_InvalidKey(t *testing.T) {
 	if res.ExitCode != clierrors.ExitAuth {
 		t.Fatalf("ExitCode = %d, want %d\nstderr: %s", res.ExitCode, clierrors.ExitAuth, res.Stderr)
 	}
-	if !strings.Contains(res.Stderr, `"code":"auth_error"`) {
-		t.Fatalf("stderr = %s, want auth_error code", res.Stderr)
+	if !strings.Contains(res.Stderr, `"code":"unauthorized"`) {
+		t.Fatalf("stderr = %s, want unauthorized code", res.Stderr)
 	}
 	if !strings.Contains(res.Stderr, `"message":"invalid API key"`) {
 		t.Fatalf("stderr = %s, want invalid API key message", res.Stderr)

cmd/heygen/context.go

@@ -77,6 +77,14 @@ func enrichAuthHint(cliErr *clierrors.CLIError, source auth.CredentialSource) {
 	if cliErr.ExitCode != clierrors.ExitAuth {
 		return
 	}
+	// Only enrich genuine not-authenticated errors. 403 (forbidden / permission)
+	// also exits 3, but re-authenticating won't help, so never stamp a "log in"
+	// hint on it — it carries its own permission-oriented hint.
+	switch cliErr.Code {
+	case "auth_error", "unauthorized", "invalid_credentials":
+	default:
+		return
+	}
 	if cliErr.Hint != "" {
 		return
 	}

cmd/heygen/context_test.go

@@ -94,3 +94,54 @@ func TestEnrichAuthHint_ExistingHint_Preserved(t *testing.T) {
 		t.Fatalf("no-key error should preserve authGuidance hint:\n%s", res.Stderr)
 	}
 }
+
+// A 403 (forbidden) exits 3 but must NOT be given a login / invalid-key hint by
+// enrichAuthHint — it keeps its permission-oriented hint.
+func TestEnrichAuthHint_Forbidden_NotOverwritten(t *testing.T) {
+	srv := setupTestServer(t, map[string]testHandler{
+		"GET /v3/videos": {
+			StatusCode: 403,
+			Body:       `{"error":{"code":"forbidden","message":"not allowed"}}`,
+		},
+	})
+	defer srv.Close()
+
+	res := runCommand(t, srv.URL, "some-key", "video", "list")
+
+	if res.ExitCode != clierrors.ExitAuth {
+		t.Fatalf("ExitCode = %d, want %d\nstderr: %s", res.ExitCode, clierrors.ExitAuth, res.Stderr)
+	}
+	if !strings.Contains(res.Stderr, `"code":"forbidden"`) {
+		t.Fatalf("stderr should carry the forbidden code:\n%s", res.Stderr)
+	}
+	if strings.Contains(res.Stderr, "HEYGEN_API_KEY environment variable") || strings.Contains(res.Stderr, "auth login") {
+		t.Fatalf("a 403 must not get a login/invalid-key hint:\n%s", res.Stderr)
+	}
+	if !strings.Contains(res.Stderr, "not permitted") {
+		t.Fatalf("a 403 should carry a permission hint:\n%s", res.Stderr)
+	}
+}
+
+// A non-envelope (HTML) 403 — status-derived forbidden — must also avoid the
+// login hint end-to-end, exercising the derived-code path through the formatter.
+func TestForbidden_NonEnvelope_NoLoginHint(t *testing.T) {
+	srv := setupTestServer(t, map[string]testHandler{
+		"GET /v3/videos": {
+			StatusCode: 403,
+			Body:       `<html>403 Forbidden</html>`,
+		},
+	})
+	defer srv.Close()
+
+	res := runCommand(t, srv.URL, "some-key", "video", "list")
+
+	if res.ExitCode != clierrors.ExitAuth {
+		t.Fatalf("ExitCode = %d, want %d\nstderr: %s", res.ExitCode, clierrors.ExitAuth, res.Stderr)
+	}
+	if !strings.Contains(res.Stderr, `"code":"forbidden"`) {
+		t.Fatalf("stderr should carry the derived forbidden code:\n%s", res.Stderr)
+	}
+	if strings.Contains(res.Stderr, "HEYGEN_API_KEY environment variable") || strings.Contains(res.Stderr, "auth login") {
+		t.Fatalf("a non-envelope 403 must not get a login hint:\n%s", res.Stderr)
+	}
+}

cmd/heygen/generated_root_test.go

@@ -181,7 +181,7 @@ func TestGeneratedRoot_Help_ShowsExitCodesAndHidesCompletion(t *testing.T) {
 	if res.ExitCode != 0 {
 		t.Errorf("ExitCode = %d, want 0\nstderr: %s", res.ExitCode, res.Stderr)
 	}
-	if !strings.Contains(res.Stdout, "Exit Codes:") || !strings.Contains(res.Stdout, "3   Authentication error") {
+	if !strings.Contains(res.Stdout, "Exit Codes:") || !strings.Contains(res.Stdout, "3   Authentication / permission error") {
 		t.Errorf("root help missing documented exit codes\nstdout: %s", res.Stdout)
 	}
 	if strings.Contains(res.Stdout, "completion") {

cmd/heygen/root.go

@@ -89,7 +89,7 @@ Exit Codes:
   0   Success
   1   General error (API error, network failure)
   2   Usage error (invalid flags, missing arguments)
-  3   Authentication error (missing or invalid API key)
+  3   Authentication / permission error (not authenticated, or not permitted)
   4   Timeout (resource created but operation not yet complete)
 
 Tip: Use --request-schema on any command to see the expected JSON input fields.

internal/client/executor.go

@@ -496,18 +496,22 @@ func classifyPollContextError(err error, resourceID string, lastResp json.RawMes
 }
 
 // parseErrorResponse parses an API error response into a CLIError.
+//
+// A body that unmarshals into the v3 envelope with a code OR message is a real app
+// error (the code-or-message gate is relaxed from the old message-only check so a
+// code-only envelope keeps its code instead of collapsing to generic "error"). Any
+// other body — gateway/WAF HTML, an empty body, or a foreign JSON shape — did not
+// come from the v3 app; it is classified by HTTP status via FromAPIError (which maps
+// 4xx to client codes, 5xx to unclassified_server_error, unmapped to
+// unclassified_client_error, and synthesizes a non-empty message).
 func parseErrorResponse(statusCode int, body []byte, requestID string) *clierrors.CLIError {
 	var envelope struct {
 		Error clierrors.APIError `json:"error"`
 	}
-	if err := json.Unmarshal(body, &envelope); err == nil && envelope.Error.Message != "" {
+	if err := json.Unmarshal(body, &envelope); err == nil &&
+		(envelope.Error.Code != "" || envelope.Error.Message != "") {
 		return clierrors.FromAPIError(statusCode, &envelope.Error, requestID)
 	}
 
-	return &clierrors.CLIError{
-		Code:      "error",
-		Message:   fmt.Sprintf("API returned HTTP %d", statusCode),
-		RequestID: requestID,
-		ExitCode:  clierrors.ExitGeneral,
-	}
+	return clierrors.FromAPIError(statusCode, &clierrors.APIError{}, requestID)
 }

internal/client/executor_test.go

@@ -921,3 +921,66 @@ func emptyInvocation() *command.Invocation {
 		QueryParams: make(url.Values),
 	}
 }
+
+func TestParseErrorResponse_NonEnvelope(t *testing.T) {
+	tests := []struct {
+		name     string
+		status   int
+		body     string
+		wantCode string
+	}{
+		{"gateway HTML 502", 502, "<html>502 Bad Gateway</html>", "unclassified_server_error"},
+		{"empty body 503", 503, "", "unclassified_server_error"},
+		{"empty body 404", 404, "", "not_found"},
+		{"unmapped 4xx 405", 405, "<html>405 Method Not Allowed</html>", "unclassified_client_error"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := parseErrorResponse(tt.status, []byte(tt.body), "req_1")
+			if err.Code != tt.wantCode {
+				t.Errorf("Code = %q, want %q", err.Code, tt.wantCode)
+			}
+			if err.Message == "" {
+				t.Errorf("Message must never be empty (status %d)", tt.status)
+			}
+		})
+	}
+}
+
+func TestParseErrorResponse_CodeOnlyEnvelope(t *testing.T) {
+	// An envelope with a code but no message must keep the code (relaxed gate),
+	// not collapse to a generic error.
+	err := parseErrorResponse(409, []byte(`{"error":{"code":"conflict"}}`), "req_1")
+	if err.Code != "conflict" {
+		t.Errorf("Code = %q, want conflict", err.Code)
+	}
+	if err.Message == "" {
+		t.Errorf("Message should be synthesized when the envelope has none")
+	}
+}
+
+// The intended behavior change: a non-envelope 401/403 (opaque or HTML body) now
+// classifies as unauthorized/forbidden with ExitAuth, through the parseErrorResponse
+// gate that changed in this PR (previously it fell to generic error / ExitGeneral).
+func TestParseErrorResponse_NonEnvelope_AuthExit(t *testing.T) {
+	tests := []struct {
+		name     string
+		status   int
+		body     string
+		wantCode string
+	}{
+		{"empty 401", 401, "", "unauthorized"},
+		{"HTML 403", 403, "<html>403 Forbidden</html>", "forbidden"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := parseErrorResponse(tt.status, []byte(tt.body), "req_1")
+			if err.Code != tt.wantCode {
+				t.Errorf("Code = %q, want %q", err.Code, tt.wantCode)
+			}
+			if err.ExitCode != clierrors.ExitAuth {
+				t.Errorf("ExitCode = %d, want %d (ExitAuth)", err.ExitCode, clierrors.ExitAuth)
+			}
+		})
+	}
+}

internal/errors/errors.go

@@ -75,47 +75,99 @@ func NewUsage(message string) *CLIError {
 	}
 }
 
+// isNonSpecific reports whether an API-provided error code is too generic to
+// trust for classification. v3 always sends a specific code; an empty or literal
+// "error" code means we should classify from the HTTP status instead.
+func isNonSpecific(code string) bool {
+	return code == "" || code == "error"
+}
+
+// forbiddenHint is the shared non-login guidance for 403 responses: the caller is
+// authenticated but not permitted, so re-authenticating will not help.
+const forbiddenHint = "Your credentials are valid but not permitted for this. Check your plan, permissions, or workspace — re-authenticating will not help. Contact support if this is unexpected."
+
 // FromAPIError converts an API error envelope and HTTP status code into a CLIError.
+//
+// The v3 API always returns a specific lowercase code, which is preserved. When the
+// code is absent or non-specific (empty or literal "error" — e.g. a non-envelope
+// edge/gateway response routed here by parseErrorResponse), the code is derived from
+// the HTTP status instead.
 func FromAPIError(statusCode int, apiErr *APIError, requestID string) *CLIError {
 	exitCode := ExitGeneral
 	code := apiErr.Code
+	if isNonSpecific(code) {
+		code = "" // fall through to status-derived classification
+	}
 
 	switch {
-	case statusCode == 401 || statusCode == 403:
+	case statusCode == 401:
 		exitCode = ExitAuth
 		if code == "" {
-			code = "auth_error"
+			code = "unauthorized"
+		}
+	case statusCode == 403:
+		// Authenticated but not permitted — auth-family exit, but NOT "log in".
+		exitCode = ExitAuth
+		if code == "" {
+			code = "forbidden"
 		}
 	case statusCode == 400:
 		if code == "" {
 			code = "validation_error"
 		}
+	case statusCode == 402:
+		if code == "" {
+			code = "insufficient_credit"
+		}
 	case statusCode == 404:
 		if code == "" {
 			code = "not_found"
 		}
+	case statusCode == 409:
+		if code == "" {
+			code = "conflict"
+		}
+	case statusCode == 413:
+		if code == "" {
+			code = "payload_too_large"
+		}
 	case statusCode == 429:
 		if code == "" {
 			code = "rate_limited"
 		}
 	case statusCode >= 500:
+		// A v3 app 5xx carries its own code (internal_error); a 5xx with no specific
+		// code did not come through the app's normal error path.
 		if code == "" {
-			code = "server_error"
+			code = "unclassified_server_error"
 		}
 	default:
+		// Unmapped 4xx (405, 410, 415, 422, ...) with no specific code: the request
+		// was rejected in a way we don't classify — client-side.
 		if code == "" {
-			code = "error"
+			code = "unclassified_client_error"
 		}
 	}
 
-	hint := hintForAPICode(apiErr.Code)
-	if apiErr.Code == "invalid_parameter" && apiErr.Param != nil && *apiErr.Param != "" {
+	message := apiErr.Message
+	if message == "" {
+		// Never render an empty message (e.g. a non-envelope body routed here).
+		message = fmt.Sprintf("API returned HTTP %d", statusCode)
+	}
+
+	hint := hintForAPICode(code)
+	if code == "invalid_parameter" && apiErr.Param != nil && *apiErr.Param != "" {
 		hint = fmt.Sprintf("Invalid field %q. %s", *apiErr.Param, hint)
 	}
+	// Generic non-login hint for any unmapped 403 code (hintForAPICode is code-only
+	// and cannot see the status).
+	if hint == "" && statusCode == 403 {
+		hint = forbiddenHint
+	}
 
 	return &CLIError{
 		Code:      code,
-		Message:   apiErr.Message,
+		Message:   message,
 		Hint:      hint,
 		RequestID: requestID,
 		ExitCode:  exitCode,
@@ -124,6 +176,9 @@ func FromAPIError(statusCode int, apiErr *APIError, requestID string) *CLIError
 
 // hintForAPICode returns a CLI-specific actionable hint for known API error
 // codes. Returns "" if the code has no associated hint.
+//
+// Note: "unauthorized" deliberately has NO hint here — the source-aware
+// enrichAuthHint (cmd/heygen) supplies better guidance (which credential is bad).
 func hintForAPICode(code string) string {
 	switch code {
 	case "avatar_not_found":
@@ -144,6 +199,16 @@ func hintForAPICode(code string) string {
 		return "The asset may still be processing or was deleted"
 	case "timeout":
 		return "The operation may still be in progress. Check status with the corresponding get command"
+	case "forbidden", "resource_access_denied", "ai_vendor_access_restricted":
+		return forbiddenHint
+	case "voice_not_usable":
+		return "This voice can't be used for this request (e.g. not permitted on your plan). Choose another: heygen voice list"
+	case "payload_too_large":
+		return "The request or upload exceeds the size limit. Reduce the file size or payload"
+	case "conflict":
+		return "This conflicts with the current state (e.g. a duplicate or in-progress request). Retrying the same request is unlikely to help"
+	case "unclassified_server_error":
+		return "The server returned an error with no details. This is often transient — retry shortly; if it persists, contact support"
 	}
 	return ""
 }