Sun/tempv2#1921
Open
SunPeiYang996 wants to merge 2 commits into
Open
Conversation
Add local XML/Markdown parsing, profiling, conversion, and unique draft creation for docs workflows. Refresh authoring guidance and tests, and ignore generated lark-cli E2E reports. Change-Id: I19cf93d576b6b2cb9ffa4cc4e1640126c95002f8
Change-Id: I54aca2613c8b7493e2b38144b350b34842ec006b
📝 WalkthroughWalkthroughAdds ChangesDocs scripting and parsing
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
domain/ccm
PR Quality SummaryCI did not complete successfully. Use the failed check links below to decide whether this PR needs a code change or a rerun. Failed checks
deterministic-gate
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@147ac3712f7fc17e45d5e0d798fb7302481ebcd1🧩 Skill updatenpx skills add larksuite/cli#sun/tempv2 -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
internal/vfs/localfileio/localfileio.go-85-92 (1)
85-92: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDo not discard temporary-directory cleanup failures.
If
RemoveAllfails, this returns no path while leaving an orphaned directory or file. Include the cleanup failure in the returned typed error so the write is not silently discarded.As per coding guidelines, filesystem writes must not be discarded silently.
🤖 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 `@internal/vfs/localfileio/localfileio.go` around lines 85 - 92, Update the error paths in the temporary-file creation and close handling around tempFile.Close to capture failures from vfs.RemoveAll instead of discarding them. Return a fileio.WriteError that preserves both the original write/close error and cleanup failure, while retaining the existing cleanup attempt and error context.Source: Coding guidelines
skills/lark-doc/references/lark-doc-script.md-66-84 (1)
66-84: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the
--outputand--overwritecontract.Add an example and the saved response fields (
saved_path,size_bytes), including existing-file behavior. This section currently documents only the inlinedata.xmlflow.As per coding guidelines, CLI flags and output formats are machine-consumed interfaces and must be documented specifically.
🤖 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 `@skills/lark-doc/references/lark-doc-script.md` around lines 66 - 84, 补充“Markdown 转 XML”部分对 --output 和 --overwrite 的契约说明：增加将转换结果保存到文件的命令示例，并记录成功响应中的 saved_path 和 size_bytes 字段；同时明确目标文件已存在时未启用 --overwrite 与启用该选项的行为。保留现有内联 data.xml 响应说明，并确保 CLI 参数及输出格式描述准确。Source: Coding guidelines
shortcuts/doc/docs_script.go-121-122 (1)
121-122: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject surrounding whitespace instead of changing the output path.
strings.TrimSpacesilently turns a requested path such as" draft.xml"into"draft.xml". Compare the raw value with its trimmed form and return a typed--outputvalidation error, as already done for--file-name.As per coding guidelines, requested CLI values must not be silently coerced.
Also applies to: 175-188, 220-222, 266-267
🤖 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/doc/docs_script.go` around lines 121 - 122, Update the output-path validation around outputPath and the related output handling at the referenced locations to preserve the raw --output value, compare it with its trimmed form, and return the existing typed --output validation error when surrounding whitespace is present. Match the established --file-name validation behavior without silently coercing the requested path.Source: Coding guidelines
skills/lark-doc/references/lark-doc-whiteboard.md-32-34 (1)
32-34: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winResolve the contradictory SVG ownership rule.
Lines 32-34 allow simple SVG to be written directly by the main Agent, but Step 2B still says SVG insertion uses a SubAgent and instructs the main Agent to start one. Rewrite Step 2B so simple SVG follows the direct path and only design-heavy or layout-sensitive SVG is delegated.
Proposed clarification
-### 步骤 2B: SubAgent 使用 SVG 插入图表 +### 步骤 2B: 使用 SVG 插入图表 -主 Agent 启动 SubAgent，让它用 `docs +create` / `docs +update` 插入： +简单 SVG 由主 Agent 直接写入本地 XML；需要专门视觉设计、较高信息密度或容易布局失败的 SVG，再由主 Agent 启动 SubAgent 使用 `docs +create` / `docs +update` 插入：🤖 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 `@skills/lark-doc/references/lark-doc-whiteboard.md` around lines 32 - 34, 更新步骤 2B 的 SVG 插入规则，消除其与前文的冲突：简单 SVG 由主 Agent 直接写入本地 XML，只有需要专门视觉设计、信息密度较高或布局容易出错的复杂 SVG 才启动 SubAgent 生成完整片段，并保留其他图表类型的既有流程。skills/lark-doc/references/lark-doc-update.md-86-103 (1)
86-103: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument
okas the envelope flag andresultas the business outcome. Inskills/lark-doc/references/lark-doc-update.md:86-103, spell out whetherpartial_success/failedstill returnok: true, and which field clients should branch on.🤖 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 `@skills/lark-doc/references/lark-doc-update.md` around lines 86 - 103, Clarify the response documentation around the result table and JSON example by adding the envelope-level ok field and explicitly distinguishing it from the business-level result field. State whether partial_success and failed responses still use ok: true, and instruct clients to branch on result for update outcomes while using ok only for envelope/request success.Source: Coding guidelines
skills/lark-doc/references/lark-doc-xml-extended-blocks.md-6-14 (1)
6-14: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClarify that these extended-tag lines are signatures, not copyable XML. The placeholders in
<figure view-type>,<button action=OpenLink|...>, etc. should be quoted and filled with real values, or this section should explicitly say it’s a signature list so readers don’t paste invalid markup.🤖 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 `@skills/lark-doc/references/lark-doc-xml-extended-blocks.md` around lines 6 - 14, Clarify in the extended-blocks section that the listed tags are signatures, not directly copyable XML examples. Mark placeholder attributes and values as illustrative or document the required quoting and replacement with real values, especially in the figure, button, time, sheet, task, chat_card, and okr entries.shortcuts/doc/internal/docxparse/markdown_cjk.go-27-39 (1)
27-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCheck fence indentation before trimming it.
Both preprocessors classify four-space-indented backticks as fences, although GFM treats them as indented code. This leaves fence state active and suppresses later transformations. (github.github.com)
shortcuts/doc/internal/docxparse/markdown_cjk.go#L27-L39: enter or close fence state only when indentation after any blockquote prefix is at most three spaces.shortcuts/doc/internal/docxparse/markdown.go#L692-L699: apply the same eligibility check before togglinginFence.As per coding guidelines, transcribe supported input faithfully rather than silently reclassifying it.
🤖 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/doc/internal/docxparse/markdown_cjk.go` around lines 27 - 39, Update the fence detection in shortcuts/doc/internal/docxparse/markdown_cjk.go at lines 27-39 to check indentation after any blockquote prefix before entering or closing fenceMarker state; only allow up to three spaces, while preserving four-space-indented backticks as code. Apply the same eligibility check in shortcuts/doc/internal/docxparse/markdown.go at lines 692-699 before toggling inFence, keeping both preprocessors consistent.Source: Coding guidelines
shortcuts/doc/internal/docxparse/markdown.go-200-205 (1)
200-205: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDetect task checkboxes anywhere in the list. If the first item is plain but a later item is a task,
renderListfalls back to normal list mode and drops the checkbox marker.🤖 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/doc/internal/docxparse/markdown.go` around lines 200 - 205, Update isTaskList to inspect every list item rather than only list.FirstChild(), returning true when findTaskCheckbox identifies a checkbox on any item and false when none do. Preserve safe handling of empty lists and non-list-item children, and ensure renderList uses task-list mode when a later item contains a task checkbox.
🧹 Nitpick comments (1)
internal/vfs/localfileio/localfileio_test.go (1)
309-316: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover every new validation rule directly.
Add table cases for empty patterns/names, patterns with zero or multiple
*, and control/dangerous Unicode characters. The current cases only prove separator rejection.As per coding guidelines, contract tests must assert each new behavior directly.
🤖 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 `@internal/vfs/localfileio/localfileio_test.go` around lines 309 - 316, Expand the table-driven cases in the local file validation tests to cover empty patterns and file names, patterns containing zero or multiple asterisks, and control or dangerous Unicode characters. Assert each validation rule directly while retaining the existing separator-rejection cases.Source: Coding guidelines
🤖 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.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 67-82: Update the results job to include windows-compat in its
needs dependency, summary table, and failure-evaluation loop. Reuse the existing
job-result formatting and failure handling used for the other checks so Windows
failures prevent the aggregate results gate from passing.
In `@shortcuts/doc/docs_script.go`:
- Around line 346-379: Replace the map-based request and response handling in
docsScriptFetchBody and fetchDocsScriptContent with typed request/response
structs, including the nested export options and document content fields. Route
the fetch request through runtime.CallAPITyped instead of doDocAPI, preserving
the existing endpoint, method, language behavior, and return the typed API error
unchanged.
- Around line 382-405: Update saveDocsScriptXML and the FileIO SaveOptions
contract to support exclusive creation when overwrite is false. Pass the
no-replace/exclusive-create option for the non-overwrite path so the write
itself atomically fails if outputPath is created concurrently, while retaining
replacement behavior when overwrite is true and mapping the exclusive-create
failure through the existing save-error handling.
In `@shortcuts/doc/internal/docxparse/markdown_cjk.go`:
- Around line 148-154: Update the CJK markup rewrite logic around
shouldRewriteCJKMarkup so content containing nested Markdown delimiters (* , _ ,
or ~) is not flattened into escaped text; skip rewriting such spans or
recursively process the inner fragment. Preserve supported nested
emphasis/strike structure, including cases like nested emphasis inside a CJK
span, and apply the same fix to the corresponding later rewrite block.
In `@shortcuts/doc/internal/docxparse/markdown_extensions.go`:
- Around line 93-108: The scanMathClose function must skip delimiter candidates
that are escaped, including cases with an odd number of preceding backslashes.
Replace the direct bytes.Index acceptance flow with candidate-by-candidate
validation of preceding backslash parity, continuing the search until an
unescaped delimiter is found while preserving the existing latex-tag and
no-close behavior.
- Around line 294-311: The containerBlock parser must reject blocks that reach
EOF without an explicit closing tag instead of allowing Goldmark to auto-close
them. Update containerBlock and its parser lifecycle, including Continue and
Close, to track whether the matching close tag was consumed, then return the
established validation error when the block ends unclosed while preserving
normal parsing for explicitly closed containers.
In `@shortcuts/doc/internal/docxparse/markdown.go`:
- Around line 130-136: Update the fallback handling in renderBlockNode and
renderInlineNode to return a validation error for unsupported Goldmark node
kinds instead of calling extractMarkdownText and flattening them into plain
text. Preserve existing rendering for explicitly supported nodes and ensure the
error identifies the unhandled node kind.
In `@shortcuts/doc/internal/docxparse/profile.go`:
- Around line 104-109: Update detectFormat to recognize valid Markdown autolinks
such as URI and email forms before the generic strings.HasPrefix(trimmed, "<")
XML check. Return FormatMarkdown for those autolinks, while preserving FormatXML
detection for other leading-angle-bracket content and the existing
BOM/whitespace handling.
- Around line 360-364: Update the node-display handling around the current !=
node branch to traverse visible child text before checking metadata attributes
such as text, name, title, or alt. Only write and return the attribute-derived
display when child traversal produces no visible text, ensuring anchor content
like “Click here” is preferred over its title.
In `@shortcuts/doc/internal/docxparse/schema.go`:
- Around line 228-233: Update the rule transformation handling in the schema
parsing flow so a false result from rule.transform does not continue with the
original alias value. Instead, return and propagate the established typed
validation error for the unsupported value, ensuring invalid aliases such as
column width are rejected rather than silently ignored.
- Around line 166-169: Add "i" to the strictPhrasingTags map so italic elements
are enforced as phrasing-only and cannot contain structural elements such as
tables. Add regression coverage for an <i> element containing a table, verifying
structural validation rejects it.
In `@shortcuts/doc/internal/docxparse/wordcount.go`:
- Around line 64-69: Update textCounter.write and the related ASCII compound
handling so each maximal ASCII/alphanumeric run is consumed and classified once,
rather than repeatedly calling matchASCIICompound on every remaining suffix.
Preserve connector-based compound detection and existing token counting behavior
while eliminating the quadratic scan for long runs.
In `@shortcuts/doc/internal/docxparse/xml.go`:
- Around line 21-29: Update validateSource to detect DOCTYPE and ENTITY
declarations through XML token scanning rather than matching raw source text.
Ignore occurrences inside comments and CDATA sections, while still rejecting
active declarations and preserving the existing input-size validation and error
behavior.
- Around line 153-155: Update the newline normalization in the default branch
handling child.text to replace CR/LF characters with whitespace rather than
deleting them, preserving a word boundary such as “Hello world” before the
existing non-empty check.
In `@skills/lark-doc/references/genres/route-personal-brand.md`:
- Line 14: 更新个人品牌路由条件，明确将邮件草稿也归入 route_platform，并要求选择对应的 email
leaf；保留现有对小红书笔记和微信公众号文章的路由规则，以及个人身份、经历和信誉目标作为 leaf contract 硬约束的要求。
In `@skills/lark-doc/references/lark-doc-create-workflow.md`:
- Line 42: Remove the requirement in Step 3 to display the Presentation Decision
in chain-of-thought, and replace it with a concise user-visible decision summary
or structured JSON record containing only the key decisions without private
reasoning.
- Line 48: Update the JSON contract example near presentation_mode so its value
is one of the declared modes—formal, normal, or rich—rather than explanatory
prose. Move the “contract 为 none 时默认 rich” rule into surrounding documentation
while keeping the contract itself machine-valid.
In `@skills/lark-doc/references/lark-doc-create.md`:
- Line 56: Update skills/lark-doc/references/lark-doc-create.md:56 so multiple
document titles are rejected with a typed, actionable validation error rather
than silently retaining the first. Update
skills/lark-doc/references/lark-doc-fetch.md:65 so context is applied to nested
matches, or the incompatible option is explicitly rejected; do not document
silently ignoring requested context.
In `@skills/lark-doc/references/lark-doc-fetch.md`:
- Line 110: Update the image/source fetching flow for <img> and <source> so
direct GET is allowed only for trusted HTTPS URLs. Validate the initial URL
scheme and reject private, loopback, and link-local hosts, then inspect every
redirect destination with the same checks before following it; otherwise use the
existing token-based preview/download flow.
In `@tests/cli_e2e/docs/docs_script_test.go`:
- Around line 310-315: Update docsScriptE2EEnv to accept *testing.T, set
LARKSUITE_CLI_CONFIG_DIR to t.TempDir(), and preserve the existing environment
entries. Update every caller of docsScriptE2EEnv to pass its test handle so
these non-live E2E tests use isolated CLI configuration.
---
Minor comments:
In `@internal/vfs/localfileio/localfileio.go`:
- Around line 85-92: Update the error paths in the temporary-file creation and
close handling around tempFile.Close to capture failures from vfs.RemoveAll
instead of discarding them. Return a fileio.WriteError that preserves both the
original write/close error and cleanup failure, while retaining the existing
cleanup attempt and error context.
In `@shortcuts/doc/docs_script.go`:
- Around line 121-122: Update the output-path validation around outputPath and
the related output handling at the referenced locations to preserve the raw
--output value, compare it with its trimmed form, and return the existing typed
--output validation error when surrounding whitespace is present. Match the
established --file-name validation behavior without silently coercing the
requested path.
In `@shortcuts/doc/internal/docxparse/markdown_cjk.go`:
- Around line 27-39: Update the fence detection in
shortcuts/doc/internal/docxparse/markdown_cjk.go at lines 27-39 to check
indentation after any blockquote prefix before entering or closing fenceMarker
state; only allow up to three spaces, while preserving four-space-indented
backticks as code. Apply the same eligibility check in
shortcuts/doc/internal/docxparse/markdown.go at lines 692-699 before toggling
inFence, keeping both preprocessors consistent.
In `@shortcuts/doc/internal/docxparse/markdown.go`:
- Around line 200-205: Update isTaskList to inspect every list item rather than
only list.FirstChild(), returning true when findTaskCheckbox identifies a
checkbox on any item and false when none do. Preserve safe handling of empty
lists and non-list-item children, and ensure renderList uses task-list mode when
a later item contains a task checkbox.
In `@skills/lark-doc/references/lark-doc-script.md`:
- Around line 66-84: 补充“Markdown 转 XML”部分对 --output 和 --overwrite
的契约说明：增加将转换结果保存到文件的命令示例，并记录成功响应中的 saved_path 和 size_bytes 字段；同时明确目标文件已存在时未启用
--overwrite 与启用该选项的行为。保留现有内联 data.xml 响应说明，并确保 CLI 参数及输出格式描述准确。
In `@skills/lark-doc/references/lark-doc-update.md`:
- Around line 86-103: Clarify the response documentation around the result table
and JSON example by adding the envelope-level ok field and explicitly
distinguishing it from the business-level result field. State whether
partial_success and failed responses still use ok: true, and instruct clients to
branch on result for update outcomes while using ok only for envelope/request
success.
In `@skills/lark-doc/references/lark-doc-whiteboard.md`:
- Around line 32-34: 更新步骤 2B 的 SVG 插入规则，消除其与前文的冲突：简单 SVG 由主 Agent 直接写入本地
XML，只有需要专门视觉设计、信息密度较高或布局容易出错的复杂 SVG 才启动 SubAgent 生成完整片段，并保留其他图表类型的既有流程。
In `@skills/lark-doc/references/lark-doc-xml-extended-blocks.md`:
- Around line 6-14: Clarify in the extended-blocks section that the listed tags
are signatures, not directly copyable XML examples. Mark placeholder attributes
and values as illustrative or document the required quoting and replacement with
real values, especially in the figure, button, time, sheet, task, chat_card, and
okr entries.
---
Nitpick comments:
In `@internal/vfs/localfileio/localfileio_test.go`:
- Around line 309-316: Expand the table-driven cases in the local file
validation tests to cover empty patterns and file names, patterns containing
zero or multiple asterisks, and control or dangerous Unicode characters. Assert
each validation rule directly while retaining the existing separator-rejection
cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 468eb50f-3a72-49c3-a7ee-2443eea468ee
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (64)
.github/workflows/ci.yml.gitignoreextension/fileio/types.gogo.modinternal/vfs/localfileio/localfileio.gointernal/vfs/localfileio/localfileio_test.goshortcuts/doc/docs_script.goshortcuts/doc/docs_script_test.goshortcuts/doc/internal/docxparse/markdown.goshortcuts/doc/internal/docxparse/markdown_cjk.goshortcuts/doc/internal/docxparse/markdown_extensions.goshortcuts/doc/internal/docxparse/model.goshortcuts/doc/internal/docxparse/parse_test.goshortcuts/doc/internal/docxparse/profile.goshortcuts/doc/internal/docxparse/schema.goshortcuts/doc/internal/docxparse/wordcount.goshortcuts/doc/internal/docxparse/xml.goshortcuts/doc/internal/docxparse/xml_compat.goshortcuts/doc/shortcuts.goskills/lark-doc/SKILL.mdskills/lark-doc/references/genres/business-analysis.mdskills/lark-doc/references/genres/data-report.mdskills/lark-doc/references/genres/email.mdskills/lark-doc/references/genres/execution-plan.mdskills/lark-doc/references/genres/formal-doc.mdskills/lark-doc/references/genres/meeting-minutes.mdskills/lark-doc/references/genres/memo-brief.mdskills/lark-doc/references/genres/official-redhead.mdskills/lark-doc/references/genres/prd.mdskills/lark-doc/references/genres/proposal.mdskills/lark-doc/references/genres/research-report.mdskills/lark-doc/references/genres/retrospective.mdskills/lark-doc/references/genres/route-consumer.mdskills/lark-doc/references/genres/route-creative.mdskills/lark-doc/references/genres/route-knowledge.mdskills/lark-doc/references/genres/route-marketing.mdskills/lark-doc/references/genres/route-media.mdskills/lark-doc/references/genres/route-opinion.mdskills/lark-doc/references/genres/route-personal-brand.mdskills/lark-doc/references/genres/route-platform.mdskills/lark-doc/references/genres/route-report.mdskills/lark-doc/references/genres/route-workplace.mdskills/lark-doc/references/genres/sop-tutorial.mdskills/lark-doc/references/genres/technical-doc.mdskills/lark-doc/references/genres/wechat.mdskills/lark-doc/references/genres/weekly-report.mdskills/lark-doc/references/genres/white-paper.mdskills/lark-doc/references/genres/xiaohongshu.mdskills/lark-doc/references/lark-doc-create-workflow.mdskills/lark-doc/references/lark-doc-create.mdskills/lark-doc/references/lark-doc-fetch.mdskills/lark-doc/references/lark-doc-script.mdskills/lark-doc/references/lark-doc-update.mdskills/lark-doc/references/lark-doc-whiteboard.mdskills/lark-doc/references/lark-doc-word-stat.mdskills/lark-doc/references/lark-doc-xml-extended-blocks.mdskills/lark-doc/references/lark-doc-xml.mdskills/lark-doc/references/style/lark-doc-create-workflow.mdskills/lark-doc/references/style/lark-doc-style.mdskills/lark-doc/references/style/lark-doc-update-workflow.mdskills/lark-doc/scripts/doc_word_stat.pytests/cli_e2e/docs/coverage.mdtests/cli_e2e/docs/docs_create_fetch_test.gotests/cli_e2e/docs/docs_script_test.go
💤 Files with no reviewable changes (5)
- skills/lark-doc/references/style/lark-doc-update-workflow.md
- skills/lark-doc/references/style/lark-doc-style.md
- skills/lark-doc/references/style/lark-doc-create-workflow.md
- skills/lark-doc/references/lark-doc-word-stat.md
- skills/lark-doc/scripts/doc_word_stat.py
Comment on lines
+67
to
+82
| windows-compat: | ||
| needs: fast-gate | ||
| runs-on: windows-latest | ||
| steps: | ||
| - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 | ||
| - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 | ||
| with: | ||
| go-version-file: go.mod | ||
| - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 | ||
| with: | ||
| python-version: '3.x' | ||
| - name: Fetch meta data | ||
| run: python scripts/fetch_meta.py | ||
| - name: Run Windows compatibility tests | ||
| run: go test -count=1 -timeout=5m . ./shortcuts/doc/... | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include windows-compat in the aggregate results gate.
The results job neither depends on nor evaluates this job, so the aggregate check can pass despite Windows failures. Add it to results.needs, the summary table, and the failure loop.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 71-71: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/ci.yml around lines 67 - 82, Update the results job to
include windows-compat in its needs dependency, summary table, and
failure-evaluation loop. Reuse the existing job-result formatting and failure
handling used for the other checks so Windows failures prevent the aggregate
results gate from passing.
Comment on lines
+346
to
+379
| func docsScriptFetchBody(runtime *common.RuntimeContext) map[string]interface{} { | ||
| body := map[string]interface{}{ | ||
| "format": "xml", | ||
| "extra_param": docsFetchExtraParam, | ||
| "export_option": map[string]interface{}{ | ||
| "export_block_id": false, | ||
| "export_style_attrs": false, | ||
| "export_cite_extra_data": false, | ||
| }, | ||
| } | ||
| if lang := resolveFetchLang(runtime); lang != "" { | ||
| body["lang"] = lang | ||
| } | ||
| return body | ||
| } | ||
|
|
||
| func fetchDocsScriptContent(runtime *common.RuntimeContext) (string, error) { | ||
| ref, _ := parseDocumentRef(runtime.Str("doc")) | ||
| apiPath := fmt.Sprintf("/open-apis/docs_ai/v1/documents/%s/fetch", ref.Token) | ||
| data, err := doDocAPI(runtime, "POST", apiPath, docsScriptFetchBody(runtime)) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| document, ok := data["document"].(map[string]interface{}) | ||
| if !ok || document == nil { | ||
| return "", errs.NewInternalError(errs.SubtypeUnknown, | ||
| "document fetch response for --doc is missing document") | ||
| } | ||
| content, ok := document["content"].(string) | ||
| if !ok { | ||
| return "", errs.NewInternalError(errs.SubtypeUnknown, | ||
| "document fetch response for --doc is missing document.content") | ||
| } | ||
| return content, nil |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Use typed request/response models and runtime.CallAPITyped.
This introduces loose maps for both the fetch body and response decoding, while routing the request through doDocAPI. Define request/response structs and return the typed API error unchanged.
As per coding guidelines, shortcuts must use runtime.CallAPITyped for Lark API requests and must not introduce new map[string]interface{} boundary parsing.
🤖 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/doc/docs_script.go` around lines 346 - 379, Replace the map-based
request and response handling in docsScriptFetchBody and fetchDocsScriptContent
with typed request/response structs, including the nested export options and
document content fields. Route the fetch request through runtime.CallAPITyped
instead of doDocAPI, preserving the existing endpoint, method, language
behavior, and return the typed API error unchanged.
Source: Coding guidelines
Comment on lines
+382
to
+405
| func saveDocsScriptXML(runtime *common.RuntimeContext, outputPath, xml string) error { | ||
| if !runtime.Bool("overwrite") { | ||
| if _, err := runtime.FileIO().Stat(outputPath); err == nil { | ||
| return errs.NewValidationError(errs.SubtypeFailedPrecondition, | ||
| "output file already exists: %s (use --overwrite to replace)", outputPath). | ||
| WithParam("--output") | ||
| } else if !errors.Is(err, fs.ErrNotExist) { | ||
| if errors.Is(err, fileio.ErrPathValidation) { | ||
| return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err). | ||
| WithParam("--output"). | ||
| WithCause(err) | ||
| } | ||
| return errs.NewInternalError(errs.SubtypeFileIO, | ||
| "cannot access output path %s: %s", outputPath, err). | ||
| WithCause(err) | ||
| } | ||
| } | ||
|
|
||
| result, err := runtime.FileIO().Save(outputPath, fileio.SaveOptions{ | ||
| ContentType: "application/xml", | ||
| ContentLength: int64(len(xml)), | ||
| }, strings.NewReader(xml)) | ||
| if err != nil { | ||
| return common.WrapSaveErrorTyped(err) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make --overwrite=false an atomic no-clobber operation.
The separate Stat and Save calls race: two processes can both observe a missing file, then the later atomic replacement silently overwrites the earlier write. Add an exclusive-create/no-replace capability to FileIO and use it here.
As per coding guidelines, requested write semantics must be honored without discarding another write.
🤖 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/doc/docs_script.go` around lines 382 - 405, Update
saveDocsScriptXML and the FileIO SaveOptions contract to support exclusive
creation when overwrite is false. Pass the no-replace/exclusive-create option
for the non-overwrite path so the write itself atomically fails if outputPath is
created concurrently, while retaining replacement behavior when overwrite is
true and mapping the exclusive-create failure through the existing save-error
handling.
Source: Coding guidelines
Comment on lines
+148
to
+154
| content := runes[i+len(rule.delimiter) : closeAt] | ||
| if !shouldRewriteCJKMarkup(content) { | ||
| continue | ||
| } | ||
| out.WriteString(rule.openXML) | ||
| out.WriteString(escapeXMLText(stripBackslashEscapes(string(content)))) | ||
| out.WriteString(rule.closeXML) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not flatten nested Markdown inside rewritten CJK spans.
shouldRewriteCJKMarkup permits *, _, and ~, but the rewrite emits the entire content as escaped text. Inputs such as **中文 *重点*。**下一步 therefore lose nested emphasis.
Either parse the inner fragment recursively or skip preprocessing when nested delimiters are present.
Minimal safe fix
for _, r := range content {
- if r == '`' || r == '[' || r == ']' || r == '<' || r == '>' {
+ if r == '`' || r == '[' || r == ']' || r == '<' || r == '>' ||
+ r == '*' || r == '_' || r == '~' {
return false
}
}As per coding guidelines, “Transcribe echoed input faithfully” without discarding supported structure.
Also applies to: 250-258
🤖 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/doc/internal/docxparse/markdown_cjk.go` around lines 148 - 154,
Update the CJK markup rewrite logic around shouldRewriteCJKMarkup so content
containing nested Markdown delimiters (* , _ , or ~) is not flattened into
escaped text; skip rewriting such spans or recursively process the inner
fragment. Preserve supported nested emphasis/strike structure, including cases
like nested emphasis inside a CJK span, and apply the same fix to the
corresponding later rewrite block.
Source: Coding guidelines
Comment on lines
+93
to
+108
| func scanMathClose(data []byte, delimiter string) ([]byte, int) { | ||
| delim := []byte(delimiter) | ||
| for offset := 0; offset < len(data); { | ||
| if data[offset] == '\\' && offset+1 < len(data) && data[offset+1] == '$' { | ||
| offset += 2 | ||
| continue | ||
| } | ||
| rel := bytes.Index(data[offset:], delim) | ||
| if rel < 0 { | ||
| return nil, -1 | ||
| } | ||
| end := offset + rel | ||
| if bytes.Contains(data[:end], []byte("<latex")) { | ||
| return nil, -1 | ||
| } | ||
| return data[:end], end + len(delim) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not treat escaped dollars as math closers.
bytes.Index jumps directly to the next delimiter without checking whether that candidate is escaped. For example, $a\$b$ closes at \$, truncating the formula. Check each candidate’s preceding backslash parity before accepting it.
🤖 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/doc/internal/docxparse/markdown_extensions.go` around lines 93 -
108, The scanMathClose function must skip delimiter candidates that are escaped,
including cases with an odd number of preceding backslashes. Replace the direct
bytes.Index acceptance flow with candidate-by-candidate validation of preceding
backslash parity, continuing the search until an unescaped delimiter is found
while preserving the existing latex-tag and no-close behavior.
| |-|-| | ||
| | [`route-platform.md`](genres/route-platform.md) | Email、微信公众号、小红书 | | ||
|
|
||
| ### Step 3：在生成草稿前完成 Presentation Decision，并使用下方 JSON 结构记录关键决策，**在思维链里显示输出**。 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not require chain-of-thought disclosure.
“在思维链里显示输出” is not a safe or reliable workflow interface. Replace it with a concise, user-visible decision summary or a structured record that does not expose private reasoning.
🤖 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 `@skills/lark-doc/references/lark-doc-create-workflow.md` at line 42, Remove
the requirement in Step 3 to display the Presentation Decision in
chain-of-thought, and replace it with a concise user-visible decision summary or
structured JSON record containing only the key decisions without private
reasoning.
| "target": "", | ||
| "genre_contract": "", | ||
| "adapter": "", | ||
| "presentation_mode": "当 contract 为 none 时，默认 rich 模式；", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use a valid presentation_mode value in the JSON contract.
This field currently contains instruction prose rather than one of the declared values (formal, normal, or rich), so consumers cannot treat the example as a valid decision record. Keep the default rule in separate documentation and set this field to an actual mode value.
🤖 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 `@skills/lark-doc/references/lark-doc-create-workflow.md` at line 48, Update
the JSON contract example near presentation_mode so its value is one of the
declared modes—formal, normal, or rich—rather than explanatory prose. Move the
“contract 为 none 时默认 rich” rule into surrounding documentation while keeping the
contract itself machine-valid.
| - [`lark-doc-media-insert.md`](lark-doc-media-insert.md) — 插入图片/文件到文档 | ||
| |参数|必填|说明| | ||
| |-|-|-| | ||
| |`--title`|否|文档标题，Markdown 导入时使用；XML 创建推荐在 `--content` 开头写 `<title>...</title>`；多个标题仅保留第一个并在 `warnings` / `degrade_details` 提示| |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not silently discard or ignore requested document content.
These documented fallbacks violate the repository contract: unsupported or ambiguous input must be rejected with a typed, actionable validation error, or honored without loss.
skills/lark-doc/references/lark-doc-create.md#L56-L56: reject multiple titles instead of retaining only the first.skills/lark-doc/references/lark-doc-fetch.md#L65-L65: either apply context to nested matches or reject the incompatible option instead of ignoring it.
📍 Affects 2 files
skills/lark-doc/references/lark-doc-create.md#L56-L56(this comment)skills/lark-doc/references/lark-doc-fetch.md#L65-L65
🤖 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 `@skills/lark-doc/references/lark-doc-create.md` at line 56, Update
skills/lark-doc/references/lark-doc-create.md:56 so multiple document titles are
rejected with a typed, actionable validation error rather than silently
retaining the first. Update skills/lark-doc/references/lark-doc-fetch.md:65 so
context is applied to nested matches, or the incompatible option is explicitly
rejected; do not document silently ignoring requested context.
Source: Coding guidelines
| 返回中可能含 `<sheet>`、`<bitable>`、`<cite file-type="sheets|bitable">`。内部数据无法通过 `docs +fetch` 获取，提取 `token` 等属性后切到 [`lark-sheets`](../../lark-sheets/SKILL.md) / [`lark-base`](../../lark-base/SKILL.md) 下钻，详见 [SKILL.md 快速决策](../SKILL.md) 路由表。 | ||
| |返回内容|处理方式| | ||
| |-|-| | ||
| |`<img>`、`<source>`|有 `url` 时直接 HTTP GET；否则提取 `token`，预览用 `docs +media-preview`，下载用 `docs +media-download`| |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="skills/lark-doc/references/lark-doc-fetch.md"
echo "== file metadata =="
wc -l "$file"
echo
echo "== relevant lines =="
sed -n '90,130p' "$file" | cat -n
echo
echo "== search for direct HTTP GET / url handling references =="
rg -n "HTTP GET|url|media-preview|media-download|source|img" skills/lark-doc -SRepository: larksuite/cli
Length of output: 12091
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== lark-doc-xml relevant section =="
sed -n '1,80p' skills/lark-doc/references/lark-doc-xml.md | cat -n
echo
echo "== media-preview / media-download sections =="
sed -n '1,120p' skills/lark-doc/references/lark-doc-media-preview.md | cat -n
echo
sed -n '1,120p' skills/lark-doc/references/lark-doc-media-download.md | cat -n
echo
echo "== resource-cover url constraints =="
sed -n '1,90p' skills/lark-doc/references/lark-doc-resource-cover.md | cat -nRepository: larksuite/cli
Length of output: 9384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== lark-shared security rules =="
sed -n '1,220p' skills/lark-shared/SKILL.md | cat -n
echo
echo "== fetch doc surrounding the affected line =="
sed -n '100,116p' skills/lark-doc/references/lark-doc-fetch.md | cat -n
echo
echo "== search for SSRF / URL restriction guidance across skills =="
rg -n "SSRF|private|loopback|link-local|multicast|userinfo|https://|HTTP GET|direct HTTP GET|publicly|安全边界|URL" skills -SRepository: larksuite/cli
Length of output: 50372
Restrict direct GET to trusted HTTPS URLs. If url can come from document content, fetching it directly can hit internal endpoints; require https://, reject private/loopback/link-local hosts, and re-check redirects before following the link.
🤖 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 `@skills/lark-doc/references/lark-doc-fetch.md` at line 110, Update the
image/source fetching flow for <img> and <source> so direct GET is allowed only
for trusted HTTPS URLs. Validate the initial URL scheme and reject private,
loopback, and link-local hosts, then inspect every redirect destination with the
same checks before following it; otherwise use the existing token-based
preview/download flow.
Comment on lines
+310
to
+315
| func docsScriptE2EEnv() map[string]string { | ||
| return map[string]string{ | ||
| "LARKSUITE_CLI_APP_ID": "docs-script-e2e", | ||
| "LARKSUITE_CLI_APP_SECRET": "secret", | ||
| "LARKSUITE_CLI_BRAND": "feishu", | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Isolate CLI configuration for these non-live E2E tests.
Accept t and set LARKSUITE_CLI_CONFIG_DIR to t.TempDir(); update callers to use docsScriptE2EEnv(t).
Proposed fix
-func docsScriptE2EEnv() map[string]string {
+func docsScriptE2EEnv(t *testing.T) map[string]string {
+ t.Helper()
return map[string]string{
"LARKSUITE_CLI_APP_ID": "docs-script-e2e",
"LARKSUITE_CLI_APP_SECRET": "secret",
"LARKSUITE_CLI_BRAND": "feishu",
+ "LARKSUITE_CLI_CONFIG_DIR": t.TempDir(),
}
}As per coding guidelines, tests must isolate configuration state with LARKSUITE_CLI_CONFIG_DIR.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func docsScriptE2EEnv() map[string]string { | |
| return map[string]string{ | |
| "LARKSUITE_CLI_APP_ID": "docs-script-e2e", | |
| "LARKSUITE_CLI_APP_SECRET": "secret", | |
| "LARKSUITE_CLI_BRAND": "feishu", | |
| } | |
| func docsScriptE2EEnv(t *testing.T) map[string]string { | |
| t.Helper() | |
| return map[string]string{ | |
| "LARKSUITE_CLI_APP_ID": "docs-script-e2e", | |
| "LARKSUITE_CLI_APP_SECRET": "secret", | |
| "LARKSUITE_CLI_BRAND": "feishu", | |
| "LARKSUITE_CLI_CONFIG_DIR": t.TempDir(), | |
| } |
🤖 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 `@tests/cli_e2e/docs/docs_script_test.go` around lines 310 - 315, Update
docsScriptE2EEnv to accept *testing.T, set LARKSUITE_CLI_CONFIG_DIR to
t.TempDir(), and preserve the existing environment entries. Update every caller
of docsScriptE2EEnv to pass its test handle so these non-live E2E tests use
isolated CLI configuration.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Test Plan
lark-cli <domain> <command>flow works as expectedRelated Issues
Summary by CodeRabbit
docs +scriptcommand for parsing documents, profiling text, converting Markdown to XML, and creating temporary XML files.