Skip to content

docs(task): clarify inline image limitation#1826

Closed
xu91102 wants to merge 1 commit into
larksuite:mainfrom
xu91102:docs/task-inline-image-limitation
Closed

docs(task): clarify inline image limitation#1826
xu91102 wants to merge 1 commit into
larksuite:mainfrom
xu91102:docs/task-inline-image-limitation

Conversation

@xu91102

@xu91102 xu91102 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

背景

Task description/comment 中的内联图片目前通过 Task OpenAPI 只能看到字面量 [Image],没有 media/file token;attachments endpoint 也只覆盖普通 task / task_delivery 附件,不能列出或下载 comment/description 内联图片。

Fixes #1746

核心改动

  • lark-task 主 skill 中明确 Task inline images 的 OpenAPI 边界。
  • +upload-attachment 引用页说明它不是 [Image] 下载路径,只用于上传普通附件。
  • +comment 引用页说明 image-only comments 可能只返回 [Image],需要用户上传普通附件或从 UI 手动下载。
  • 增加文档回归测试,防止这条关键限制被后续文档改动移除。

验证

  • GOCACHE=$PWD/.cache/go-build GOPROXY=https://goproxy.cn,direct go test ./shortcuts/task -count=1:通过
  • GOCACHE=$PWD/.cache/go-build GOPROXY=https://goproxy.cn,direct go build -o ./lark-cli .:通过
  • git diff --check:通过
  • gofmt -l shortcuts/task/task_skill_docs_test.go:通过,无输出
  • rg -n '^(<<<<<<< .+|=======$|>>>>>>> .+)$' --glob '!vendor/**' --glob '!node_modules/**' .:通过,无冲突标记

风险与回滚

风险低。该 PR 只更新 task skill 文档并添加文档测试,不改变 CLI 请求、响应或认证行为。回滚提交即可恢复原文档。

Summary by CodeRabbit

  • Documentation

    • Clarified that inline images in task descriptions and comments can’t be downloaded and may appear as [Image] placeholders.
    • Added guidance on how to handle screenshots by using normal task attachments or downloading them from the Lark UI.
  • Tests

    • Added coverage to verify task skill documentation includes the expected inline-image restrictions and related guidance.

@github-actions github-actions Bot added domain/task PR touches the task domain size/M Single-domain feat or fix with limited business impact labels Jul 9, 2026
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR documents that inline images in task descriptions and comments cannot be downloaded via the Task OpenAPI, adding clarifying notes to SKILL.md and two reference docs, and adds a Go test that verifies these documentation markers exist in the relevant files.

Changes

Inline image limitation docs and validation test

Layer / File(s) Summary
Skill and reference docs clarify inline image limitations
skills/lark-task/SKILL.md, skills/lark-task/references/lark-task-comment.md, skills/lark-task/references/lark-task-upload-attachment.md
Adds notes stating inline images appear as [Image] placeholders with no media/file token, and instructs users to upload attachments normally or download manually from the Lark UI instead of retrying download/upload commands.
Test verifying documentation markers
shortcuts/task/task_skill_docs_test.go
Adds TestTaskSkillDocumentsInlineImageLimit and helper taskSkillRepoRoot to confirm the skill docs contain expected inline-image warning text.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • larksuite/cli#736: Introduced the +upload-attachment shortcut and its reference docs, which this PR's documentation updates directly build on.

Suggested reviewers: liangshuo-1

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR documents the limitation but does not implement the requested API-side retrieval or media-download capability for task inline images in #1746. Implement one of the requested fixes in #1746: expose a downloadable media token or rich-content for task images, or add a task media-download path.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the docs-only clarification about inline image limitations.
Description check ✅ Passed It includes motivation, changes, tests, and related issue, but uses custom headings instead of the template's exact Summary/Changes/Test Plan/Related Issues structure.
Out of Scope Changes check ✅ Passed All changes are on-topic: docs updates plus a regression test that enforces the inline-image limitation wording.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
shortcuts/task/task_skill_docs_test.go (1)

21-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider t.Errorf over t.Fatalf for marker assertions.

Using t.Fatalf stops at the first missing marker, so only one failure is reported. Switching to t.Errorf would surface all missing markers across all files in a single run, making debugging faster when documentation drifts.

Based on learnings, os.* usage in *_test.go files for reading fixtures is the correct pattern in this repo, and cmdutil.TestFactory / t.Setenv are not required since this test exercises no CLI/config/auth behavior.

Optional refactor
 		if !strings.Contains(text, want) {
-			t.Fatalf("%s missing %q", rel, want)
+			t.Errorf("%s missing %q", rel, want)
 		}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@shortcuts/task/task_skill_docs_test.go` around lines 21 - 37, The marker
checks in the test loop currently use t.Fatalf, which stops after the first
missing string and hides other failures. Update the assertion flow in the test
body that reads files with os.ReadFile so it reports each missing marker with
t.Errorf instead of exiting early, keeping the same strings slice and looping
over all files and markers to surface every documentation drift in one run.

Source: Learnings

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@shortcuts/task/task_skill_docs_test.go`:
- Around line 21-37: The marker checks in the test loop currently use t.Fatalf,
which stops after the first missing string and hides other failures. Update the
assertion flow in the test body that reads files with os.ReadFile so it reports
each missing marker with t.Errorf instead of exiting early, keeping the same
strings slice and looping over all files and markers to surface every
documentation drift in one run.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d7539cd6-94f1-4145-88b6-3eb775beafbc

📥 Commits

Reviewing files that changed from the base of the PR and between 74d8458 and 6ad456f.

📒 Files selected for processing (4)
  • shortcuts/task/task_skill_docs_test.go
  • skills/lark-task/SKILL.md
  • skills/lark-task/references/lark-task-comment.md
  • skills/lark-task/references/lark-task-upload-attachment.md

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

PR Quality Summary

CI did not complete successfully. Use the failed check links below to decide whether this PR needs a code change or a rerun.

Failed checks

@github-actions

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@6ad456f5012ec3ff6c4f38e4c2dcf0394a3becdf

🧩 Skill update

npx skills add xu91102/cli#docs/task-inline-image-limitation -y -g

@luoyangxing

Copy link
Copy Markdown
Collaborator

@xu91102 当前 CI 失败不是本 PR 的 task 文档改动导致的。unit-test / coverage 都失败在 TestRunLoadsWaiversFromOverrideFile:测试里的 waiver 已于 2026-07-11 过期。该问题已由 main 上的 e96c4fa 修复,请将分支同步到最新 main 后重新 push 触发 CI。results 是上述失败的汇总,无需单独处理。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/task PR touches the task domain size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Task inline images (description & comments) are unreachable — content returns "[Image]" with no downloadable media token

2 participants