Compare commits

...

1 Commits

Author SHA1 Message Date
houzhicong
6bd3c4b601 fix(minutes,vc): align failure and validation semantics 2026-06-08 17:05:55 +08:00
6 changed files with 97 additions and 55 deletions

View File

@@ -80,6 +80,11 @@ var MinutesDownload = common.Shortcut{
if err := common.ValidateSafePathTyped(runtime.FileIO(), outDir); err != nil {
return err
}
if fi, statErr := runtime.FileIO().Stat(outDir); statErr == nil && !fi.IsDir() {
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--output-dir %q must be a directory", outDir).WithParam("--output-dir")
} else if statErr != nil && !errors.Is(statErr, fs.ErrNotExist) {
return errs.NewInternalError(errs.SubtypeFileIO, "cannot access --output-dir %q: %s", outDir, statErr).WithCause(statErr)
}
}
return nil
},

View File

@@ -588,6 +588,34 @@ func TestDownload_OutputDirFlag_SingleToken(t *testing.T) {
}
}
func TestDownload_Validation_OutputDirIsExistingFile(t *testing.T) {
chdir(t, t.TempDir())
if err := os.WriteFile("not-a-dir", []byte("x"), 0644); err != nil {
t.Fatalf("setup: %v", err)
}
f, _, _, _ := cmdutil.TestFactory(t, defaultConfig())
err := mountAndRun(t, MinutesDownload, []string{
"+download", "--minute-tokens", "tok001", "--output-dir", "not-a-dir", "--as", "user",
}, f, nil)
if err == nil {
t.Fatal("expected validation error for --output-dir pointing at a 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 != "--output-dir" {
t.Errorf("Param = %q, want --output-dir", ve.Param)
}
if !strings.Contains(err.Error(), "must be a directory") {
t.Errorf("expected directory validation message, got: %v", err)
}
}
func TestDownload_Batch_OutputNonExistentPath(t *testing.T) {
// Batch mode with --output pointing at a path that doesn't exist yet:
// auto-upgrade to --output-dir semantics and create the directory.

View File

@@ -76,9 +76,11 @@ func minutesReadError(err error, minuteToken string) error {
if !ok || p.Code != minutesNoReadPermissionCode {
return err
}
p.Message = fmt.Sprintf("No read permission for minute %s: cannot query the minute.", minuteToken)
p.Hint = "Ask the minute owner for minute file read permission"
return err
return errs.NewPermissionError(errs.SubtypePermissionDenied,
"No read permission for minute %s: cannot query the minute.", minuteToken).
WithCode(minutesNoReadPermissionCode).
WithHint("Ask the minute owner for minute file read permission").
WithCause(err)
}
// validMinuteToken matches the server's minute-token format and blocks any
@@ -367,18 +369,28 @@ func joinErrors(msgs ...string) string {
return strings.Join(parts, "; ")
}
// hasNotesPayload reports whether a result map carries any usable note or
// minute payload, irrespective of partial failures surfaced via `error`.
// hasNotesPayload reports whether a result map carries usable note or minute
// payload. An echoed --minute-tokens input alone is not treated as payload when
// that item also carries an error; a meeting/calendar-derived minute_token is.
func hasNotesPayload(m map[string]any) bool {
if m == nil {
return false
}
for _, k := range []string{"note_doc_token", "verbatim_doc_token", "minute_token", "meeting_notes", "shared_doc_tokens", "artifacts"} {
for _, k := range []string{"note_doc_token", "verbatim_doc_token", "meeting_notes", "shared_doc_tokens", "artifacts"} {
if v, ok := m[k]; ok && v != nil && v != "" {
return true
}
}
return false
minuteToken, _ := m["minute_token"].(string)
if minuteToken == "" {
return false
}
if errMsg, _ := m["error"].(string); errMsg == "" {
return true
}
_, fromMeeting := m["meeting_id"]
_, fromCalendarEvent := m["calendar_event_id"]
return fromMeeting || fromCalendarEvent
}
// fetchNoteByMinuteToken queries notes via minute_token.

View File

@@ -1219,29 +1219,38 @@ func minuteGetErrStub(token string, code int, msg string) *httpmock.Stub {
}
}
// TestMinutesReadError_ProblemOf_EnrichesMessage pins that minutesReadError
// mutates the typed error's Message and Hint in-place via errs.ProblemOf when
// the server returns code 2091005 (minutes no-read-permission).
func TestMinutesReadError_ProblemOf_EnrichesMessage(t *testing.T) {
// TestMinutesReadError_AllFailed_OutPartialFailure verifies that a failing
// minute-token lookup is treated as a failed item even though the result keeps
// the input minute_token for caller correlation.
func TestMinutesReadError_AllFailed_OutPartialFailure(t *testing.T) {
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig())
reg.Register(minuteGetErrStub("tokperm", minutesNoReadPermissionCode, "no permission"))
// artifactsStub not needed: we never reach it on error
// A single minute-token that fails on a no-read-permission code still
// produces a note carrying minute_token, so the batch exits 0 with the
// enriched error surfaced inline rather than becoming an all-fail.
if err := mountAndRun(t, VCNotes, []string{"+notes", "--minute-tokens", "tokperm", "--as", "user"}, f, stdout); err != nil {
t.Fatalf("unexpected error: %v", err)
err := mountAndRun(t, VCNotes, []string{"+notes", "--minute-tokens", "tokperm", "--as", "user"}, f, stdout)
if err == nil {
t.Fatal("expected all-failed minute-token query to return an error")
}
var pfErr *output.PartialFailureError
if !errors.As(err, &pfErr) {
t.Fatalf("expected *output.PartialFailureError, got %T: %v", err, err)
}
if pfErr.Code != output.ExitAPI {
t.Errorf("PartialFailureError.Code = %d, want ExitAPI (%d)", pfErr.Code, output.ExitAPI)
}
// stdout carries the note with the enriched error/hint
var resp map[string]any
var resp struct {
OK bool `json:"ok"`
Data map[string]any `json:"data"`
}
if parseErr := json.Unmarshal(stdout.Bytes(), &resp); parseErr != nil {
t.Fatalf("unmarshal stdout: %v\n%s", parseErr, stdout.String())
}
data, _ := resp["data"].(map[string]any)
notes, _ := data["notes"].([]any)
if resp.OK {
t.Errorf("ok must be false on all-failed minute-token query, got ok:true")
}
notes, _ := resp.Data["notes"].([]any)
if len(notes) != 1 {
t.Fatalf("expected 1 note, got %d", len(notes))
}

View File

@@ -20,6 +20,7 @@ import (
"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/internal/auth"
"github.com/larksuite/cli/internal/credential"
"github.com/larksuite/cli/internal/output"
"github.com/larksuite/cli/internal/validate"
"github.com/larksuite/cli/shortcuts/common"
@@ -97,7 +98,7 @@ var VCRecording = common.Shortcut{
{Name: "meeting-ids", Desc: "meeting IDs, comma-separated for batch"},
{Name: "calendar-event-ids", Desc: "calendar event instance IDs, comma-separated for batch"},
},
Validate: func(_ context.Context, runtime *common.RuntimeContext) error {
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
if err := common.ExactlyOneTyped(runtime, "meeting-ids", "calendar-event-ids"); err != nil {
return err
}
@@ -116,18 +117,14 @@ var VCRecording = common.Shortcut{
case runtime.Str("calendar-event-ids") != "":
required = scopesRecordingCalendarEventIDs
}
appID := runtime.Config.AppID
userOpenID := runtime.UserOpenId()
if appID != "" && userOpenID != "" {
stored := auth.GetStoredToken(appID, userOpenID)
if stored != nil {
if missing := auth.MissingScopes(stored.Scope, required); len(missing) > 0 {
return errs.NewPermissionError(errs.SubtypeMissingScope,
"missing required scope(s): %s", strings.Join(missing, ", ")).
WithHint("run `lark-cli auth login --scope %q` in the background. It blocks and outputs a verification URL — retrieve the URL and open it in a browser to complete login.", strings.Join(missing, " ")).
WithMissingScopes(missing...).
WithIdentity(string(runtime.As()))
}
result, err := runtime.Factory.Credential.ResolveToken(ctx, credential.NewTokenSpec(runtime.As(), runtime.Config.AppID))
if err == nil && result != nil && result.Scopes != "" {
if missing := auth.MissingScopes(result.Scopes, required); len(missing) > 0 {
return errs.NewPermissionError(errs.SubtypeMissingScope,
"missing required scope(s): %s", strings.Join(missing, ", ")).
WithHint("run `lark-cli auth login --scope %q` in the background. It blocks and outputs a verification URL — retrieve the URL and open it in a browser to complete login.", strings.Join(missing, " ")).
WithMissingScopes(missing...).
WithIdentity(string(runtime.As()))
}
}
return nil

View File

@@ -10,19 +10,25 @@ import (
"fmt"
"strings"
"testing"
"time"
"github.com/spf13/cobra"
keyring "github.com/zalando/go-keyring"
"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/internal/auth"
"github.com/larksuite/cli/internal/cmdutil"
"github.com/larksuite/cli/internal/credential"
"github.com/larksuite/cli/internal/httpmock"
"github.com/larksuite/cli/internal/output"
"github.com/larksuite/cli/shortcuts/common"
)
type recordingScopeTokenResolver struct {
scopes string
}
func (r recordingScopeTokenResolver) ResolveToken(context.Context, credential.TokenSpec) (*credential.TokenResult, error) {
return &credential.TokenResult{Token: "test-token", Scopes: r.scopes}, nil
}
// ---------------------------------------------------------------------------
// Unit tests: extractMinuteToken
// ---------------------------------------------------------------------------
@@ -141,27 +147,12 @@ func TestRecording_BatchLimit_CalendarEventIDs(t *testing.T) {
}
func TestRecording_Validate_MissingScope(t *testing.T) {
keyring.MockInit() // use in-memory keyring to avoid macOS keychain popups
t.Setenv("HOME", t.TempDir())
cfg := defaultConfig()
// Store a token that intentionally lacks the vc:record:readonly scope.
token := &auth.StoredUAToken{
UserOpenId: cfg.UserOpenId,
AppId: cfg.AppID,
AccessToken: "test-user-access-token",
RefreshToken: "test-refresh-token",
ExpiresAt: time.Now().Add(1 * time.Hour).UnixMilli(),
RefreshExpiresAt: time.Now().Add(24 * time.Hour).UnixMilli(),
Scope: "calendar:calendar:read",
GrantedAt: time.Now().Add(-1 * time.Hour).UnixMilli(),
}
if err := auth.SetStoredToken(token); err != nil {
t.Fatalf("SetStoredToken() error = %v", err)
}
t.Cleanup(func() { _ = auth.RemoveStoredToken(cfg.AppID, cfg.UserOpenId) })
f, _, _, _ := cmdutil.TestFactory(t, cfg)
f.Credential = credential.NewCredentialProvider(nil, nil, recordingScopeTokenResolver{
scopes: "calendar:calendar:read",
}, nil)
err := mountAndRun(t, VCRecording, []string{"+recording", "--meeting-ids", "m001", "--as", "user"}, f, nil)
if err == nil {
t.Fatal("expected missing_scope error, got nil")