mirror of
https://github.com/larksuite/cli.git
synced 2026-09-14 18:42:53 +08:00
c6c040c2c5
* refactor(sheets)!: remove legacy sheets command surface
The `shortcuts/sheets/backward` package kept 42 pre-refactor command names
(`+create`, `+read`, `+write`, `+create-sheet`, `+media-upload`, ...) alive
alongside the refactored ones. Monitoring puts their combined share below 5%,
so they are dropped along with the machinery that carried them.
Removed with the package:
- the `sheetsAliasReplacement` map and `wrapSheetsBackwardDeprecation`, which
tagged each alias with a `_notice` deprecation envelope on execution;
- the deprecated cobra group and the custom `sheets --help` usage template that
existed only to hide it. `applySheetsCompatGroups` becomes
`applySheetsCommandGroups`: it still groups the `+`-shortcuts so the OpenAPI
metaapi subcommands keep filing under cobra's stock "Additional Commands".
The deleted package owned no shared logic. Its `parent_type` mapping for image
uploads was, by its own header, a deliberate mirror of the canonical one in
`shortcuts/sheets/helpers.go`; `common.IsLocalOfficeToken` and the drive upload
helpers are untouched and keep their other callers.
E2E tests still drove the removed commands and are ported to the refactored
surface: `+workbook-create` / `+workbook-info` / `+cells-set` / `+cells-get` /
`+cells-search` / `+sheet-*`. The sub-sheet dry-run assertions had to be
rewritten rather than renamed, because the old commands posted to
`sheets/v2/sheets_batch_update` while the new ones invoke
`modify_workbook_structure` over `sheet_ai/v2`. `+update-sheet` fanned out to
`+sheet-rename` + `+sheet-hide` + `+dim-freeze`. Every migrated command was
verified against a live workbook, which is where the assertions come from:
rename and hide answer with a bare revision counter, so their effect is read
back from `+workbook-info` (`sheet_name`, `is_hidden`).
Also updated, since these referenced the removed surface:
- three `skills/lark-drive` reference docs that instructed agents to run
`sheets +read` / `sheets +find`; these ship embedded in the binary, so the
instructions would have produced unknown-subcommand errors;
- `skill-template/domains/sheets.md`, deleted: every sheets command it named
was removed and the cell payload shape it taught
(`{"type":"formula","text":...}`) is rejected by `+cells-set`;
- stale comments naming `backward.uploadSheetMediaFile` and
`backward/helpers.go`.
`Shortcut.OnInvoke` and `internal/deprecation` now have no producers. Both are
generic framework plumbing wired into the `_notice` envelope in `cmd/root.go`,
so they are left in place; the `OnInvoke` doc comment no longer claims a caller.
BREAKING CHANGE: removes the 42 pre-refactor sheets commands (`+create`,
`+read`, `+write`, `+append`, `+find`, `+set-style`, `+create-sheet`,
`+update-sheet`, `+add-dimension`, `+set-dropdown`, `+media-upload`,
`+create-filter-view`, ...). They now fail with `unknown subcommand` and carry
no deprecation notice, so a caller still on the old names gets no migration
pointer at runtime.
Replacements for all 42, plus the differences that are not simple renames — the
cell payload vocabulary (`{"type":"formula","text":...}` is now rejected),
response field paths, and `+update-sheet` / `+update-dimension` fanning out to
several commands — are documented in
skills/lark-sheets/references/lark-sheets-legacy-command-migration.md, reachable
at runtime via:
lark-cli skills read lark-sheets references/lark-sheets-legacy-command-migration.md
* test(sheets): close the assertion gaps found in review
- The append subtest asserted only the ok envelope, so a +cells-set that
reported success without persisting would pass; the later +cells-search
covers row 2 only. Read A4:C4 back and assert the row landed. Verified
non-vacuous against a live sheet: an unwritten row returns cells carrying
no value, so the read-back fails if the write does not persist.
- Cover the omitted-title +sheet-copy path. The comment on the empty-title
suite states an omitted --title means "let the server name the copy", but
nothing exercised it; the new case pins that new_name is absent from the
payload rather than sent empty.
- Assert tool_name in the shared dry-run loop instead of only in the create
case, so copy / delete / rename / move cannot pass by selecting a different
tool on the same /tools/invoke_write endpoint.
* docs(sheets): fix the migration guide examples found in review
- `+table-put --sheets` requires the `{"sheets":[…]}` envelope; the `+append`
row showed a bare array, which the flag rejects outright ("top level must be
the object {\"sheets\":[…]}, got a bare JSON array"). Verified both forms
against a dry-run before and after.
- Tag the diagnostic fence as `text` (markdownlint MD040).
259 lines
8.7 KiB
Go
259 lines
8.7 KiB
Go
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
|
||
// SPDX-License-Identifier: MIT
|
||
|
||
package sheets
|
||
|
||
import (
|
||
"errors"
|
||
"regexp"
|
||
"strings"
|
||
"testing"
|
||
|
||
"github.com/spf13/cobra"
|
||
|
||
"github.com/larksuite/cli/errs"
|
||
)
|
||
|
||
// registeredCommands is every name mounted on the sheets group.
|
||
func registeredCommands(t *testing.T) map[string]bool {
|
||
t.Helper()
|
||
names := map[string]bool{}
|
||
for _, s := range Shortcuts() {
|
||
names[s.Command] = true
|
||
}
|
||
if len(names) == 0 {
|
||
t.Fatal("no sheets shortcuts registered; the assertions below would pass vacuously")
|
||
}
|
||
return names
|
||
}
|
||
|
||
// sheetsGroupWithHints mounts every prescribed target, since the hook resolves
|
||
// targets against the live tree.
|
||
func sheetsGroupWithHints() *cobra.Command {
|
||
svc := &cobra.Command{Use: "sheets"}
|
||
root := &cobra.Command{Use: "lark-cli"}
|
||
root.AddCommand(svc)
|
||
mounted := map[string]bool{}
|
||
for _, rx := range unknownSubcommandHints {
|
||
if mounted[rx.Command] {
|
||
continue
|
||
}
|
||
mounted[rx.Command] = true
|
||
svc.AddCommand(&cobra.Command{
|
||
Use: rx.Command,
|
||
RunE: func(*cobra.Command, []string) error { return nil },
|
||
})
|
||
}
|
||
InstallUnknownSubcommandHints(svc)
|
||
return svc
|
||
}
|
||
|
||
// A rename on the target side would turn a hint into the very failure it was
|
||
// written to fix: an agent sent to a subcommand the CLI rejects.
|
||
func TestPrescribedTargetsExist(t *testing.T) {
|
||
real := registeredCommands(t)
|
||
|
||
for typed, rx := range unknownSubcommandHints {
|
||
if rx.Command == "" {
|
||
t.Errorf("%s prescribes no command; a name that points at more than one belongs with the ranker", typed)
|
||
continue
|
||
}
|
||
if !real[rx.Command] {
|
||
t.Errorf("%s prescribes %q, which is not a registered sheets shortcut", typed, rx.Command)
|
||
}
|
||
}
|
||
}
|
||
|
||
// The hook answers before cobra dispatches, so a key naming a real command would
|
||
// shadow a working command with an error.
|
||
func TestPrescribedNamesAreNotRealCommands(t *testing.T) {
|
||
real := registeredCommands(t)
|
||
|
||
for typed := range unknownSubcommandHints {
|
||
if real[typed] {
|
||
t.Errorf("%s is a registered shortcut; the hook would shadow it", typed)
|
||
}
|
||
}
|
||
}
|
||
|
||
// The hint replaces the ranked candidate list, so an empty one leaves the caller
|
||
// with less than they had.
|
||
func TestEveryPrescriptionCarriesAHint(t *testing.T) {
|
||
for typed, rx := range unknownSubcommandHints {
|
||
if strings.TrimSpace(rx.Hint) == "" {
|
||
t.Errorf("%s has no hint", typed)
|
||
}
|
||
if !strings.Contains(rx.Hint, rx.Command) {
|
||
t.Errorf("%s: hint should spell the target %q so the prose and the machine-readable suggestion agree, got %q",
|
||
typed, rx.Command, rx.Hint)
|
||
}
|
||
}
|
||
}
|
||
|
||
// TestPrescribedExamplesActuallyValidate runs the flags a hint spells out
|
||
// through the command it names. A hint is the only thing the caller gets back —
|
||
// the ranked candidate list is replaced — so one whose own example fails
|
||
// validation costs them a round trip and teaches the wrong shape. The +cells-put
|
||
// hint shipped exactly that: a 1×2 matrix against A1:B2, plus prose forbidding
|
||
// the bare scalars this domain now accepts.
|
||
func TestPrescribedExamplesActuallyValidate(t *testing.T) {
|
||
t.Parallel()
|
||
// Pull the example out of the hint rather than restating it, so prose and
|
||
// assertion cannot drift apart.
|
||
flagRe := regexp.MustCompile(`--range (\S+) --cells '([^']*)'`)
|
||
m := flagRe.FindStringSubmatch(unknownSubcommandHints["+cells-put"].Hint)
|
||
if m == nil {
|
||
t.Fatalf("+cells-put hint no longer spells a --range/--cells example: %q", unknownSubcommandHints["+cells-put"].Hint)
|
||
}
|
||
if _, _, err := runShortcutCapturingErr(t, CellsSet, []string{
|
||
"--url", testURL,
|
||
"--sheet-name", "S1",
|
||
"--range", m[1],
|
||
"--cells", m[2],
|
||
"--dry-run",
|
||
}); err != nil {
|
||
t.Errorf("the +cells-put hint prescribes --range %s --cells %s, which does not validate: %v", m[1], m[2], err)
|
||
}
|
||
}
|
||
|
||
// Pinned so a table edit that drops one of these fails here instead of silently
|
||
// regressing.
|
||
func TestCoreEntriesArePinned(t *testing.T) {
|
||
pinned := map[string]string{
|
||
"+sheet-add": "+sheet-create",
|
||
"+col-resize": "+cols-resize",
|
||
}
|
||
|
||
for typed, want := range pinned {
|
||
rx, ok := unknownSubcommandHints[typed]
|
||
if !ok {
|
||
t.Errorf("%s is not prescribed", typed)
|
||
continue
|
||
}
|
||
if rx.Command != want {
|
||
t.Errorf("%s prescribes %q, want %q", typed, rx.Command, want)
|
||
}
|
||
}
|
||
}
|
||
|
||
func TestHookReturnsPrescriptionAsValidationError(t *testing.T) {
|
||
svc := sheetsGroupWithHints()
|
||
|
||
err := svc.Args(svc, []string{"+sheet-add"})
|
||
var verr *errs.ValidationError
|
||
if !errors.As(err, &verr) {
|
||
t.Fatalf("expected *errs.ValidationError, got %T", err)
|
||
}
|
||
// The name genuinely does not exist, so the message stays byte-compatible
|
||
// with the framework guard's wording; only the hint improves.
|
||
if verr.Message != `unknown subcommand "+sheet-add" for "lark-cli sheets"` {
|
||
t.Errorf("message = %q, want the framework guard's wording", verr.Message)
|
||
}
|
||
if !strings.Contains(verr.Hint, "+sheet-create") {
|
||
t.Errorf("hint should name the real command, got %q", verr.Hint)
|
||
}
|
||
if len(verr.Params) != 1 || len(verr.Params[0].Suggestions) != 1 ||
|
||
verr.Params[0].Suggestions[0] != "+sheet-create" {
|
||
t.Errorf("the prescribed command should be the sole machine-readable suggestion, got %+v", verr.Params)
|
||
}
|
||
}
|
||
|
||
// Names this table does not claim must reach the framework's ranked path: nil is
|
||
// what lets cobra proceed to the guard's RunE.
|
||
func TestUnclaimedNamesFallThrough(t *testing.T) {
|
||
svc := sheetsGroupWithHints()
|
||
|
||
// Near-miss spellings this table deliberately does not claim.
|
||
for _, name := range []string{
|
||
"+searh", "+bogus-xyz", "+cells-get",
|
||
"+sheet-list", "+columns-resize", "+col-size-set", "+column-set-width",
|
||
"+columns-set-width", "+cells-set-column-width", "+dimension-size",
|
||
} {
|
||
if err := svc.Args(svc, []string{name}); err != nil {
|
||
t.Errorf("Args(%q) = %v, want nil so the framework ranking still answers", name, err)
|
||
}
|
||
}
|
||
// A bare group carries no args at all and must still reach its help path.
|
||
if err := svc.Args(svc, nil); err != nil {
|
||
t.Errorf("Args(nil) = %v, want nil", err)
|
||
}
|
||
}
|
||
|
||
// Underscore and casing variants reach the same entry, so the table stays keyed
|
||
// by the canonical name alone.
|
||
func TestHookFoldsUnderscoreAndCase(t *testing.T) {
|
||
svc := sheetsGroupWithHints()
|
||
|
||
for _, typed := range []string{"+sheet_add", "+Sheet-Add", "+SHEET_ADD"} {
|
||
err := svc.Args(svc, []string{typed})
|
||
var verr *errs.ValidationError
|
||
if !errors.As(err, &verr) {
|
||
t.Errorf("Args(%q) = %v, want the +sheet-add prescription", typed, err)
|
||
continue
|
||
}
|
||
// The message echoes the name as typed, not the folded key.
|
||
if !strings.Contains(verr.Message, typed) {
|
||
t.Errorf("message should echo %q as typed, got %q", typed, verr.Message)
|
||
}
|
||
}
|
||
}
|
||
|
||
// Compose with, not discard, an Args validator already on the group: a later
|
||
// framework change that sets one must not be silently dropped.
|
||
func TestInstallPreservesAnInheritedValidator(t *testing.T) {
|
||
svc := &cobra.Command{Use: "sheets"}
|
||
svc.AddCommand(&cobra.Command{
|
||
Use: "+sheet-create",
|
||
RunE: func(*cobra.Command, []string) error { return nil },
|
||
})
|
||
sentinel := errors.New("inherited validator ran")
|
||
svc.Args = func(*cobra.Command, []string) error { return sentinel }
|
||
|
||
InstallUnknownSubcommandHints(svc)
|
||
|
||
if err := svc.Args(svc, []string{"+bogus"}); !errors.Is(err, sentinel) {
|
||
t.Errorf("unclaimed name should reach the inherited validator, got %v", err)
|
||
}
|
||
// A claimed name is still answered here, ahead of the inherited validator.
|
||
if err := svc.Args(svc, []string{"+sheet-add"}); errors.Is(err, sentinel) {
|
||
t.Error("a prescribed name should be answered by the hook, not the inherited validator")
|
||
}
|
||
}
|
||
|
||
func TestInstallOnNilGroupIsNoop(t *testing.T) {
|
||
InstallUnknownSubcommandHints(nil) // must not panic
|
||
}
|
||
|
||
// A concealed distribution or max_risk: read replaces a target with a hidden
|
||
// deny stub. Prescribing it would name a command that can only answer
|
||
// command_unavailable, and that the ranker has already stopped suggesting.
|
||
func TestConcealedTargetIsNotPrescribed(t *testing.T) {
|
||
svc := sheetsGroupWithHints()
|
||
for _, c := range svc.Commands() {
|
||
if c.Name() == "+sheet-create" {
|
||
c.Hidden = true
|
||
}
|
||
}
|
||
|
||
if err := svc.Args(svc, []string{"+sheet-add"}); err != nil {
|
||
t.Errorf("a concealed target must fall through to the ranker, got %v", err)
|
||
}
|
||
// Sibling prescriptions whose target is still reachable keep working.
|
||
if err := svc.Args(svc, []string{"+col-resize"}); err == nil {
|
||
t.Error("+col-resize target is still visible; its prescription should fire")
|
||
}
|
||
}
|
||
|
||
// A target that vanished outright must not produce a hint pointing at nothing.
|
||
func TestMissingTargetIsNotPrescribed(t *testing.T) {
|
||
svc := &cobra.Command{Use: "sheets"}
|
||
(&cobra.Command{Use: "lark-cli"}).AddCommand(svc)
|
||
InstallUnknownSubcommandHints(svc) // no targets mounted at all
|
||
|
||
for typed := range unknownSubcommandHints {
|
||
if err := svc.Args(svc, []string{typed}); err != nil {
|
||
t.Errorf("Args(%q) = %v, want fall-through when the target is absent", typed, err)
|
||
}
|
||
}
|
||
}
|