4 Commits

Author SHA1 Message Date
xiongyuanwen-byted c6c040c2c5 refactor(sheets)!: remove legacy sheets command surface (#2572)
* 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).
2026-09-01 19:47:18 +08:00
xiongyuanwen-byted 2f8d816512 fix(sheets): make image-upload previews match what Execute sends (#2537)
* fix(sheets): repair the build after the local-office detection move

main does not compile: shortcuts/sheets/lark_sheet_workbook.go references
isOfficeSpreadsheet and officePrefixes, which no longer exist in the package.

Neither PR was wrong on its own. #2531 (merged 10:39) moved local-office token
detection into common.IsLocalOfficeToken and deleted the sheets-local copies;
#2533 (merged 12:48) added errLocalOfficeExportUnsupported, which calls them.
#2533's branch predated the move, so its CI was green against a base that still
had the symbols, and merging it left main broken.

Repoints both references at the moved API. Behaviour is unchanged:
common.IsLocalOfficeToken is the same predicate #2531 moved, and the prefix pair
is the exported form of the same two constants. #2533's own coverage —
TestWorkbookExport_LocalOfficeTokenRejected across the local_office_ prefix, the
fake_office_ prefix, and an interleaved OFL0X token, plus the wiki-node and
dry-run cases — passes unchanged, which is what pins the equivalence.

* fix(sheets): derive dry-run image parent_type from the ref kind, not the token

A `/wiki/` URL reaches a DryRun hook as the wiki node_token: resolving it to the
backing spreadsheet needs the get_node call a preview must not make. Both
image-upload previews fed that node_token straight to sheetMediaParentType, so
the parent_type they showed was derived from a token that is not the one Execute
uploads against. A node_token shaped like an imported office token previewed
office_sheet_file for a spreadsheet that will upload as sheet_image.

sheetsDryRunParentType decides from the ref's kind instead. A wiki ref is native
by construction, not by default: resolveWikiNodeToSpreadsheetToken rejects any
node whose obj_type is not "sheet", and a spreadsheet backed by an imported
office file sits in drive as a "file" node, so it never survives that gate to
reach an upload. Execute is unaffected either way — it derives from the resolved
token. This mirrors slidesDryRunParentType, which the slides domain already
applies for the same reason.

The hooks now hold the parsed ref rather than re-deriving the token from it, so
the kind is visible where the preview is built.

Also records, at uploadSheetMediaFile, that office_sheet_file survives the
multipart path. Slides caps image uploads at 20 MB because upload_prepare
rejects its parent types outright, which raised the question for sheets, whose
deprecated +media-upload has no such cap. Verified against the live API:
upload_prepare accepts both sheet_image and office_sheet_file, and a 20.6 MB
file uploaded with office_sheet_file completes prepare -> 6 x upload_part ->
upload_finish and returns a file_token a float image then accepts.

Tests: sheetsDryRunParentType over both ref kinds, including wiki refs carrying
office-shaped and office-prefixed node tokens; dry-run coverage through the
public flags for +cells-set-image and +float-image-create, each checked against
the identical token as a /wiki/ URL and as a raw spreadsheet token so the two
rows differ only in kind; the same pair added to the e2e dry-run lane. All four
fail against the previous behaviour.

* fix(sheets): send oversized images through the chunked upload, not upload_all

uploadSheetImage always used the single-part endpoint, so an image past the
20 MB ceiling failed with a bare 1061002 "upload media failed: params error"
naming neither the size nor the limit. The capability was already in the domain:
the deprecated sheets +media-upload has dispatched by size since it was written
(backward.uploadSheetMediaFile), which left the same image succeeding through
the old shortcut and failing through +cells-set-image and +float-image-create,
the ones meant to replace it.

uploadSheetImage now picks the endpoint by size the way doc and the deprecated
shortcut already do. The parent_type is unchanged and still comes from
sheetMediaParentType, so the office/native split rides along either branch.

The preview follows the same branch. appendSheetImageUploadDryRun renders one
upload_all under the ceiling and the upload_prepare / upload_part /
upload_finish trio above it, and both image-write hooks now build their upload
step through it rather than each spelling out an upload_all. A preview that
promised a single-part upload for a file the CLI will send in chunks is a
preview of a different request.

Verified against the live API with a 20.6 MB PNG: +cells-set-image and
+float-image-create both complete, the cell reads back holding the uploaded
image_token at the file's real 3000x2400 dimensions, and the dry-run shows the
three chunked steps Execute hits. The same file failed with 1061002 before.

Tests: the chunked branch at exactly one byte past the ceiling, asserted through
upload_prepare's parent_type with upload_all deliberately left unstubbed so a
regression fails loudly; the preview's step list on both sides of the boundary,
as a unit test and in the e2e dry-run lane. All fail against the previous
behaviour.
2026-08-28 14:51:14 +08:00
caojie0621 7fb71c6947 feat(sheets): add sheet management shortcuts (#722)
* feat(sheets): add sheet management shortcuts

- add +create-sheet, +copy-sheet, +delete-sheet, and +update-sheet
- cover request-shape dry-run and sheet workflow tests
- document new sheet management shortcuts in lark-sheets skill

* docs(sheets): consolidate lark-sheets reference docs
2026-05-01 15:49:24 +08:00
Yuxuan Zhao 5280517d4b Feat/cli e2e tests with UAT (#528)
* test: expand and stabilize cli e2e workflows

* ci: run deadcode with test entrypoints
2026-04-17 16:57:17 +08:00