From 886cca6032462f868e35dfcdc7b9d4e45b0d57ed Mon Sep 17 00:00:00 2001 From: zhengzhijie Date: Wed, 10 Jun 2026 22:37:32 +0800 Subject: [PATCH] feat(sheets): anchor +undo by revision (--rev) instead of --op Undo is now addressed by the document revision a prior write returned (write responses carry data.revision): +undo --rev reverses exactly the edit that produced that revision, rejected if the document has moved past it; omitting --rev targets the latest edit. --steps N still reverses the last N edits in one atomic call. --op is removed: operation ordinals were scoped to a per-session transaction id that fresh-shell agent harnesses cannot thread across commands, so the handle was unreachable in practice. The revision anchor needs no session state at all. --- shortcuts/sheets/data/flag-defs.json | 8 +-- shortcuts/sheets/flag_defs_gen.go | 22 ++++---- shortcuts/sheets/lark_sheet_undo.go | 68 ++++++++++++------------ shortcuts/sheets/lark_sheet_undo_test.go | 35 +++++++----- 4 files changed, 72 insertions(+), 61 deletions(-) diff --git a/shortcuts/sheets/data/flag-defs.json b/shortcuts/sheets/data/flag-defs.json index 9993918b9..88b039f8a 100644 --- a/shortcuts/sheets/data/flag-defs.json +++ b/shortcuts/sheets/data/flag-defs.json @@ -21,15 +21,15 @@ "kind": "own", "type": "int", "required": "optional", - "desc": "Undo the most recent N edits made through this CLI link (default 1); XOR with `--op`", + "desc": "Undo the most recent N edits made through this CLI link (default 1); one step = one prior write call", "default": "1" }, { - "name": "op", + "name": "rev", "kind": "own", - "type": "string", + "type": "int", "required": "optional", - "desc": "Undo a specific operation by its operation_id (from a prior write's undo handle); XOR with `--steps`" + "desc": "Undo anchor: the document revision returned by a prior write's response (`data.revision`). Omit to undo the latest edit. Doubles as an optimistic-concurrency check — rejected if the document has moved past this revision" }, { "name": "dry-run", diff --git a/shortcuts/sheets/flag_defs_gen.go b/shortcuts/sheets/flag_defs_gen.go index 6f850d7dc..d4e2bd2cf 100644 --- a/shortcuts/sheets/flag_defs_gen.go +++ b/shortcuts/sheets/flag_defs_gen.go @@ -734,6 +734,15 @@ var flagDefs = map[string]commandDef{ {Name: "dry-run", Kind: "system", Type: "bool", Required: "optional"}, }, }, + "+recover": { + Risk: "write", + Flags: []flagDef{ + {Name: "url", Kind: "public", Type: "string", Required: "xor", Desc: "Spreadsheet URL (XOR with `--spreadsheet-token`)"}, + {Name: "spreadsheet-token", Kind: "public", Type: "string", Required: "xor", Desc: "Spreadsheet token (XOR with `--url`)"}, + {Name: "to-revision", Kind: "own", Type: "int", Required: "required", Desc: "Restore the whole spreadsheet to this revision (a revision number returned by a prior write)"}, + {Name: "dry-run", Kind: "system", Type: "bool", Required: "optional"}, + }, + }, "+rows-resize": { Risk: "write", Flags: []flagDef{ @@ -941,17 +950,8 @@ var flagDefs = map[string]commandDef{ Flags: []flagDef{ {Name: "url", Kind: "public", Type: "string", Required: "xor", Desc: "Spreadsheet URL (XOR with `--spreadsheet-token`)"}, {Name: "spreadsheet-token", Kind: "public", Type: "string", Required: "xor", Desc: "Spreadsheet token (XOR with `--url`)"}, - {Name: "steps", Kind: "own", Type: "int", Required: "optional", Desc: "Undo the most recent N edits made through this CLI link (default 1); XOR with `--op`", Default: "1"}, - {Name: "op", Kind: "own", Type: "string", Required: "optional", Desc: "Undo a specific operation by its operation_id (from a prior write's undo handle); XOR with `--steps`"}, - {Name: "dry-run", Kind: "system", Type: "bool", Required: "optional"}, - }, - }, - "+recover": { - Risk: "write", - Flags: []flagDef{ - {Name: "url", Kind: "public", Type: "string", Required: "xor", Desc: "Spreadsheet URL (XOR with `--spreadsheet-token`)"}, - {Name: "spreadsheet-token", Kind: "public", Type: "string", Required: "xor", Desc: "Spreadsheet token (XOR with `--url`)"}, - {Name: "to-revision", Kind: "own", Type: "int", Required: "required", Desc: "Restore the whole spreadsheet to this revision (a revision number returned by a prior write)"}, + {Name: "steps", Kind: "own", Type: "int", Required: "optional", Desc: "Undo the most recent N edits made through this CLI link (default 1); one step = one prior write call", Default: "1"}, + {Name: "rev", Kind: "own", Type: "int", Required: "optional", Desc: "Undo anchor: the document revision returned by a prior write's response (`data.revision`). Omit to undo the latest edit. Doubles as an optimistic-concurrency check — rejected if the document has moved past this revision"}, {Name: "dry-run", Kind: "system", Type: "bool", Required: "optional"}, }, }, diff --git a/shortcuts/sheets/lark_sheet_undo.go b/shortcuts/sheets/lark_sheet_undo.go index 5deb35709..7761111e8 100644 --- a/shortcuts/sheets/lark_sheet_undo.go +++ b/shortcuts/sheets/lark_sheet_undo.go @@ -5,9 +5,7 @@ package sheets import ( "context" - "strings" - "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -16,27 +14,32 @@ import ( // Wraps: // - undo_last (write) — powers +undo // -// Reverses the most recent edits this CLI link made to a spreadsheet. The -// backend already records an inverse changeset for every write (see the -// undo design doc, "方案 A"); +undo asks the backend executor to read that -// changeset back and re-apply it in reverse order on the node Workbook, then -// push the result upstream as a collaboration change. The CLI only triggers -// the tool — the read-back endpoint is space-internal and not reachable -// through the /open-apis gateway, so all the heavy lifting stays server-side. +// Reverses the most recent edits this CLI link made to a spreadsheet, addressed +// by document revision. Every write response carries `data.revision`; that +// number is the undo anchor. The backend records an inverse changeset for every +// write and indexes it by the revision it produced (see the undo design doc, +// "方案 A · rev 寻址"); +undo asks the backend executor to locate that inverse +// data through the revision pointer, verify nobody else changed the document +// since (tip / continuity / object-version / identity checks), re-apply it in +// reverse order on the node Workbook, and push the result upstream as a +// collaboration change. The CLI only triggers the tool — the read-back endpoint +// is space-internal and not reachable through the /open-apis gateway, so all +// the heavy lifting stays server-side. // // +undo carries no sheet selector: undo is scoped to the spreadsheet + this -// link's edit history, not a single sub-sheet. The two selection modes are -// XOR: -// - --steps N : undo the last N edits (default 1) -// - --op : undo one specific operation_id surfaced by a prior write's -// undo handle +// link's edit history, not a single sub-sheet. Selection: +// - (no flags) : undo the latest edit, if it was made by this caller +// - --rev N : undo anchored at revision N (from a prior write response); +// rejected when the document has moved past N +// - --steps N : undo the last N edits in one atomic call (default 1) // Undo wraps undo_last: reverse the most recent edits made through this CLI -// link, either the last N steps (--steps) or one specific operation (--op). +// link, anchored by the revision a prior write returned (--rev), defaulting +// to the latest edit. var Undo = common.Shortcut{ Service: "sheets", Command: "+undo", - Description: "Undo the most recent edits this CLI link made to a spreadsheet (last N steps, or a specific operation).", + Description: "Undo the most recent edits this CLI link made to a spreadsheet (anchored by a write's returned revision).", Risk: "write", Scopes: []string{"sheets:spreadsheet:write_only"}, AuthTypes: []string{"user", "bot"}, @@ -72,31 +75,28 @@ var Undo = common.Shortcut{ return nil }, Tips: []string{ - "Undo is scoped to edits made through this CLI link — it never touches changes other collaborators (or the web UI) made to the same range.", - "Use --dry-run to preview which steps would be undone before running it.", + "Every write response carries data.revision — remember it; +undo --rev undoes exactly that edit, and +recover --to-revision is the full-rollback fallback.", + "Without --rev, +undo targets the document's latest edit — it succeeds only when that edit was made through this CLI link by you.", + "Repeated +undo steps back one edit at a time; --steps N undoes the last N edits in one atomic call. Already-undone edits are skipped automatically.", + "If anyone else edited the document after (or between) the edits you want to undo, +undo refuses entirely and suggests +recover — it never partially undoes or overwrites others' changes.", + "A success response with undone:0 plus warning_message means nothing was actually undone — the targeted revision wasn't produced by this caller, or was already undone.", + "Use --dry-run to preview the request before running it.", }, } -// undoInput builds the undo_last tool body and enforces the --steps / --op -// XOR. --steps carries a default of 1, so the mutual-exclusion check keys off -// Changed("steps") (whether the user actually passed it) rather than its -// value. Network-free; shared by Validate, DryRun, and Execute. +// undoInput builds the undo_last tool body. --rev anchors the undo at the +// revision a prior write returned (omitted = latest); --steps selects how many +// edits to reverse in one atomic call. Network-free; shared by Validate, +// DryRun, and Execute. func undoInput(runtime flagView, token string) (map[string]interface{}, error) { - op := strings.TrimSpace(runtime.Str("op")) - stepsSet := runtime.Changed("steps") - - if op != "" && stepsSet { - return nil, common.FlagErrorf("--steps and --op are mutually exclusive") - } - input := map[string]interface{}{"excel_id": token} - if op != "" { - if err := validate.RejectControlChars(op, "op"); err != nil { - return nil, common.FlagErrorf("%v", err) + if runtime.Changed("rev") { + rev := runtime.Int("rev") + if rev < 1 { + return nil, common.FlagErrorf("--rev must be a positive revision number (from a prior write's data.revision)") } - input["operation_id"] = op - return input, nil + input["rev"] = rev } steps := runtime.Int("steps") diff --git a/shortcuts/sheets/lark_sheet_undo_test.go b/shortcuts/sheets/lark_sheet_undo_test.go index cdb8fc437..69fe4ce4a 100644 --- a/shortcuts/sheets/lark_sheet_undo_test.go +++ b/shortcuts/sheets/lark_sheet_undo_test.go @@ -9,8 +9,9 @@ import ( ) // TestUndo_DryRun asserts the undo_last body for the three selection shapes: -// default (steps=1), explicit --steps, and --op. Numbers round-trip through -// the wire JSON as float64, matching the other dry-run body tests. +// default (latest, steps=1), explicit --steps, and a --rev anchor. Numbers +// round-trip through the wire JSON as float64, matching the other dry-run +// body tests. func TestUndo_DryRun(t *testing.T) { t.Parallel() @@ -20,7 +21,7 @@ func TestUndo_DryRun(t *testing.T) { wantInput map[string]interface{} }{ { - name: "default undoes last step", + name: "default undoes the latest edit", args: []string{"--url", testURL}, wantInput: map[string]interface{}{ "excel_id": testToken, @@ -36,11 +37,21 @@ func TestUndo_DryRun(t *testing.T) { }, }, { - name: "--op targets a specific operation", - args: []string{"--spreadsheet-token", testToken, "--op", "op_5"}, + name: "--rev anchors at a write's returned revision", + args: []string{"--spreadsheet-token", testToken, "--rev", "123"}, wantInput: map[string]interface{}{ - "excel_id": testToken, - "operation_id": "op_5", + "excel_id": testToken, + "rev": float64(123), + "steps": float64(1), + }, + }, + { + name: "--rev composes with --steps", + args: []string{"--url", testURL, "--rev", "123", "--steps", "2"}, + wantInput: map[string]interface{}{ + "excel_id": testToken, + "rev": float64(123), + "steps": float64(2), }, }, } @@ -54,8 +65,8 @@ func TestUndo_DryRun(t *testing.T) { } } -// TestUndo_Validation covers the XOR token check, the --steps/--op -// mutual exclusion, and the --steps lower bound. +// TestUndo_Validation covers the XOR token check, the --rev lower bound, and +// the --steps lower bound. func TestUndo_Validation(t *testing.T) { t.Parallel() @@ -70,9 +81,9 @@ func TestUndo_Validation(t *testing.T) { wantMsg: "at least one of --url or --spreadsheet-token", }, { - name: "--steps and --op are mutually exclusive", - args: []string{"--url", testURL, "--steps", "2", "--op", "op_5"}, - wantMsg: "mutually exclusive", + name: "--rev must be positive", + args: []string{"--url", testURL, "--rev", "0"}, + wantMsg: "--rev must be a positive revision number", }, { name: "--steps must be >= 1",