feat(docx): refine history restore flow with revision-fetch fallback - #2127
Open
yballul-bytedance wants to merge 1 commit into
Conversation
Clarify the bounded restore path in the lark-doc history reference: prefer an exact history entry for +history-revert, and when no exact entry exists but the target revision is readable, fall back to fetching the target revision and overwriting the current document with faithful content/reference_map replay, optimistic revision locking, and post-write verification against the target revision. Co-authored-by: TRAE CLI <noreply@bytedance.com>
📝 WalkthroughWalkthroughThe rollback guidance now distinguishes exact history reverts from revision-based fallback restoration. The fallback fetches and validates target data, applies an optimistic-locked overwrite, verifies content and references, and handles conflicts or fidelity failures. ChangesRollback restoration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@skills/lark-doc/references/lark-doc-history.md`:
- Around line 10-15: Update the history restore flow around +history-revert and
+history-revert-status so only status: done proceeds to post-restore
verification; failed, partial_failed, and other terminal non-success statuses
must stop without reporting restoration. For the exact-entry path, obtain and
retain the target document snapshot, including content and reference_map, so the
final docs +fetch verification can compare against it.
- Around line 47-51: Strengthen target response validation before the extraction
commands: in the revision-check jq expression, require that
.data.document.content is a string in addition to matching target_revision_id.
Keep the existing failure behavior so jq -j does not run when content is missing
or null, preventing target-content.xml from being overwritten with invalid data.
🪄 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 Plus
Run ID: e0bbf0ce-5236-46ad-930f-a36659385a23
📒 Files selected for processing (1)
skills/lark-doc/references/lark-doc-history.md
Comment on lines
+10
to
+15
| 4. **有精确 history entry**：用该条目的 `history_version_id` 调用 `+history-revert`。不要猜测或使用邻近条目。默认最多等待 30 秒；如果返回 `status: running`，记录 `task_id`，再用 `+history-revert-status` 轮询到终态。 | ||
| 5. **没有精确 history entry**：不要直接停止。若用户给出了目标 `revision_id`，尝试一次 `docs +fetch --revision-id <revision_id> --detail full --format json`： | ||
| - 成功返回目标 revision 的真实完整内容：将同一次响应的 `data.document.content` 与 `data.document.reference_map` 配套保存并用于 `docs +update --command overwrite`，不得根据摘要、缓存、模型记忆或硬编码重建正文。 | ||
| - 目标 revision 明确不可读或返回业务错误：停止并如实说明。权限、网络或临时系统错误保持原分类，不要据此声称 revision 不存在，也不要绕过身份、权限或确认门禁。 | ||
| 6. `overwrite` 会清空并重建正文，旧 block ID 会失效，当前版本中未包含在目标 fetch 响应里的评论等非正文对象可能丢失。用户明确要求整篇版本恢复时可以替换正文；若还要求保留当前评论或其它未包含在 fetch 响应里的对象，先说明该 fallback 无法保证保留并确认，不得宣称无损恢复。 | ||
| 7. 恢复完成后，用 `docs +fetch --detail full --format json` 读取最新完整文档，验证 `content` 和可回放的 `reference_map` 均与目标 revision 的真实返回一致。 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Only treat status: done as a successful history restore.
The flow currently polls any terminal status and then proceeds to post-restore verification. partial_failed and failed must stop the success flow and must not be reported as restored. The exact-entry path also needs a defined target document snapshot if verification must compare content and reference_map with the target revision.
🤖 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-history.md` around lines 10 - 15, Update
the history restore flow around +history-revert and +history-revert-status so
only status: done proceeds to post-restore verification; failed, partial_failed,
and other terminal non-success statuses must stop without reporting restoration.
For the exact-entry path, obtain and retain the target document snapshot,
including content and reference_map, so the final docs +fetch verification can
compare against it.
Comment on lines
+47
to
+51
| jq -e --arg revision_id "$target_revision_id" \ | ||
| '.data.document.revision_id | tostring == $revision_id' \ | ||
| "$doc_restore_tmp/target/target-revision.json" > /dev/null | ||
| jq -j '.data.document.content' "$doc_restore_tmp/target/target-revision.json" > "$doc_restore_tmp/target/target-content.xml" | ||
| jq '.data.document.reference_map // {}' "$doc_restore_tmp/target/target-revision.json" > "$doc_restore_tmp/target/target-reference-map.json" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject incomplete target responses before overwrite.
The revision check validates only revision_id. jq -j can still succeed when content is missing or null, which can write the literal null into target-content.xml and start a destructive overwrite. Require a string content value before extracting it.
Proposed validation
-jq -j '.data.document.content' "$doc_restore_tmp/target/target-revision.json" > "$doc_restore_tmp/target/target-content.xml"
+jq -er '.data.document.content | select(type == "string")' \
+ "$doc_restore_tmp/target/target-revision.json" > "$doc_restore_tmp/target/target-content.xml"📝 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
| jq -e --arg revision_id "$target_revision_id" \ | |
| '.data.document.revision_id | tostring == $revision_id' \ | |
| "$doc_restore_tmp/target/target-revision.json" > /dev/null | |
| jq -j '.data.document.content' "$doc_restore_tmp/target/target-revision.json" > "$doc_restore_tmp/target/target-content.xml" | |
| jq '.data.document.reference_map // {}' "$doc_restore_tmp/target/target-revision.json" > "$doc_restore_tmp/target/target-reference-map.json" | |
| jq -e --arg revision_id "$target_revision_id" \ | |
| '.data.document.revision_id | tostring == $revision_id' \ | |
| "$doc_restore_tmp/target/target-revision.json" > /dev/null | |
| jq -er '.data.document.content | select(type == "string")' \ | |
| "$doc_restore_tmp/target/target-revision.json" > "$doc_restore_tmp/target/target-content.xml" | |
| jq '.data.document.reference_map // {}' "$doc_restore_tmp/target/target-revision.json" > "$doc_restore_tmp/target/target-reference-map.json" |
🤖 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-history.md` around lines 47 - 51,
Strengthen target response validation before the extraction commands: in the
revision-check jq expression, require that .data.document.content is a string in
addition to matching target_revision_id. Keep the existing failure behavior so
jq -j does not run when content is missing or null, preventing
target-content.xml from being overwritten with invalid data.
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@779b47e764a8fd88900a19955376273b8218931b🧩 Skill updatenpx skills add yballul-bytedance/cli#auto-research-sync/01KYSRNXC7EG74JKVFN4BKMMKA/mr-1340-d83cc2d5 -y -g |
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
Refines the Docx history restore guidance in
skills/lark-doc/references/lark-doc-history.mdso the restore path is an explicit, bounded either/or decision instead of an unconditionalhistory-revertsequence. When an exact history entry exists, the flow still uses+history-revert; when no exact entry exists but the target revision is readable, it adds a safedocs +fetch→overwrite→ verify fallback.Changes
安全流程(safe flow) steps to branch on whether an exact history entry is found: exact entry →+history-revertwith polling; no exact entry → revision-fetch fallback with faithful content replay, or stop honestly on unreadable/error revisions.overwriteclears and rebuilds the body (block IDs invalidated, non-body objects such as comments may be lost) and required explicit confirmation before claiming lossless restore.contentplus a replayablereference_mapagainst the target revision.按 revision_id 或时间点回滚section with a fail-closed bash recipe: isolatedmktemp -dworking dirs, target/currentdocument_idguard, currentrevision_idas an optimistic lock,reference_mapsidecar replay, and a Python content/reference-map comparison that fails closed on mismatch.Test Plan
git diff --checkpasses (no whitespace errors or conflict markers).Related Issues
Auto research task: 01KYSRNXC7EG74JKVFN4BKMMKA
Summary by CodeRabbit