refactor: retire legacy error envelopes and enforce typed contract

.golangci.yml

@@ -29,11 +29,11 @@ linters:
     - unused          # checks for unused constants, variables, functions and types
     - depguard        # blocks forbidden package imports
     - forbidigo       # forbids specific function calls
+    - errorlint       # enforces error wrapping (%w) and errors.Is/As over == and type asserts
 
     # To enable later after fixing existing issues:
     # - errcheck      # checks for unchecked errors
     # - errname       # checks that error types are named XxxError
-    # - errorlint     # checks error wrapping best practices
     # - gosec         # security-oriented linter
     # - misspell      # finds commonly misspelled English words
     # - staticcheck   # comprehensive static analysis
@@ -49,9 +49,16 @@ linters:
           - gocritic
           - depguard
           - forbidigo
-      # Paths that run forbidigo. Add an entry when a path joins one of
-      # the rules below.
+          - errorlint # tests legitimately do identity (==) and concrete type-assert checks
+      # forbidigo runs repo-wide (minus the boundaries below) so errs-no-bare-wrap
+      # has no gap. The framework bans (os/vfs, raw HTTP, fmt.Print, filepath,
+      # log) stay scoped to shortcuts/ + internal/ + config/auth/service via the
+      # next rule; elsewhere only errs-no-bare-wrap fires.
+      - path-except: (shortcuts/|internal/|cmd/|events/)
+        linters:
+          - forbidigo
       - path-except: (shortcuts/|internal/|cmd/auth/|cmd/config/|cmd/service/)
+        text: (vfs|IOStreams|ctx\.Out|shortcuts-no-raw-http|filepath functions|os\.Exit|structured error return)
         linters:
           - forbidigo
       - path: internal/vfs/
@@ -71,25 +78,14 @@ linters:
         text: shortcuts-no-raw-http
         linters:
           - forbidigo
-      # errs-typed-only enforced on paths already migrated to errs.NewXxxError.
-      # Add a path when its migration is complete.
-      - path-except: (internal/auth/|internal/errcompat/|internal/errclass/|internal/client/|internal/cmdutil/factory\.go|cmd/auth/|cmd/config/|cmd/service/|shortcuts/common/mcp_client\.go|shortcuts/apps/|shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/markdown/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/wiki/|internal/event/consume/|cmd/event/|events/|shortcuts/event/)
-        text: errs-typed-only
-        linters:
-          - forbidigo
-      # errs-no-bare-wrap enforced on paths fully migrated to typed final
-      # errors. Scoped separately from errs-typed-only because cmd/auth/,
-      # cmd/config/ still have residual fmt.Errorf and must not be caught.
-      - path-except: (shortcuts/apps/|shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/markdown/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/wiki/|shortcuts/common/mcp_client\.go|cmd/event/|events/|shortcuts/event/)
+      # errs-no-bare-wrap enforced across every command/wire boundary by
+      # structural prefix, so any future business domain or command is covered
+      # without editing an allowlist. Genuine intermediate wraps inside these
+      # paths use //nolint:forbidigo with a reason.
+      - path-except: (cmd/|shortcuts/|events/)
         text: errs-no-bare-wrap
         linters:
           - forbidigo
-      # errs-no-legacy-helper enforced on domains whose shared validation/save
-      # helpers have migrated to typed final errors.
-      - path-except: (shortcuts/apps/|shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/markdown/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/wiki/|cmd/event/|events/|shortcuts/event/)
-        text: errs-no-legacy-helper
-        linters:
-          - forbidigo
 
   settings:
     depguard:
@@ -108,22 +104,6 @@ linters:
                 Use runtime.FileIO() for file operations or runtime.ValidatePath() for path validation.
     forbidigo:
       forbid:
-        # ── legacy output.Err* helpers banned on migrated paths ──
-        # output.ErrBare is intentionally not listed — it is the predicate-
-        # command silent-exit signal, outside the typed envelope contract.
-        - pattern: output\.(ErrValidation|ErrAuth|ErrNetwork|ErrAPI|ErrWithHint|Errorf)\b
-          msg: >-
-            [errs-typed-only] use errs.NewXxxError(...) builder
-            (see errs/types.go).
-        # ── legacy shared error helpers banned on migrated domains ──
-        # These helpers emit legacy output.Err* / bare error shapes or drop
-        # typed metadata such as Param/Cause. Migrated domains must use typed
-        # common replacements or local typed helpers instead.
-        - pattern: (common\.FlagErrorf|common\.RejectDangerousChars|common\.WrapInputStatError|common\.WrapSaveErrorByCategory)\b
-          msg: >-
-            [errs-no-legacy-helper] these shared helpers emit legacy or
-            metadata-poor error shapes. Use typed common replacements, typed
-            errs.NewXxxError builders, or domain-local typed helpers.
         # ── bare error wraps banned on fully-typed paths ──
         - pattern: (fmt\.Errorf|errors\.New)\b
           msg: >-

cmd/api/api.go

@@ -10,6 +10,7 @@
 	"regexp"
 	"strings"
 
+	"github.com/larksuite/cli/errs"
 	"github.com/larksuite/cli/internal/client"
 	"github.com/larksuite/cli/internal/cmdutil"
 	"github.com/larksuite/cli/internal/core"
@@ -123,7 +124,13 @@
 
 	// stdin conflict: --params and --data cannot both read from stdin, regardless of --file.
 	if opts.Params == "-" && opts.Data == "-" {
-		return client.RawApiRequest{}, nil, output.ErrValidation("--params and --data cannot both read from stdin (-)")
+		return client.RawApiRequest{}, nil, errs.NewValidationError(errs.SubtypeInvalidArgument,
+			"--params and --data cannot both read from stdin (-)").
+			WithHint("pass at most one flag as '-'; give the other inline JSON or @file").
+			WithParams(
+				errs.InvalidParam{Name: "--params", Reason: "reads from stdin (-)"},
+				errs.InvalidParam{Name: "--data", Reason: "reads from stdin (-)"},
+			)
 	}
 
 	params, err := cmdutil.ParseJSONMap(opts.Params, "--params", stdin, fileIO)
@@ -153,7 +160,10 @@
 				return client.RawApiRequest{}, nil, err
 			}
 			if _, ok := dataFields.(map[string]any); !ok {
-				return client.RawApiRequest{}, nil, output.ErrValidation("--data must be a JSON object when used with --file")
+				return client.RawApiRequest{}, nil, errs.NewValidationError(errs.SubtypeInvalidArgument,
+					"--data must be a JSON object when used with --file").
+					WithHint(`with --file, --data carries multipart form fields, e.g. --data '{"image_type":"message"}'`).
+					WithParam("--data")
 			}
 		}
 
@@ -196,7 +206,13 @@
 	}
 
 	if opts.PageAll && opts.Output != "" {
-		return output.ErrValidation("--output and --page-all are mutually exclusive")
+		return errs.NewValidationError(errs.SubtypeInvalidArgument,
+			"--output and --page-all are mutually exclusive").
+			WithHint("drop --page-all to save a binary response, or drop --output to paginate JSON").
+			WithParams(
+				errs.InvalidParam{Name: "--output", Reason: "conflicts with --page-all"},
+				errs.InvalidParam{Name: "--page-all", Reason: "conflicts with --output"},
+			)
 	}
 	if err := output.ValidateJqFlags(opts.JqExpr, opts.Output, opts.Format); err != nil {
 		return err
@@ -239,11 +255,11 @@
 
 	resp, err := ac.DoAPI(opts.Ctx, request)
 	if err != nil {
-		// MarkRaw tells the dispatcher to skip the legacy enrichPermissionError
-		// pass on *output.ExitError values. Typed *errs.* errors that flow
-		// through here keep their canonical message / hint from BuildAPIError;
-		// MarkRaw is a no-op on those (it only flips a flag on *ExitError).
-		return output.MarkRaw(err)
+		// MarkRaw tells the dispatcher to skip hint enrichment so the typed
+		// error keeps its canonical message / hint from BuildAPIError: the
+		// `api` command is a passthrough surface and the caller wants the
+		// original Lark response wording, not a locally enriched variant.
+		return errs.MarkRaw(err)
 	}
 	err = client.HandleResponse(resp, client.ResponseOptions{
 		OutputPath:  opts.Output,
@@ -260,10 +276,10 @@
 		// the client populate identity-aware fields (ConsoleURL etc.).
 		CheckError: ac.CheckResponse,
 	})
-	// MarkRaw: see comment above on the DoAPI path. Skips legacy
-	// *ExitError enrichment; typed errors flow through unchanged.
+	// MarkRaw: see comment above on the DoAPI path. Skips dispatcher hint
+	// enrichment; the typed error's own message / hint flow through unchanged.
 	if err != nil {
-		return output.MarkRaw(err)
+		return errs.MarkRaw(err)
 	}
 	return nil
 }
@@ -279,7 +295,7 @@
 	// When jq is set, always aggregate all pages then filter.
 	if jqExpr != "" {
 		if err := client.PaginateWithJq(ctx, ac, request, jqExpr, out, pagOpts, ac.CheckResponse); err != nil {
-			return output.MarkRaw(err)
+			return errs.MarkRaw(err)
 		}
 		return nil
 	}
@@ -291,11 +307,11 @@
 			pf.FormatPage(items)
 		}, pagOpts)
 		if err != nil {
-			return output.MarkRaw(err)
+			return errs.MarkRaw(err)
 		}
 		if apiErr := ac.CheckResponse(result, pagOpts.Identity); apiErr != nil {
 			output.FormatValue(out, result, output.FormatJSON)
-			return output.MarkRaw(apiErr)
+			return errs.MarkRaw(apiErr)
 		}
 		if !hasItems {
 			fmt.Fprintf(errOut, "warning: this API does not return a list, format %q is not supported, falling back to json\n", format)
@@ -305,11 +321,11 @@
 	default:
 		result, err := ac.PaginateAll(ctx, request, pagOpts)
 		if err != nil {
-			return output.MarkRaw(err)
+			return errs.MarkRaw(err)
 		}
 		if apiErr := ac.CheckResponse(result, pagOpts.Identity); apiErr != nil {
 			output.FormatValue(out, result, output.FormatJSON)
-			return output.MarkRaw(apiErr)
+			return errs.MarkRaw(apiErr)
 		}
 		output.FormatValue(out, result, format)
 		return nil

cmd/api/api_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/larksuite/cli/errs"
 	"github.com/larksuite/cli/internal/cmdutil"
 	"github.com/larksuite/cli/internal/core"
+	"github.com/larksuite/cli/internal/errclass"
 	"github.com/larksuite/cli/internal/httpmock"
 	"github.com/spf13/cobra"
 )
@@ -250,6 +251,28 @@ func TestApiCmd_ParamsAndDataBothStdinConflict(t *testing.T) {
 	if !strings.Contains(err.Error(), "cannot both read from stdin") {
 		t.Errorf("expected stdin conflict error, got: %v", err)
 	}
+	var ve *errs.ValidationError
+	if !errors.As(err, &ve) {
+		t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err)
+	}
+	if ve.Subtype != errs.SubtypeInvalidArgument {
+		t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument)
+	}
+	requireInvalidParamNames(t, ve, "--params", "--data")
+}
+
+// requireInvalidParamNames asserts that ve.Params carries exactly the given
+// flag names (order-sensitive, mirroring declaration order at the call site).
+func requireInvalidParamNames(t *testing.T, ve *errs.ValidationError, names ...string) {
+	t.Helper()
+	if len(ve.Params) != len(names) {
+		t.Fatalf("Params = %+v, want %d entries %v", ve.Params, len(names), names)
+	}
+	for i, name := range names {
+		if ve.Params[i].Name != name {
+			t.Errorf("Params[%d].Name = %q, want %q", i, ve.Params[i].Name, name)
+		}
+	}
 }
 
 func TestApiCmd_OutputAndPageAllConflict(t *testing.T) {
@@ -270,6 +293,41 @@ func TestApiCmd_OutputAndPageAllConflict(t *testing.T) {
 	if gotOpts != nil && !strings.Contains(err.Error(), "mutually exclusive") {
 		t.Errorf("expected 'mutually exclusive' error, got: %v", err)
 	}
+	var ve *errs.ValidationError
+	if !errors.As(err, &ve) {
+		t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err)
+	}
+	if ve.Subtype != errs.SubtypeInvalidArgument {
+		t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument)
+	}
+	requireInvalidParamNames(t, ve, "--output", "--page-all")
+}
+
+// TestApiCmd_FileDataNotObject_TypedValidation pins the typed envelope for
+// the --file + non-object --data rejection: *errs.ValidationError with
+// subtype invalid_argument and the offending flag on Param.
+func TestApiCmd_FileDataNotObject_TypedValidation(t *testing.T) {
+	f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{
+		AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu,
+	})
+	cmd := NewCmdApi(f, func(opts *APIOptions) error {
+		return apiRun(opts)
+	})
+	cmd.SetArgs([]string{"POST", "/open-apis/test", "--as", "bot", "--file", "photo.jpg", "--data", `["not-an-object"]`})
+	err := cmd.Execute()
+	if err == nil {
+		t.Fatal("expected error for non-object --data with --file")
+	}
+	var ve *errs.ValidationError
+	if !errors.As(err, &ve) {
+		t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err)
+	}
+	if ve.Subtype != errs.SubtypeInvalidArgument {
+		t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument)
+	}
+	if ve.Param != "--data" {
+		t.Errorf("Param = %q, want %q", ve.Param, "--data")
+	}
 }
 
 func TestApiCmd_BinaryResponse_AutoSave(t *testing.T) {
@@ -360,6 +418,11 @@ func TestApiCmd_PageAll_NonBatchAPI_ErrorStillOutputsJSON(t *testing.T) {
 	if !strings.Contains(stdout.String(), "no permission") {
 		t.Errorf("expected error message in stdout, got: %s", stdout.String())
 	}
+	// --page-all errors are raw passthrough: the dispatcher must not rewrite
+	// the message / hint with local enrichment.
+	if !errs.IsRaw(err) {
+		t.Errorf("expected --page-all error to be marked raw, got %T: %v", err, err)
+	}
 }
 
 func TestApiCmd_PageAll_BatchAPI_StreamsItems(t *testing.T) {
@@ -737,6 +800,64 @@ func TestApiCmd_PermissionError_DerivesFirstClassFields(t *testing.T) {
 	}
 }
 
+// TestApiCmd_APIError_RawPassthrough pins the raw-passthrough contract of the
+// `api` command: errors returned to the dispatcher are marked raw via
+// errs.MarkRaw, so the dispatcher skips hint enrichment and the message /
+// hint stay exactly what errclass.BuildAPIError derived from the Lark
+// response at classification time — nothing in the api command path rewrites
+// them afterwards.
+func TestApiCmd_APIError_RawPassthrough(t *testing.T) {
+	f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{
+		AppID: "cli_test_raw", AppSecret: "secret", Brand: core.BrandFeishu,
+	})
+
+	respBody := map[string]interface{}{
+		"code": 99991679,
+		"msg":  "scope missing",
+	}
+	reg.Register(&httpmock.Stub{
+		URL:  "/open-apis/docx/v1/documents/raw",
+		Body: respBody,
+	})
+
+	cmd := NewCmdApi(f, nil)
+	cmd.SetArgs([]string{"GET", "/open-apis/docx/v1/documents/raw", "--as", "bot"})
+	err := cmd.Execute()
+	if err == nil {
+		t.Fatal("expected error for non-zero code")
+	}
+
+	if !errs.IsRaw(err) {
+		t.Fatalf("expected error to be marked raw (errs.IsRaw), got %T: %v", err, err)
+	}
+	// The raw marker must not hide the typed error from the envelope writer.
+	var pe *errs.PermissionError
+	if !errors.As(err, &pe) {
+		t.Fatalf("expected *errs.PermissionError through the raw marker, got %T: %v", err, err)
+	}
+
+	// Canonical baseline: classify the same response body the same way
+	// CheckResponse does. Message and hint surfaced by the command must equal
+	// the classification-time values byte for byte.
+	canonical := errclass.BuildAPIError(respBody, errclass.ClassifyContext{
+		Brand: "feishu", AppID: "cli_test_raw", Identity: "bot",
+	})
+	var want *errs.PermissionError
+	if !errors.As(canonical, &want) {
+		t.Fatalf("expected canonical *errs.PermissionError, got %T: %v", canonical, canonical)
+	}
+	if pe.Message != want.Message {
+		t.Errorf("Message = %q, want canonical %q (raw mode must not rewrite it)", pe.Message, want.Message)
+	}
+	if pe.Hint != want.Hint {
+		t.Errorf("Hint = %q, want canonical %q (raw mode must not rewrite it)", pe.Hint, want.Hint)
+	}
+	// The dispatcher-side scope enrichment string must never appear in raw mode.
+	if strings.Contains(pe.Hint, "current command requires scope(s)") {
+		t.Errorf("hint was rewritten by enrichment in raw mode: %q", pe.Hint)
+	}
+}
+
 func TestApiCmd_JsonFlag_Accepted(t *testing.T) {
 	f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{
 		AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu,

cmd/auth/check_test.go

@@ -33,12 +33,9 @@ func TestAuthCheckRun_NotLoggedIn_ExitOneWithStdoutOnly(t *testing.T) {
 	if got := output.ExitCodeOf(err); got != 1 {
 		t.Errorf("exit code = %d, want 1 (predicate 'missing' signal)", got)
 	}
-	var bare *output.ExitError
+	var bare *output.BareError
 	if !errors.As(err, &bare) {
-		t.Fatalf("expected *output.ExitError (ErrBare), got %T: %v", err, err)
-	}
-	if bare.Detail != nil {
-		t.Errorf("ErrBare must carry no Detail (no envelope), got %+v", bare.Detail)
+		t.Fatalf("expected *output.BareError (ErrBare), got %T: %v", err, err)
 	}
 
 	if stderr.Len() != 0 {

cmd/auth/list.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"github.com/larksuite/cli/errs"
 	larkauth "github.com/larksuite/cli/internal/auth"
 	"github.com/larksuite/cli/internal/cmdutil"
 	"github.com/larksuite/cli/internal/core"
@@ -59,7 +60,7 @@ func authListRun(opts *ListOptions) error {
 		// keep the same contract here. We still want the hint to be
 		// workspace-aware, so we pull the message+hint out of
 		// NotConfiguredError() instead of hard-coding it.
-		var cfgErr *core.ConfigError
+		var cfgErr *errs.ConfigError
 		if errors.As(core.NotConfiguredError(), &cfgErr) {
 			fmt.Fprintln(f.IOStreams.ErrOut, cfgErr.Message)
 			if cfgErr.Hint != "" {