Commit Graph

6 Commits

Author SHA1 Message Date
xiongyuanwen-byted 835b52cc88 feat(sheets): cut the top command-error clusters from the 08-18..24 eval batch (#2559)
* feat(sheets): cut the top command-error clusters from the 08-18..24 eval batch

A trace analysis over 14,818 `lark-cli sheets` calls attributed 2,036 command
errors to 57 (subcommand, flag) groups, with the top 6 covering 76%. Four of
them are ours to fix; each is addressed at the layer that produced it.

--styles vocabulary (291 cases, the only group whose retry also failed):

  - Prescribe the border family as a whole. foldBorderFamilyAliases already
    absorbs border / borders / border_<side> / border_<attr>; what still
    reached the error path was the Lark OpenAPI's border_type (FULL_BORDER,
    OUTER_BORDER) and CSS's border_width — real vocabularies with no
    equivalent here, and border_type was the single top field in the group.
    Neither maps unambiguously onto a per-side style/weight/color triple, so
    they get one shared answer, not a silent alias.
  - Prescribe the OpenAPI's nested {range, style:{...}} envelope, plus
    bg_color / fill_color / text_color.
  - Match prescriptions on the key's letters alone, so border_type,
    borderType and border-type are one mistake, not three.
  - Collapse repeated issues in the --styles / --writes folds. One wrong
    field name in a payload styling N cells produced N identical issues, each
    re-listing the full supported vocabulary: the fold meant to save round
    trips was burying its own answer. The defect is now stated once and the
    other locations are named.

--sheets payload (419 cases):

  - Accept dtypes / formats as a positional array. The same pandas habit that
    produces `columns` and `data` produces df.dtypes.tolist(), and the
    payloads were otherwise correct. Only a 1:1 match with `columns` is
    accepted; a length mismatch is rejected rather than guessed.

+csv-put --file (35 cases):

  - Read the value as a path. --file is aliased onto --csv because agents
    reach for it, but the names promise different things — rewriting only the
    name left the path to be written into the sheet as literal text, which the
    file-path guard then rejected, with an error naming a flag the caller
    never typed. The read goes through the same cmdutil.ReadInputFile as
    @file, so the relative-path policy is unchanged and stdin stays the
    out-of-tree route. A value naming nothing readable still falls through to
    the guard, so --file holding literal CSV keeps working.

--help (88 cases of "required flag(s) ... not set"):

  - Mark required flags in the sheets help. MarkFlagRequired only sets a
    completion annotation cobra never renders, so a required flag read exactly
    like an optional one. Sourced from flag-defs, since +chart-create and
    +csv-put deliberately clear that annotation after mounting; a flag cobra
    has put in a one-required group is left unmarked, because neither member
    of such a pair is individually required.

The remaining big group (absolute paths passed to --file / @file, 636 cases)
is deliberately untouched: the cwd-relative policy is a protocol decision, and
that thread is being followed up separately.

* fix(sheets): correct --file alias provenance and mark its value resolved

Two defects in the +csv-put --file rule from the previous commit, both found
in review:

- The alias record was written from the flag-name normalizer, which pflag also
  runs for Lookup and Set — including once with the canonical name right after
  the rewrite, and again on every later lookup. `--file a.csv --csv ./b.csv`
  therefore still counted as "supplied by --file", and an explicit --csv path
  was silently read as a file instead of meeting its guard. The spelling is now
  staged by the normalizer and committed by the flag's Value, which runs once
  per real occurrence, so the last occurrence wins in either order.

- The rewritten value was not marked as read from a source, so a file whose
  contents are themselves path-shaped ("report.csv") failed csvPutInput's shape
  check — a valid CSV rejected as a caller who forgot the @. Marking it makes
  --file behave exactly like --csv @<path> all the way down, which also lets
  the guard skip it on its own rather than through a special case in Validate.

RuntimeContext gains an exported MarkInputResolved for the second half: the bit
already existed for @file / stdin, and a domain that resolves a source itself
needs to set it. No other domain calls it, so nothing else changes.

Also assert the typed contract (Param, Cause) rather than message text alone in
the style-prescription corpus and the collapsed-issue fold, per the repo's
error-test guideline.

* fix(sheets): answer every unreadable --file path under --file, and stage alias provenance only while parsing

Both from self-review of the branch.

An unreadable path passed as --file fell through to the --csv guard, which
answered naming a flag the caller never typed — and for a file that exists but
cannot be opened, prescribed "pass the same path with an @ prefix", which routes
through this very reader and fails identically. Only one case may fall through
now: a value that names nothing AND is not path-shaped, i.e. literal CSV text,
which --file accepted before this rule existed. A path-shaped value naming
nothing, an unreadable file, and a directory each answer under --file.

The alias spelling was staged with no Parsed() guard (the first commit had one;
flagalias.Bind still does). chainFlagAliases looks its aliases up while
installing and pflag normalizes on Lookup, so composing PostMount twice — which
installAliasProvenance explicitly anticipates — replayed "file" through the
already-installed normalizer at mount time, and the next real --csv occurrence
committed it: an explicit --csv path would then be read from disk instead of
meeting its guard. Verified the new regression test fails without the guard.

* fix(sheets): reset alias staging on remount, and assert cause on every --file read error

Second review pass, both valid.

FlagSet.Parsed() stays true once parsing has started, so the guard added in the
last commit only covers a remount that happens BEFORE the first parse. A remount
afterwards — its own alias lookups running through the normalizer the first pass
installed — could still leave a spelling staged for the next occurrence to
commit, which would read an explicit --csv path from disk. Re-running the
install now resets staging, closing the window from the other side. Verified the
new parse/remount/reparse test fails without the reset.

The unreadable-file and directory branches preserve the read error as Cause;
their tests now assert it, matching the sibling that already did.
2026-09-01 15:05:10 +08:00
xiongyuanwen-byted be2a96f490 feat(sheets): harden error prescriptions, batch updates, and read workflows
Aggregate the sheets work from feat/lark-sheets-develop:

- Improve validation errors with schema hints, aggregated issues, enum guidance, and prescriptive flag/style-field messages.
- Harden +batch-update input contracts, key normalization, style vocabulary handling, and resource-budget checks.
- Add read offload and truncation handling for cells, csv, and table-get, with typed output-path errors and safer jq/output-path semantics.
- Correct freeze semantics by emitting full-state freeze/unfreeze operations and adding --rows/--cols for +dim-freeze.
- Improve +styles-put and shared --styles parsing for styles, merges, row/column sizing, freeze, and sheet-prefixed range validation.
- Fix dim-insert inherit-style mapping, table-get date/time handling, table-put style anchors, and CSV path-shaped input guards.
- Update lark-sheets skill docs, scripts, tests, and generated flag data.

Tested with:
- go test ./shortcuts/common ./shortcuts/sheets/...
- go test ./shortcuts/... ./internal/...
- python3 -m py_compile skills/lark-sheets/scripts/*.py
2026-08-07 11:21:12 +08:00
河伯 bc6590abef feat(doc): add --from-clipboard flag to docs +media-insert (#508)
* feat(doc): add --from-clipboard flag to docs +media-insert

Allow users to upload the current clipboard image directly to a Lark
document without saving to a local file first.

- New --from-clipboard bool flag (mutually exclusive with --file)
- shortcuts/doc/clipboard.go: readClipboardToTempFile() with per-OS impl
    macOS   — osascript (built-in, no extra deps)
    Windows — PowerShell + System.Windows.Forms (built-in)
    Linux   — tries xclip / wl-paste / xsel in order; clear install hint
              on failure
- No new Go dependencies, no Cgo
- Temp file is created before upload and removed via defer cleanup()
- --file changed from Required:true to optional; Validate enforces
  exactly-one of --file / --from-clipboard

* fix(doc): fix clipboard image read on macOS for screenshots and browser-copied images

- Add TIFF fallback (macOS screenshots default to TIFF, not PNG)
- Add HTML base64 fallback (images copied from Feishu/browser embed data URI)
- Use current directory for temp file so FileIO path validation passes

* fix(doc): scan HTML/RTF/text clipboard formats for base64 image data URIs

Extend attempt-3 fallback to iterate all text-based clipboard formats
(HTML, RTF, UTF-8, plain text) rather than only HTML.  Any format that
contains a "data:<mime>;base64,<data>" pattern is accepted, covering
images copied from Feishu, Chrome, Safari, and other apps that embed
base64 in non-HTML clipboard slots.  Also handle URL-safe base64.

* test(doc): add unit tests for clipboard helpers to meet 60% coverage threshold

Cover decodeHex, hexVal, decodeOsascriptData, reBase64DataURI, and
extractBase64ImageFromClipboard (via fake osascript on PATH).
Package coverage: 57% → 61.2%.

* fix(doc): address CodeRabbit review comments on clipboard feature

- Extend reBase64DataURI regex to cover URL-safe base64 chars (-_) so
  URL-safe payloads are matched before decoding is attempted
- Fix readClipboardLinux to continue to next tool when a found tool
  returns empty output instead of failing immediately
- Guard fake-osascript test with runtime.GOOS == "darwin" skip
- Use os.PathListSeparator instead of hardcoded ":" in test PATH setup

* fix(doc): replace os.* temp-file clipboard path with in-memory streaming

Fixes forbidigo lint violations in shortcuts/doc: os.CreateTemp, os.Remove,
os.Stat, os.WriteFile are banned in shortcuts/; replaced with vfs.* equivalents
for sips TIFF→PNG conversion, and eliminated temp files entirely elsewhere by
having platform clipboard readers return []byte directly.

- readClipboardDarwin: osascript outputs hex literals decoded in Go (no file I/O)
- readClipboardWindows: PowerShell outputs base64 to stdout, decoded in Go
- readClipboardLinux: tool stdout bytes returned directly
- convertTIFFToPNGViaSips: still needs temp files — uses vfs.CreateTemp/Remove
- DriveMediaUploadAllConfig/DriveMediaMultipartUploadConfig: add Content io.Reader
  field so in-memory clipboard bytes skip FileIO.Open() path
- Fix ineffassign in clipboard_test.go (scriptBody double-assignment)
- Update TestReadClipboardLinux_NoToolsReturnsError for new signature

* fix(doc): address CodeRabbit review comments on Linux clipboard path

- Update --from-clipboard flag description to list xclip, xsel and wl-paste
- Preserve last backend-specific error in readClipboardLinux so users see
  a meaningful message when a tool is found but fails
- Validate PNG magic bytes for xsel output (xsel cannot negotiate MIME types)
- Add URL-safe base64 regression test for reBase64DataURI

* fix(doc): strip whitespace from base64 payload before decoding clipboard data URI

HTML and RTF clipboard content often line-wraps base64 at 76 characters.
FindSubmatch returns the raw wrapped token so direct decode would fail.
Normalize whitespace with strings.Fields before passing to base64.Decode.

* fix(doc): drop TIFF fallback and internal/vfs import on macOS clipboard

depguard rule shortcuts-no-vfs forbids shortcuts/ from importing
internal/vfs directly. The only caller was the sips TIFF→PNG
conversion, which was already a fragile best-effort fallback that
required temp files.

Remove the TIFF fallback entirely; the remaining two attempts cover
the real-world cases:
  1. osascript → PNG hex literal — native screenshots and most apps
  2. scan text clipboard formats for base64 data URI — Feishu/browsers

* test(doc): cover readClipboardLinux xsel PNG validation and dispatcher path

Added tests:
- TestReadClipboardLinux_XselRejectsNonPNG: fake xsel that returns plain
  text is rejected by the PNG-magic check, preventing text from being
  uploaded as an "image".
- TestHasPNGMagic: table-driven coverage of the PNG signature check.
- TestReadClipboardImageBytes_UnsupportedPlatform: exercises the shared
  dispatcher post-processing and asserts the (nil, nil) invariant.

Raises clipboard.go diff coverage and brings the package from 61.6% to
63.8% overall.

* test: cover in-memory Content upload paths for clipboard feature

Adds unit tests for the new Content io.Reader branches introduced by
the clipboard feature:

- UploadDriveMediaAll with in-memory Content (drive_media_upload.go 87.5%)
- UploadDriveMediaMultipart with in-memory Content (84.6%)
- uploadDocMediaFile single-part and multipart with clipboard bytes
  (doc_media_upload.go 0% -> 88.9%)

Adds TestNewRuntimeContextForAPI helper that wires Factory, context,
and bot identity so package tests can invoke DoAPI without mounting
the full cobra command tree.

* test: cover clipboard Validate/DryRun branches and testing helper

Adds unit tests for the clipboard-related Validate/DryRun paths that
Codecov patch-coverage was flagging as uncovered:

- Validate error when neither --file nor --from-clipboard is supplied
- Validate error when both are supplied (mutual exclusion)
- DryRun output contains <clipboard image> placeholder
- Self-test for TestNewRuntimeContextForAPI so shortcuts/common
  sees coverage for the new helper (not just shortcuts/doc)

* test: cover Execute clipboard branch via injectable readClipboardImage

Makes readClipboardImageBytes swappable in tests by routing the call
through a package-level variable readClipboardImage. Tests inject a
synthetic PNG payload so the full Execute clipboard flow
(resolve → create block → upload in-memory bytes → bind) runs under
unit test without a real pasteboard.

Covers:
- TestDocMediaInsertExecuteFromClipboard: end-to-end happy path
- TestDocMediaInsertExecuteClipboardReadError: early-return on
  readClipboardImage() failure

* ci: re-trigger pull_request workflow for PR #508

Previous push to 9dedb7a did not trigger the main CI workflow via
the pull_request event (only PR Labels ran). The workflow_dispatch
run I triggered manually lacks PR-scoped secrets so security and
e2e-live failed. An empty commit replays the pull_request event so
the full matrix (deadcode, license-header, security, e2e-live) runs
with proper context.

* test(doc): guard info.Size() behind err check to prevent nil-deref

CodeRabbit flagged that 't.Fatalf("... size=%d err=%v", info.Size(), err)'
evaluates info.Size() even when os.Stat returned (nil, err), which nil-derefs.
Split the check into two stages so the error-path t.Fatalf does not touch
info.

* fix(doc): address fangshuyu-768 review on clipboard PR

Seven code changes driven by review feedback:

1. clipboard.go: stop using CombinedOutput() on osascript / powershell.
   Stdout is decoded, stderr is captured separately via cmd.Stderr and
   surfaced in the terminal error message, so locale warnings or
   AppleEvent permission prompts no longer pollute the hex/base64
   payload or mask the real failure.

2. clipboard.go: validate decoded base64 data URI bytes against known
   image magic headers (PNG/JPEG/GIF/WebP/BMP). A text clipboard that
   happens to contain a literal 'data:image/...;base64,...' fragment
   (documentation, tutorials, pasted HTML source) no longer silently
   becomes an image upload.

3. clipboard.go: simplify the Linux 'no tool found' install hint to a
   distro-agnostic phrasing instead of apt/yum only.

4. clipboard_test.go: delete the stale TestReadClipboardToTempFile_*
   tests. They referenced a readClipboardToTempFile function that no
   longer exists and only exercised os.CreateTemp/os.Remove. Replace
   with TestReadClipboardImageBytes_EmptyResultReturnsError which
   actually locks in the 'empty clipboard' → error contract of the
   current API (Linux-only since mac/Windows need a real pasteboard).

5. doc_media_upload.go: introduce UploadDocMediaFileConfig struct so
   uploadDocMediaFile takes a named config instead of 8 positional
   params. Drops the //nolint:lll the old call site had to carry.

6. doc_media_insert.go: convert the clipboard upload call to the new
   config struct and only set Config.Content when the clipboard branch
   actually produced bytes — this also fixes a latent typed-nil bug
   where a nil *bytes.Reader was being passed through an io.Reader
   parameter, which tripped the 'if cfg.Content != nil' check in
   UploadDriveMediaAll and crashed --file uploads.

7. shortcuts/common/testing.go: TestNewRuntimeContextForAPI now takes
   the identity as an explicit core.Identity parameter instead of
   hardcoding core.AsBot, and its self-test covers both AsBot and
   AsUser. Existing call sites pass core.AsBot explicitly.

Also annotates DryRun output with an 'upload_size_note' when
--from-clipboard is set, since DryRun never reads the pasteboard and
can't predict whether the payload will take the single-part or
multipart path.

* fix(doc): capture line-wrapped base64 in clipboard data URI regex (#586)

HTML and RTF clipboard content commonly folds base64 payloads at
76 chars (standard MIME folding). The previous character class
[A-Za-z0-9+/\-_]+=* stopped at the first \n, so the downstream
strings.Fields normalisation was a no-op (nothing to strip) and
extractBase64ImageFromClipboard silently uploaded a truncated
payload whose 8-byte prefix happened to pass hasKnownImageMagic.

Extend the class to include \s so the Fields strip actually has
whitespace to remove before base64 decoding. Terminators (", <,
), ;) remain outside the class so the match still ends at the
URI boundary.

Add TestReBase64DataURI_LineWrapped covering \n, \r\n, and \t
folds, full round-trip byte-equality, and the terminator-boundary
invariant so any future regression trips a failing test.

* docs(skill): add clipboard-empty fallback guidance for +media-insert

When --from-clipboard returns 'no image data' (empty clipboard, non-image
content, or Linux without xclip/wl-paste/xsel), the agent must NOT silently
swallow the error. It should tell the user the clipboard had no image, ask
for a local file path, then retry the same insert command with --file.

Lists three anti-patterns (silent success, guessing a file path, pre-emptive
save-then-file workaround) that agents have been tempted into.

* docs(skill): user-stated source trumps clipboard/file heuristic

The heuristic table (prefer --from-clipboard when image is on the
clipboard) is a fallback for when the user is vague. If the user
explicitly says 'use the screenshot I just copied' → clipboard; if
they give a path → --file. Agent must not silently swap sources even
when the other looks 'better'.

---------

Co-authored-by: fangshuyu-768 <shuyufang768@outlook.com>
2026-04-22 22:05:33 +08:00
liangshuo-1 9f81e7e567 feat: add RuntimeContext.BotInfo() for lazy bot identity retrieval (#409)
Add BotInfo() method on RuntimeContext that lazily fetches the current
app's bot open_id and display name from /bot/v3/info on first call,
cached via sync.OnceValues for the lifetime of the process.

- BotInfo struct (OpenID, AppName) in Identity section of runner.go
- fetchBotInfo() uses DoAPIAsBot for consistent header injection
- CanBot() on CliConfig gates the call when bot identity is unavailable
- Nil guard prevents panic in test contexts
- Full test coverage via httpmock.Registry + mounted shortcuts

Change-Id: I40ac710fb52d13939853f71827a5cbdbddd4f80f
2026-04-11 11:53:02 +08:00
shifengjuan-dev a641fdd5e6 feat: support user identity for im +chat-create (#242)
- Add --as user support to +chat-create
  - Add UserScopes (im:chat:create_by_user) / BotScopes (im:chat:create)
  - Update skill docs and reference files to reflect user/bot support
  - Default identity remains bot (first element of AuthTypes)

Change-Id: I6be0a160567a0d87a92f176ae12297a11d06dcb1
2026-04-03 16:35:28 +08:00
梁硕 83dfb068ad feat: open-source lark-cli — the official CLI for Lark/Feishu
Change-Id: I113d9cdb5403cec347efe4595415e34a18b7decf
2026-03-28 10:36:25 +08:00