diff --git a/shortcuts/sheets/sheet_ai_api.go b/shortcuts/sheets/sheet_ai_api.go index 0f4ee4530..da0eafa28 100644 --- a/shortcuts/sheets/sheet_ai_api.go +++ b/shortcuts/sheets/sheet_ai_api.go @@ -5,9 +5,12 @@ package sheets import ( "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" "os" + "strconv" "strings" "github.com/larksuite/cli/internal/output" @@ -20,18 +23,65 @@ import ( // transaction id for sheet tool calls. const sheetTxnIDEnv = "LARK_CLI_SHEET_TRANSACTION_ID" -// sheetTransactionID returns the session-stable transaction id from the -// environment, or "" when unset. +// sheetTransactionID returns the session-stable transaction id threaded into a +// write tool call's extra.transaction_id. // // Sheet write tools persist their reverse ("undo") changeset keyed by the // request's transaction id; the server mints a fresh uuid per request when the -// caller supplies none, which isolates every CLI invocation into its own -// single-call undo stack. Threading one stable id across a group of edits (and -// a later +undo) is what lets +undo find and reverse those edits. An agent -// driving lark-cli sets this once per session; empty preserves today's -// per-request behavior. +// caller supplies none, which would isolate every CLI invocation into its own +// single-call undo stack. Sharing one stable id across a group of edits (and a +// later +undo) is what lets +undo find and reverse those edits. +// +// Resolution order: +// 1. $LARK_CLI_SHEET_TRANSACTION_ID — explicit caller override (highest). +// 2. else a value derived from this shell session (see +// deriveSessionTransactionID) so a group of edits and a later +undo group +// by default, with no env var to set. +// 3. else "" — the server mints a per-request id as before. func sheetTransactionID() string { - return strings.TrimSpace(os.Getenv(sheetTxnIDEnv)) + if v := strings.TrimSpace(os.Getenv(sheetTxnIDEnv)); v != "" { + return v + } + return deriveSessionTransactionID() +} + +// deriveSessionTransactionID builds a transaction id that is stable across the +// lark-cli invocations of one shell session and distinct across sessions, so a +// group of edits and a later +undo share an undo stack without the caller +// exporting LARK_CLI_SHEET_TRANSACTION_ID. +// +// Each lark-cli run is a fresh process and cannot mutate its parent's +// environment, so a *generated* id can't survive to the next command. Instead +// every run independently *recomputes* the same id from its own OS session — +// nothing is persisted between invocations. +// +// Returns "" when no trustworthy session signal exists (e.g. the process was +// reparented to init); the server then mints a per-request id and a +// missing-grouping +undo surfaces undone:0 rather than silently grouping +// unrelated callers. The grouping signal is a per-shell-session token +// (sessionSignal, platform-specific) salted with the uid and boot/host so a +// session id recycled after a reboot, or reused by a different user, can't +// collide with a stale undo stack. +func deriveSessionTransactionID() string { + sig, ok := sessionSignal() + if !ok { + return "" + } + seed := strings.Join([]string{sig, strconv.Itoa(os.Getuid()), sessionSalt()}, "|") + sum := sha256.Sum256([]byte(seed)) + return "larkcli-" + hex.EncodeToString(sum[:16]) +} + +// sessionSalt pins the derived id to this boot (Linux boot_id) or, failing +// that, this host, so a session id recycled after a reboot can't address a +// pre-reboot undo stack. Best-effort: an empty salt only weakens collision +// resistance across reboots, never correctness within a session. +func sessionSalt() string { + if b, err := os.ReadFile("/proc/sys/kernel/random/boot_id"); err == nil { + return strings.TrimSpace(string(b)) + } + h, _ := os.Hostname() + return h } // ToolKind selects the One-OpenAPI endpoint and its rate-limit bucket. diff --git a/shortcuts/sheets/sheet_ai_api_test.go b/shortcuts/sheets/sheet_ai_api_test.go index 383d76dfb..76e13d6e2 100644 --- a/shortcuts/sheets/sheet_ai_api_test.go +++ b/shortcuts/sheets/sheet_ai_api_test.go @@ -3,7 +3,10 @@ package sheets -import "testing" +import ( + "strings" + "testing" +) // cellsSetArgs is a minimal valid +cells-set invocation used to inspect the // tool-call request body. @@ -32,13 +35,32 @@ func TestBuildToolBody_ThreadsTransactionID(t *testing.T) { } } -// TestBuildToolBody_OmitsTransactionIDWhenUnset verifies the body carries no -// extra when the env var is empty, preserving the per-request default. -func TestBuildToolBody_OmitsTransactionIDWhenUnset(t *testing.T) { +// TestBuildToolBody_DerivesTransactionIDWhenUnset verifies that with the env +// var unset a write tool carries a session-derived transaction id (so a group +// of edits and a later +undo group by default), that the derived id is stable +// across invocations in the same session, and that it differs from any literal +// override. In an environment with no trustworthy session signal the derived +// id is "" and the body carries no extra, preserving the per-request default. +func TestBuildToolBody_DerivesTransactionIDWhenUnset(t *testing.T) { t.Setenv(sheetTxnIDEnv, "") + want := sheetTransactionID() body := parseDryRunBody(t, CellsSet, cellsSetArgs()) - if _, ok := body["extra"]; ok { - t.Errorf("extra should be absent when %s is unset: %#v", sheetTxnIDEnv, body) + extra, hasExtra := body["extra"].(map[string]interface{}) + + if want == "" { + if hasExtra { + t.Errorf("no session signal: extra should be absent: %#v", body) + } + return + } + if !strings.HasPrefix(want, "larkcli-") { + t.Errorf("derived transaction_id = %q, want larkcli- prefix", want) + } + if !hasExtra || extra["transaction_id"] != want { + t.Errorf("write tool should carry derived transaction_id %q: %#v", want, body) + } + if got := sheetTransactionID(); got != want { + t.Errorf("derived transaction_id not stable: %q vs %q", got, want) } } diff --git a/shortcuts/sheets/sheet_session_unix.go b/shortcuts/sheets/sheet_session_unix.go new file mode 100644 index 000000000..d4e3472e6 --- /dev/null +++ b/shortcuts/sheets/sheet_session_unix.go @@ -0,0 +1,32 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build !windows + +package sheets + +import ( + "os" + "strconv" + "syscall" +) + +// sessionSignal returns a token that is stable across every process of one +// shell/login session and distinct across sessions, plus ok=false when no such +// signal is trustworthy. +// +// The POSIX session id (getsid) is preferred: every process in the same +// terminal/login session shares it, and unlike the parent pid it survives +// subshell wrapping (e.g. `sh -c "lark-cli ..."` spawned afresh per command), +// which is the common way an agent drives the CLI. It falls back to the parent +// pid, then gives up when the process was reparented to init (sid/ppid <= 1) — +// init is shared by unrelated processes and would over-group distinct callers. +func sessionSignal() (string, bool) { + if sid, err := syscall.Getsid(0); err == nil && sid > 1 { + return "sid:" + strconv.Itoa(sid), true + } + if ppid := os.Getppid(); ppid > 1 { + return "ppid:" + strconv.Itoa(ppid), true + } + return "", false +} diff --git a/shortcuts/sheets/sheet_session_windows.go b/shortcuts/sheets/sheet_session_windows.go new file mode 100644 index 000000000..80acb9858 --- /dev/null +++ b/shortcuts/sheets/sheet_session_windows.go @@ -0,0 +1,21 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build windows + +package sheets + +import ( + "os" + "strconv" +) + +// sessionSignal returns a per-session grouping token. Windows has no POSIX +// session id, so the parent process id is the best portable signal; ok=false +// when the process has no real parent (ppid <= 1). +func sessionSignal() (string, bool) { + if ppid := os.Getppid(); ppid > 1 { + return "ppid:" + strconv.Itoa(ppid), true + } + return "", false +}