docs(im): Document critical Feishu Markdown silent drop limitations#276
docs(im): Document critical Feishu Markdown silent drop limitations#276Scofy0123 wants to merge 1 commit intolarksuite:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughUpdated the Markdown documentation for Lark IM message sending by adding a new caveat section that explicitly forbids non-HTTP(S) URL schemes in links and warns that bold-wrapped links will be silently destroyed, with an alternative formatting suggestion. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR makes a documentation-only addition to
Confidence Score: 5/5Documentation-only change with no code modifications; safe to merge. The PR adds three lines of accurate, well-placed documentation. No code is changed, no logic is altered, and the added content correctly describes known Feishu client behaviour. No issues were found. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User writes Markdown\nwith --markdown flag] --> B{Link URL scheme?}
B -->|http:// or https://| C[URL accepted by Feishu]
B -->|Other scheme\ne.g. from:user| D[⚠️ Silent drop\nlink or paragraph removed]
C --> E{Wrapped in bold?}
E -->|Plain link| F[Link rendered correctly]
E -->|**Title** wraps link| G[⚠️ Link silently destroyed\nby Feishu Post renderer]
Reviews (2): Last reviewed commit: "docs(im): Document critical Feishu Markd..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/base/base_ops.go`:
- Around line 106-127: The prune logic currently swallows errors from
listAllTables and baseV3Call so callers can miss partial failures; update the
code in the block using listAllTables, baseV3Call and baseV3Path (the flow that
computes defaultTableID and iterates records) to collect and surface errors
instead of ignoring them—either aggregate errors into a returned/pruned error
value or emit structured logs via the existing logger when listAllTables fails,
when the records GET fails, and when a DELETE fails (include the table/record
IDs and underlying error), and ensure the function sets a failure flag or
includes the aggregate error in the function return so callers know pruning was
incomplete.
- Around line 97-130: The dry-run messaging (e.g., dryRunBaseCreate) currently
only mentions the initial POST but the code path guarded by
runtime.Bool("keep-empty-rows") will also perform GET and DELETE calls to remove
empty rows; update the dry-run output/help text to accurately reflect that when
--keep-empty-rows is false the operation will list tables (listAllTables), fetch
records (baseV3Call GET to bases/.../records) and may delete empty records
(baseV3Call DELETE), and clarify that setting --keep-empty-rows prevents these
follow-up GET/DELETE actions. Locate and edit the user-facing string(s) emitted
by dryRunBaseCreate (and any corresponding CLI help for --dry-run /
--keep-empty-rows) to mention the potential destructive GET/DELETE behavior and
the --keep-empty-rows safeguard.
- Around line 114-116: The emptiness guard currently treats missing/non-object
"fields" as empty because it ignores the type-assertion result; change the check
in the deletion branch to first verify the assertion succeeded (e.g., check the
boolean from recMap["fields"].(map[string]interface{})) and only treat the row
as prunable when the assertion is true AND len(fieldsMap) == 0; update the logic
around the variables referenced (recMap and fieldsMap) so the deletion branch
runs only when the type assertion succeeded and the map is actually empty.
🪄 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: de301070-aa8b-4a9b-8917-474da3512162
📒 Files selected for processing (3)
shortcuts/base/base_create.goshortcuts/base/base_ops.goskills/lark-im/references/lark-im-messages-send.md
- Add warning against using non-HTTP(S) schemas in Markdown links which triggers silent client drops. - Add warning against nesting links inside bold tags (`**[title](url)**`), which strips the link completely in the native Feishu Post renderer.
c0dc53a to
1aa7cb9
Compare
This PR adds critical documentation to the
im +messages-sendshortcut regarding Feishu's Markdown parser limitations.Specifically, it warns users about two silent message drop conditions:
[query](from:user)) triggering client-side dropping.**[title](url)**), which strips the link completely in the native Feishu Post renderer.These additions will prevent other agents and CLI users from generating malformed markdown that quietly breaks rendering without raising an API error.
Summary by CodeRabbit