Skip to content

Commit 5ee283f

Browse files
committed
feat(task): emit typed error envelopes across the task domain
Task commands now return structured, typed errors instead of the legacy exit-code envelope: every failure carries a stable category, subtype, and recovery hint, so callers can branch on the error class instead of parsing messages. Exit codes derive from the error category — input validation exits 2, a permission denial exits 3, other API errors exit 1. Batch operations (adding tasks to a tasklist, creating a tasklist with tasks) now report partial failure honestly: the per-item successes and failures stay on stdout and the command exits non-zero instead of masking failures as a success.
1 parent ac116e7 commit 5ee283f

40 files changed

Lines changed: 1788 additions & 779 deletions

.golangci.yml

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,21 @@ linters:
7373
- forbidigo
7474
# errs-typed-only enforced on paths already migrated to errs.NewXxxError.
7575
# Add a path when its migration is complete.
76-
- 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/calendar/helpers\.go|shortcuts/drive/|shortcuts/mail/)
76+
- 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/calendar/helpers\.go|shortcuts/drive/|shortcuts/mail/|shortcuts/task/)
7777
text: errs-typed-only
7878
linters:
7979
- forbidigo
8080
# errs-no-bare-wrap enforced on paths fully migrated to typed final
8181
# errors. Scoped separately from errs-typed-only because cmd/auth/,
8282
# cmd/config/ still have residual fmt.Errorf and must not be caught.
83-
- path-except: (shortcuts/drive/|shortcuts/mail/|shortcuts/calendar/helpers\.go|shortcuts/common/mcp_client\.go)
83+
- path-except: (shortcuts/drive/|shortcuts/mail/|shortcuts/task/|shortcuts/calendar/helpers\.go|shortcuts/common/mcp_client\.go)
8484
text: errs-no-bare-wrap
8585
linters:
8686
- forbidigo
87-
# errs-no-legacy-helper is scoped to migrated domains: the shared helpers
88-
# it bans are still used by other domains until their later migration phase.
89-
- path-except: (shortcuts/drive/|shortcuts/mail/)
87+
# errs-no-legacy-helper is enforced on domains fully migrated to typed
88+
# errs.* helpers. The shared helpers it bans are still used by other
89+
# domains until their later migration phase.
90+
- path-except: (shortcuts/drive/|shortcuts/mail/|shortcuts/task/)
9091
text: errs-no-legacy-helper
9192
linters:
9293
- forbidigo
@@ -115,17 +116,17 @@ linters:
115116
msg: >-
116117
[errs-typed-only] use errs.NewXxxError(...) builder
117118
(see errs/types.go).
118-
# ── legacy shared error helpers banned on migrated domains ──
119+
# ── legacy shared error helpers banned on migrated shortcut domains ──
119120
# These helpers internally produce legacy output.Err* shapes, so they
120121
# are invisible to the errs-typed-only ban above. Migrated domains use
121-
# typed errs.* builders or domain-local file-I/O helpers instead; this
122-
# prevents reintroduction while unmigrated domains continue to use the
123-
# shared helpers until their later migration phase.
122+
# domain-local typed helpers (for example driveInputStatError,
123+
# driveSaveError, or taskInputStatError); this prevents reintroduction.
124+
# Other domains still use the shared helpers until later migrations.
124125
- pattern: (common\.FlagErrorf|common\.WrapInputStatError|common\.WrapSaveErrorByCategory)\b
125126
msg: >-
126127
[errs-no-legacy-helper] these shared helpers emit legacy output.Err*
127-
shapes. Use typed errs.NewXxxError builders or a domain-local
128-
file-I/O helper.
128+
shapes. Use typed errs.NewXxxError builders or a domain-local typed
129+
helper such as driveInputStatError, driveSaveError, or taskInputStatError.
129130
# ── bare error wraps banned on fully-typed paths ──
130131
- pattern: (fmt\.Errorf|errors\.New)\b
131132
msg: >-

lint/errscontract/rule_no_legacy_envelope_literal.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
var migratedEnvelopePaths = []string{
1919
"shortcuts/drive/",
2020
"shortcuts/mail/",
21+
"shortcuts/task/",
2122
}
2223

2324
// legacyOutputImportPath is the import path of the package that declares the

lint/errscontract/rule_no_legacy_runtime_api_call.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import (
1818
// forbidigo's errs-typed-only ban does not see them because they are method
1919
// calls, not output.Err* identifiers — this AST rule covers that gap.
2020
//
21-
// Migrated code must call a typed API wrapper (e.g. drive's driveCallAPI) or use
21+
// Migrated code must call the domain's typed API wrapper or use
2222
// runtime.DoAPI + errclass.BuildAPIError directly, so failures classify into
2323
// typed errs.* errors.
2424
//
@@ -53,7 +53,7 @@ func CheckNoLegacyRuntimeAPICall(path, src string) []Violation {
5353
File: path,
5454
Line: fset.Position(call.Pos()).Line,
5555
Message: "runtime." + name + " emits a legacy output.ExitError api_error envelope and downgrades typed network/auth boundary errors; it is forbidden on migrated paths",
56-
Suggestion: "call the domain's typed API wrapper (e.g. driveCallAPI) or runtime.DoAPI + errclass.BuildAPIError " +
56+
Suggestion: "call the domain's typed API wrapper (for example driveCallAPI or callTaskAPITyped) or runtime.DoAPI + errclass.BuildAPIError " +
5757
"so failures classify into typed errs.* errors",
5858
})
5959
}

lint/errscontract/rules_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,27 @@ func boom() error {
618618
}
619619
}
620620

621+
func TestCheckNoLegacyEnvelopeLiteral_RejectsExitErrorLiteralOnTaskPath(t *testing.T) {
622+
src := `package task
623+
624+
import "github.com/larksuite/cli/internal/output"
625+
626+
func boom() error {
627+
return &output.ExitError{Code: 1}
628+
}
629+
`
630+
v := CheckNoLegacyEnvelopeLiteral("shortcuts/task/task_update.go", src)
631+
if len(v) != 1 {
632+
t.Fatalf("expected 1 violation, got %d: %+v", len(v), v)
633+
}
634+
if v[0].Action != ActionReject {
635+
t.Errorf("action = %q, want REJECT", v[0].Action)
636+
}
637+
if !strings.Contains(v[0].Message, "ExitError") {
638+
t.Errorf("message should name the legacy type: %s", v[0].Message)
639+
}
640+
}
641+
621642
func TestCheckNoLegacyEnvelopeLiteral_RejectsErrDetailLiteralOnDrivePath(t *testing.T) {
622643
src := `package drive
623644
@@ -801,6 +822,26 @@ func boom(runtime *common.RuntimeContext) error {
801822
}
802823
}
803824

825+
func TestCheckNoLegacyRuntimeAPICall_RejectsCallAPIOnTaskPath(t *testing.T) {
826+
src := `package task
827+
828+
func boom(runtime *common.RuntimeContext) error {
829+
_, err := runtime.CallAPI("POST", "/x", nil, nil)
830+
return err
831+
}
832+
`
833+
v := CheckNoLegacyRuntimeAPICall("shortcuts/task/task_update.go", src)
834+
if len(v) != 1 {
835+
t.Fatalf("expected 1 violation, got %d: %+v", len(v), v)
836+
}
837+
if v[0].Action != ActionReject {
838+
t.Errorf("action = %q, want REJECT", v[0].Action)
839+
}
840+
if !strings.Contains(v[0].Message, "CallAPI") {
841+
t.Errorf("message should name the legacy method: %s", v[0].Message)
842+
}
843+
}
844+
804845
func TestCheckNoLegacyRuntimeAPICall_RejectsDoAPIJSONWithLogIDOnDrivePath(t *testing.T) {
805846
src := `package drive
806847

shortcuts/task/shortcuts.go

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,8 @@ import (
1313
"strings"
1414
"time"
1515

16-
"github.com/larksuite/cli/internal/output"
16+
"github.com/larksuite/cli/errs"
1717
"github.com/larksuite/cli/shortcuts/common"
18-
larkcore "github.com/larksuite/oapi-sdk-go/v3/core"
1918
)
2019

2120
func inferTaskMemberType(id string) string {
@@ -107,7 +106,7 @@ func buildTaskCreateBody(runtime *common.RuntimeContext) (map[string]interface{}
107106
// Handle generic JSON payload if provided
108107
if dataStr := runtime.Str("data"); dataStr != "" {
109108
if err := json.Unmarshal([]byte(dataStr), &body); err != nil {
110-
return nil, output.ErrValidation("--data must be a valid JSON object: %v", err)
109+
return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--data must be a valid JSON object: %v", err).WithParam("--data")
111110
}
112111
}
113112

@@ -143,7 +142,7 @@ func buildTaskCreateBody(runtime *common.RuntimeContext) (map[string]interface{}
143142
if dueStr := runtime.Str("due"); dueStr != "" {
144143
dueObj, err := parseTaskTime(dueStr)
145144
if err != nil {
146-
return nil, output.ErrValidation("failed to parse due time: %v", err)
145+
return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "failed to parse due time: %v", err).WithParam("--due")
147146
}
148147
body["due"] = dueObj
149148
}
@@ -154,7 +153,7 @@ func buildTaskCreateBody(runtime *common.RuntimeContext) (map[string]interface{}
154153

155154
summary, _ := body["summary"].(string)
156155
if strings.TrimSpace(summary) == "" {
157-
return nil, output.ErrValidation("task summary is required")
156+
return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "task summary is required").WithParam("--summary")
158157
}
159158

160159
return body, nil
@@ -194,27 +193,11 @@ var CreateTask = common.Shortcut{
194193
Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
195194
body, err := buildTaskCreateBody(runtime)
196195
if err != nil {
197-
return WrapTaskError(ErrCodeTaskInvalidParams, err.Error(), "create task")
198-
}
199-
200-
queryParams := make(larkcore.QueryParams)
201-
queryParams.Set("user_id_type", "open_id")
202-
203-
apiResp, err := runtime.DoAPI(&larkcore.ApiReq{
204-
HttpMethod: http.MethodPost,
205-
ApiPath: "/open-apis/task/v2/tasks",
206-
QueryParams: queryParams,
207-
Body: body,
208-
})
209-
210-
var result map[string]interface{}
211-
if err == nil {
212-
if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil {
213-
return output.Errorf(output.ExitAPI, "api_error", "failed to parse response: %v", parseErr)
214-
}
196+
return err
215197
}
216198

217-
data, err := HandleTaskApiResult(result, err, "create task")
199+
params := map[string]interface{}{"user_id_type": "open_id"}
200+
data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks", params, body)
218201
if err != nil {
219202
return err
220203
}

shortcuts/task/task_assign.go

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,13 @@ package task
55

66
import (
77
"context"
8-
"encoding/json"
98
"fmt"
109
"io"
1110
"net/http"
1211
"net/url"
1312
"strings"
1413

15-
larkcore "github.com/larksuite/oapi-sdk-go/v3/core"
16-
14+
"github.com/larksuite/cli/errs"
1715
"github.com/larksuite/cli/shortcuts/common"
1816
)
1917

@@ -35,7 +33,7 @@ var AssignTask = common.Shortcut{
3533

3634
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
3735
if runtime.Str("add") == "" && runtime.Str("remove") == "" {
38-
return WrapTaskError(ErrCodeTaskInvalidParams, "must specify either --add or --remove", "validate assign")
36+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "must specify either --add or --remove")
3937
}
4038
return nil
4139
},
@@ -62,28 +60,13 @@ var AssignTask = common.Shortcut{
6260

6361
Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
6462
taskId := url.PathEscape(runtime.Str("task-id"))
65-
queryParams := make(larkcore.QueryParams)
66-
queryParams.Set("user_id_type", "open_id")
63+
params := map[string]interface{}{"user_id_type": "open_id"}
6764

6865
var lastData map[string]interface{}
6966

7067
if addStr := runtime.Str("add"); addStr != "" {
7168
body := buildMembersBody(addStr, "assignee", runtime.Str("idempotency-key"))
72-
apiResp, err := runtime.DoAPI(&larkcore.ApiReq{
73-
HttpMethod: http.MethodPost,
74-
ApiPath: "/open-apis/task/v2/tasks/" + taskId + "/add_members",
75-
QueryParams: queryParams,
76-
Body: body,
77-
})
78-
79-
var result map[string]interface{}
80-
if err == nil {
81-
if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil {
82-
return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse add members")
83-
}
84-
}
85-
86-
data, err := HandleTaskApiResult(result, err, "add task members")
69+
data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks/"+taskId+"/add_members", params, body)
8770
if err != nil {
8871
return err
8972
}
@@ -92,21 +75,7 @@ var AssignTask = common.Shortcut{
9275

9376
if removeStr := runtime.Str("remove"); removeStr != "" {
9477
body := buildMembersBody(removeStr, "assignee", "")
95-
apiResp, err := runtime.DoAPI(&larkcore.ApiReq{
96-
HttpMethod: http.MethodPost,
97-
ApiPath: "/open-apis/task/v2/tasks/" + taskId + "/remove_members",
98-
QueryParams: queryParams,
99-
Body: body,
100-
})
101-
102-
var result map[string]interface{}
103-
if err == nil {
104-
if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil {
105-
return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse remove members")
106-
}
107-
}
108-
109-
data, err := HandleTaskApiResult(result, err, "remove task members")
78+
data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks/"+taskId+"/remove_members", params, body)
11079
if err != nil {
11180
return err
11281
}

shortcuts/task/task_assign_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,100 @@
44
package task
55

66
import (
7+
"errors"
78
"testing"
89

910
"github.com/spf13/cobra"
1011

12+
"github.com/larksuite/cli/errs"
13+
"github.com/larksuite/cli/internal/httpmock"
14+
"github.com/larksuite/cli/internal/output"
1115
"github.com/larksuite/cli/shortcuts/common"
1216
"github.com/smartystreets/goconvey/convey"
1317
)
1418

19+
// TestAssignTask_RequiresAddOrRemove covers the Validate guard: neither --add
20+
// nor --remove yields a typed validation error (exit 2) before any API call.
21+
func TestAssignTask_RequiresAddOrRemove(t *testing.T) {
22+
f, stdout, _, _ := taskShortcutTestFactory(t)
23+
24+
s := AssignTask
25+
args := []string{"+assign", "--task-id", "task-1", "--as", "bot", "--format", "json"}
26+
err := runMountedTaskShortcut(t, s, args, f, stdout)
27+
28+
var ve *errs.ValidationError
29+
if !errors.As(err, &ve) {
30+
t.Fatalf("err = %T, want *errs.ValidationError; err = %v", err, err)
31+
}
32+
if ve.Subtype != errs.SubtypeInvalidArgument {
33+
t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument)
34+
}
35+
if got := output.ExitCodeOf(err); got != output.ExitValidation {
36+
t.Errorf("exit code = %d, want %d", got, output.ExitValidation)
37+
}
38+
}
39+
40+
// TestAssignTask_MalformedResponse covers the Execute parse-response arm: a
41+
// 200 with an unparseable body surfaces a typed internal invalid_response
42+
// error (exit 5).
43+
func TestAssignTask_MalformedResponse(t *testing.T) {
44+
f, stdout, _, reg := taskShortcutTestFactory(t)
45+
warmTenantToken(t, f, reg)
46+
47+
reg.Register(&httpmock.Stub{
48+
Method: "POST",
49+
URL: "/open-apis/task/v2/tasks/task-1/add_members",
50+
Status: 200,
51+
RawBody: []byte("{not-json"),
52+
})
53+
54+
s := AssignTask
55+
args := []string{"+assign", "--task-id", "task-1", "--add", "ou_user_1", "--as", "bot", "--format", "json"}
56+
err := runMountedTaskShortcut(t, s, args, f, stdout)
57+
58+
var ie *errs.InternalError
59+
if !errors.As(err, &ie) {
60+
t.Fatalf("err = %T, want *errs.InternalError; err = %v", err, err)
61+
}
62+
if ie.Subtype != errs.SubtypeInvalidResponse {
63+
t.Errorf("subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse)
64+
}
65+
if got := output.ExitCodeOf(err); got != output.ExitInternal {
66+
t.Errorf("exit code = %d, want %d", got, output.ExitInternal)
67+
}
68+
}
69+
70+
// TestAssignTask_MalformedResponse_RemoveArm covers the Execute remove-members
71+
// parse arm: with only --remove set, the add arm is skipped and the
72+
// remove_members POST returns a 200 with an unparseable body, which must
73+
// surface a typed internal invalid_response error (exit 5).
74+
func TestAssignTask_MalformedResponse_RemoveArm(t *testing.T) {
75+
f, stdout, _, reg := taskShortcutTestFactory(t)
76+
warmTenantToken(t, f, reg)
77+
78+
reg.Register(&httpmock.Stub{
79+
Method: "POST",
80+
URL: "/open-apis/task/v2/tasks/task-1/remove_members",
81+
Status: 200,
82+
RawBody: []byte("{not-json"),
83+
})
84+
85+
s := AssignTask
86+
args := []string{"+assign", "--task-id", "task-1", "--remove", "ou_user_1", "--as", "bot", "--format", "json"}
87+
err := runMountedTaskShortcut(t, s, args, f, stdout)
88+
89+
var ie *errs.InternalError
90+
if !errors.As(err, &ie) {
91+
t.Fatalf("err = %T, want *errs.InternalError; err = %v", err, err)
92+
}
93+
if ie.Subtype != errs.SubtypeInvalidResponse {
94+
t.Errorf("subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse)
95+
}
96+
if got := output.ExitCodeOf(err); got != output.ExitInternal {
97+
t.Errorf("exit code = %d, want %d", got, output.ExitInternal)
98+
}
99+
}
100+
15101
func TestBuildMembersBody(t *testing.T) {
16102
convey.Convey("Build with ids and token", t, func() {
17103
body := buildMembersBody("u1, u2 , ", "assignee", "token1")

0 commit comments

Comments
 (0)