feat(wiki): add wiki move shortcut with async task polling#436
feat(wiki): add wiki move shortcut with async task polling#436fangshuyu-768 merged 2 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WikiMove as Wiki Move<br/>Shortcut
participant WikiAPI as Wiki API
participant TaskPolling as Task Polling<br/>Logic
Client->>WikiMove: Execute docs_to_wiki mode
WikiMove->>WikiAPI: POST /spaces/{space_id}/nodes/move_docs_to_wiki
WikiAPI-->>WikiMove: Response (task_id | wiki_token | applied)
alt Synchronous (wiki_token)
WikiMove-->>Client: Ready result with node fields
else Applied (approval needed)
WikiMove-->>Client: Submitted status
else Asynchronous (task_id)
WikiMove->>TaskPolling: Start polling loop (max 30 attempts)
loop Poll until ready/failed/timeout
TaskPolling->>WikiAPI: GET /tasks/{task_id}?task_type=move
WikiAPI-->>TaskPolling: Task status
end
alt Polling success
TaskPolling-->>WikiMove: Ready or failed status
WikiMove-->>Client: Result with move_results
else Polling timeout
TaskPolling-->>Client: Timeout result with next_command hint
end
end
sequenceDiagram
participant Client
participant WikiMove as Wiki Move<br/>Shortcut
participant WikiAPI as Wiki API
Client->>WikiMove: Execute node_move mode
alt Missing source_space_id
WikiMove->>WikiAPI: GET /spaces/get_node?node_token={node_token}
WikiAPI-->>WikiMove: Node space_id
end
alt target-parent-token provided
WikiMove->>WikiAPI: GET /spaces/get_node?node_token={parent_token}
WikiAPI-->>WikiMove: Parent space_id
WikiMove->>WikiMove: Validate space_id consistency
end
WikiMove->>WikiAPI: POST /spaces/{source_space_id}/nodes/{node_token}/move
WikiAPI-->>WikiMove: Move result
WikiMove-->>Client: Result with source/target space_ids and node fields
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Comment |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@afcdc431b54e7e17b4632ea8c03ee3bdae350416🧩 Skill updatenpx skills add larksuite/cli#feat/wiki_move_shortcut -y -g |
There was a problem hiding this comment.
🧹 Nitpick comments (5)
shortcuts/drive/drive_task_result.go (2)
18-23: Refresh the shortcut doc comment.The exported comment above
DriveTaskResultstill describes only Drive import/export/folder-move flows, butwiki_moveis now part of the public surface too. Keeping that GoDoc in sync will avoid stale context around this shortcut.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_task_result.go` around lines 18 - 23, Update the exported GoDoc comment above the DriveTaskResult variable to accurately reflect current supported operations by mentioning wiki_move in addition to import, export, drive move/delete; locate the comment immediately above the DriveTaskResult declaration and edit the text so the description includes wiki_move as part of the public surface.
31-40: Consider centralizing the scenario definitions.
wiki_movenow has to be added in several places here plusshortcuts/drive/drive_task_result_wiki.gofor scope rules. A small per-scenario registry for required flags, scope checks, dry-run wiring, and executor wiring would make future scenario additions much harder to wire up partially.Also applies to: 43-78, 80-108, 112-134
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_task_result.go` around lines 31 - 40, The Validate function currently hardcodes validScenarios (and similar per-scenario logic elsewhere) causing duplicated wiring for new scenarios like "wiki_move"; replace these dispersed checks with a centralized scenario registry: define a map (e.g., scenarioRegistry) keyed by scenario string to a struct containing required flags, scope checks, dry-run wiring, and executor wiring, then update Validate (the scenario variable and validScenarios lookup), the scope/dry-run wiring code paths, and the executor selection logic to consult scenarioRegistry instead of per-file hardcoded branches so adding a new scenario only requires registering it once.shortcuts/wiki/wiki_move.go (1)
432-437: Inconsistent path segment encoding in dry-run.
dryRunWikiMoveSourceSpaceIDappliesvalidate.EncodePathSegmentwhen returning the explicit source space ID (line 427), butdryRunWikiMoveTargetSpaceIDreturns the rawspec.TargetSpaceIDwithout encoding. For consistency and to avoid issues with special characters in space IDs, consider encoding here as well.♻️ Suggested fix
func dryRunWikiMoveTargetSpaceID(spec wikiMoveSpec) string { if spec.TargetSpaceID != "" { - return spec.TargetSpaceID + return validate.EncodePathSegment(spec.TargetSpaceID) } return "<target_space_id>" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/wiki/wiki_move.go` around lines 432 - 437, dryRunWikiMoveTargetSpaceID currently returns spec.TargetSpaceID raw while dryRunWikiMoveSourceSpaceID encodes the value; change dryRunWikiMoveTargetSpaceID to return validate.EncodePathSegment(spec.TargetSpaceID) when TargetSpaceID is non-empty so both source and target IDs are consistently path-segment encoded (update the return in dryRunWikiMoveTargetSpaceID to mirror dryRunWikiMoveSourceSpaceID).shortcuts/drive/drive_task_result_wiki.go (2)
17-97: Consider extracting shared scope validation logic.The scope validation logic (
taskResultScopeRequirementGroup,requireDriveAnyScopeGroups,hasAnyDriveGrantedScope) is nearly identical toscopeRequirementGroup,requireAnyScopeGroups,hasAnyGrantedScopeinshortcuts/wiki/wiki_move.go(lines 686-743).While the types live in different packages making direct sharing harder, this could be extracted to a shared package (e.g.,
shortcuts/common) to reduce duplication and ensure consistent behavior. This is optional and could be deferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_task_result_wiki.go` around lines 17 - 97, The duplicated scope-validation logic (taskResultScopeRequirementGroup, validateDriveTaskResultScopes, requireDriveAnyScopeGroups, hasAnyDriveGrantedScope) mirrors scopeRequirementGroup, requireAnyScopeGroups, hasAnyGrantedScope in shortcuts/wiki/wiki_move.go; extract these into a shared package (e.g., shortcuts/common) as a single generic implementation (e.g., type scopeRequirementGroup, func requireAnyScopeGroups(storedScopes string, groups []scopeRequirementGroup) error, func hasAnyGrantedScope(granted map[string]bool, options []string) bool), update both validateDriveTaskResultScopes and the wiki_move code to call the shared functions/types and adjust imports, and remove the duplicated definitions; if extraction is deferred, add a TODO in both files referencing the planned shared package and ensure both implementations remain behaviorally identical.
99-159: Duplicated task status types mirror wiki package.
wikiMoveTaskQueryStatusand its methods closely mirrorwikiMoveTaskStatusinshortcuts/wiki/wiki_move.go. The main difference iswikiMoveTaskResultStatus.Nodeusesmap[string]interface{}here vs*wikiNodeRecordin the wiki package.This duplication is understandable given the package boundaries, but worth noting for future maintainability—changes to task status semantics would need updates in both places.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_task_result_wiki.go` around lines 99 - 159, The new wiki-specific task status types duplicate logic in shortcuts/wiki/wiki_move.go (types wikiMoveTaskQueryStatus and wikiMoveTaskResultStatus plus methods Ready, Failed, FirstResult, PrimaryStatusCode, PrimaryStatusLabel); extract the shared struct(s) and methods into a single shared package or the existing shortcuts/wiki package (e.g., MoveTaskResultStatus / MoveTaskQueryStatus or reuse wikiMoveTaskStatus) and update this file to import and use those shared types instead of redefining them; ensure the Node field type is reconciled (either change map[string]interface{} to the canonical *wikiNodeRecord used by the wiki package or add a conversion helper where needed) so callers compile without behavioral changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/drive/drive_task_result_wiki.go`:
- Around line 17-97: The duplicated scope-validation logic
(taskResultScopeRequirementGroup, validateDriveTaskResultScopes,
requireDriveAnyScopeGroups, hasAnyDriveGrantedScope) mirrors
scopeRequirementGroup, requireAnyScopeGroups, hasAnyGrantedScope in
shortcuts/wiki/wiki_move.go; extract these into a shared package (e.g.,
shortcuts/common) as a single generic implementation (e.g., type
scopeRequirementGroup, func requireAnyScopeGroups(storedScopes string, groups
[]scopeRequirementGroup) error, func hasAnyGrantedScope(granted map[string]bool,
options []string) bool), update both validateDriveTaskResultScopes and the
wiki_move code to call the shared functions/types and adjust imports, and remove
the duplicated definitions; if extraction is deferred, add a TODO in both files
referencing the planned shared package and ensure both implementations remain
behaviorally identical.
- Around line 99-159: The new wiki-specific task status types duplicate logic in
shortcuts/wiki/wiki_move.go (types wikiMoveTaskQueryStatus and
wikiMoveTaskResultStatus plus methods Ready, Failed, FirstResult,
PrimaryStatusCode, PrimaryStatusLabel); extract the shared struct(s) and methods
into a single shared package or the existing shortcuts/wiki package (e.g.,
MoveTaskResultStatus / MoveTaskQueryStatus or reuse wikiMoveTaskStatus) and
update this file to import and use those shared types instead of redefining
them; ensure the Node field type is reconciled (either change
map[string]interface{} to the canonical *wikiNodeRecord used by the wiki package
or add a conversion helper where needed) so callers compile without behavioral
changes.
In `@shortcuts/drive/drive_task_result.go`:
- Around line 18-23: Update the exported GoDoc comment above the DriveTaskResult
variable to accurately reflect current supported operations by mentioning
wiki_move in addition to import, export, drive move/delete; locate the comment
immediately above the DriveTaskResult declaration and edit the text so the
description includes wiki_move as part of the public surface.
- Around line 31-40: The Validate function currently hardcodes validScenarios
(and similar per-scenario logic elsewhere) causing duplicated wiring for new
scenarios like "wiki_move"; replace these dispersed checks with a centralized
scenario registry: define a map (e.g., scenarioRegistry) keyed by scenario
string to a struct containing required flags, scope checks, dry-run wiring, and
executor wiring, then update Validate (the scenario variable and validScenarios
lookup), the scope/dry-run wiring code paths, and the executor selection logic
to consult scenarioRegistry instead of per-file hardcoded branches so adding a
new scenario only requires registering it once.
In `@shortcuts/wiki/wiki_move.go`:
- Around line 432-437: dryRunWikiMoveTargetSpaceID currently returns
spec.TargetSpaceID raw while dryRunWikiMoveSourceSpaceID encodes the value;
change dryRunWikiMoveTargetSpaceID to return
validate.EncodePathSegment(spec.TargetSpaceID) when TargetSpaceID is non-empty
so both source and target IDs are consistently path-segment encoded (update the
return in dryRunWikiMoveTargetSpaceID to mirror dryRunWikiMoveSourceSpaceID).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed3cc95f-1ccd-4b35-89a2-8145d09b55c8
📒 Files selected for processing (11)
shortcuts/drive/drive_task_result.goshortcuts/drive/drive_task_result_test.goshortcuts/drive/drive_task_result_wiki.goshortcuts/drive/drive_task_result_wiki_test.goshortcuts/wiki/shortcuts.goshortcuts/wiki/wiki_move.goshortcuts/wiki/wiki_move_test.goshortcuts/wiki/wiki_node_create_test.goskills/lark-drive/references/lark-drive-move.mdskills/lark-wiki/SKILL.mdskills/lark-wiki/references/lark-wiki-move.md
Greptile SummaryThis PR introduces a
Confidence Score: 4/5Safe to merge after addressing the my_library space-mismatch defect in resolveWikiNodeMoveSpaces One confirmed P1 defect: combining --target-space-id my_library with --target-parent-token in node-move mode always returns a false validation error because the alias is never resolved to the real numeric space ID before the equality check. shortcuts/wiki/wiki_move.go — resolveWikiNodeMoveSpaces needs my_library resolution; shortcuts/wiki/wiki_move_test.go — add a test for my_library + target-parent-token Important Files Changed
Reviews (6): Last reviewed commit: "Merge branch 'main' into feat/wiki_move_..." | Re-trigger Greptile |
Change-Id: I58400054e6c3c3c8e7b0cf72b874602b22fa287d
343a0c4 to
d271fb4
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/wiki/wiki_move.go (1)
390-395: Minor inconsistency: MissingEncodePathSegmentin dry-run helper.
dryRunWikiMoveSourceSpaceID(line 385) encodes the path segment, butdryRunWikiMoveTargetSpaceIDreturns the raw value. For consistency in dry-run output, consider encoding here as well.♻️ Suggested fix
func dryRunWikiMoveTargetSpaceID(spec wikiMoveSpec) string { if spec.TargetSpaceID != "" { - return spec.TargetSpaceID + return validate.EncodePathSegment(spec.TargetSpaceID) } return "<target_space_id>" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/wiki/wiki_move.go` around lines 390 - 395, dryRunWikiMoveTargetSpaceID currently returns spec.TargetSpaceID raw while dryRunWikiMoveSourceSpaceID encodes the value; update dryRunWikiMoveTargetSpaceID to mirror that behavior by returning EncodePathSegment(spec.TargetSpaceID) when spec.TargetSpaceID is non-empty (otherwise keep "<target_space_id>") so dry-run outputs are consistent; locate the function dryRunWikiMoveTargetSpaceID and apply EncodePathSegment accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/wiki/wiki_move.go`:
- Around line 390-395: dryRunWikiMoveTargetSpaceID currently returns
spec.TargetSpaceID raw while dryRunWikiMoveSourceSpaceID encodes the value;
update dryRunWikiMoveTargetSpaceID to mirror that behavior by returning
EncodePathSegment(spec.TargetSpaceID) when spec.TargetSpaceID is non-empty
(otherwise keep "<target_space_id>") so dry-run outputs are consistent; locate
the function dryRunWikiMoveTargetSpaceID and apply EncodePathSegment
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c74c1862-9263-4ee4-971c-5795c2b191a3
📒 Files selected for processing (11)
shortcuts/drive/drive_task_result.goshortcuts/drive/drive_task_result_test.goshortcuts/drive/drive_task_result_wiki.goshortcuts/drive/drive_task_result_wiki_test.goshortcuts/wiki/shortcuts.goshortcuts/wiki/wiki_move.goshortcuts/wiki/wiki_move_test.goshortcuts/wiki/wiki_node_create_test.goskills/lark-drive/references/lark-drive-move.mdskills/lark-wiki/SKILL.mdskills/lark-wiki/references/lark-wiki-move.md
✅ Files skipped from review due to trivial changes (3)
- shortcuts/drive/drive_task_result_test.go
- skills/lark-wiki/SKILL.md
- skills/lark-drive/references/lark-drive-move.md
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/wiki/shortcuts.go
There was a problem hiding this comment.
🧹 Nitpick comments (2)
skills/lark-wiki/references/lark-wiki-move.md (1)
175-178: Minor: Blank line between blockquotes triggers markdownlint warning.The blank line between
> When...and> [!CAUTION]causes MD028. This is cosmetic and does not affect rendering in most Markdown parsers.📝 Optional fix to remove blank line
> 当异步任务超时后,后续 `lark-cli drive +task_result --scenario wiki_move --task-id <TASK_ID>` 只需要 `wiki:space:read` 权限。 - +> > [!CAUTION]Or consolidate into a single blockquote block without the blank line.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-wiki/references/lark-wiki-move.md` around lines 175 - 178, Remove the blank line between the two blockquote lines that triggers MD028: merge the separate blockquote segments so that the line starting with "> When..." and the subsequent "> [!CAUTION]" follow consecutively (or combine them into a single multi-line blockquote) to eliminate the MD028 warning while preserving the caution content and wording.shortcuts/wiki/wiki_move.go (1)
390-395: Minor inconsistency:dryRunWikiMoveTargetSpaceIDdoesn't encode the path segment.
dryRunWikiMoveSourceSpaceID(line 385) callsvalidate.EncodePathSegment, butdryRunWikiMoveTargetSpaceID(line 392) returns the raw value. This is inconsistent, though both may work in practice since space IDs typically don't need encoding.♻️ Align encoding behavior with dryRunWikiMoveSourceSpaceID
func dryRunWikiMoveTargetSpaceID(spec wikiMoveSpec) string { if spec.TargetSpaceID != "" { - return spec.TargetSpaceID + return validate.EncodePathSegment(spec.TargetSpaceID) } return "<target_space_id>" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/wiki/wiki_move.go` around lines 390 - 395, dryRunWikiMoveTargetSpaceID returns the raw TargetSpaceID whereas dryRunWikiMoveSourceSpaceID uses validate.EncodePathSegment; update dryRunWikiMoveTargetSpaceID to mirror the source behavior by returning validate.EncodePathSegment(spec.TargetSpaceID) when non-empty (and keep the "<target_space_id>" fallback) so both source and target IDs are consistently encoded; modify the dryRunWikiMoveTargetSpaceID function accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/wiki/wiki_move.go`:
- Around line 390-395: dryRunWikiMoveTargetSpaceID returns the raw TargetSpaceID
whereas dryRunWikiMoveSourceSpaceID uses validate.EncodePathSegment; update
dryRunWikiMoveTargetSpaceID to mirror the source behavior by returning
validate.EncodePathSegment(spec.TargetSpaceID) when non-empty (and keep the
"<target_space_id>" fallback) so both source and target IDs are consistently
encoded; modify the dryRunWikiMoveTargetSpaceID function accordingly.
In `@skills/lark-wiki/references/lark-wiki-move.md`:
- Around line 175-178: Remove the blank line between the two blockquote lines
that triggers MD028: merge the separate blockquote segments so that the line
starting with "> When..." and the subsequent "> [!CAUTION]" follow consecutively
(or combine them into a single multi-line blockquote) to eliminate the MD028
warning while preserving the caution content and wording.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c5486323-1229-4601-9af1-cb36c5c3bcc6
📒 Files selected for processing (6)
shortcuts/drive/drive_task_result_wiki.goshortcuts/drive/drive_task_result_wiki_test.goshortcuts/wiki/wiki_move.goshortcuts/wiki/wiki_move_test.goskills/lark-wiki/SKILL.mdskills/lark-wiki/references/lark-wiki-move.md
✅ Files skipped from review due to trivial changes (1)
- shortcuts/wiki/wiki_move_test.go
9b30bb4 to
989fa22
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
shortcuts/drive/drive_task_result.go (1)
54-67: Consider consolidating duplicate validation logic fortask_checkandwiki_move.Both cases perform identical
--task-idrequired checks with the samevalidate.ResourceNamecall. This could be combined.♻️ Optional: Combine task-id validation
- case "task_check": - if runtime.Str("task-id") == "" { - return output.ErrValidation("--task-id is required for task_check scenario") - } - if err := validate.ResourceName(runtime.Str("task-id"), "--task-id"); err != nil { - return output.ErrValidation("%s", err) - } - case "wiki_move": - if runtime.Str("task-id") == "" { - return output.ErrValidation("--task-id is required for wiki_move scenario") - } - if err := validate.ResourceName(runtime.Str("task-id"), "--task-id"); err != nil { - return output.ErrValidation("%s", err) - } + case "task_check", "wiki_move": + if runtime.Str("task-id") == "" { + return output.ErrValidation("--task-id is required for %s scenario", scenario) + } + if err := validate.ResourceName(runtime.Str("task-id"), "--task-id"); err != nil { + return output.ErrValidation("%s", err) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_task_result.go` around lines 54 - 67, The duplicated validation for the "--task-id" flag in the "task_check" and "wiki_move" switch branches should be consolidated: extract the check into a single helper (e.g., validateTaskIDOrReturnError) that calls runtime.Str("task-id"), returns output.ErrValidation when empty, and invokes validate.ResourceName(runtime.Str("task-id"), "--task-id") and returns any error wrapped via output.ErrValidation("%s", err); then replace the duplicated blocks in the "task_check" and "wiki_move" cases with a single call to that helper (or run the helper once before the switch when the scenario is one of {"task_check","wiki_move"}).shortcuts/wiki/wiki_move.go (1)
125-189: Note: Status types are duplicated between wiki and drive packages.
wikiMoveTaskStatusandwikiMoveTaskResulthere are similar towikiMoveTaskQueryStatusandwikiMoveTaskResultStatusinshortcuts/drive/drive_task_result_wiki.go. The duplication exists because:
- Wiki package uses typed
*wikiNodeRecord- Drive package uses
map[string]interface{}for node representationThis is acceptable given the different typing requirements, but consider extracting shared logic if the codebase grows more wiki-related shortcuts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/wiki/wiki_move.go` around lines 125 - 189, There is duplication of status logic between wikiMoveTaskStatus/wikiMoveTaskResult and the drive equivalents (wikiMoveTaskQueryStatus/wikiMoveTaskResultStatus); extract the shared behavior by creating a small shared helper (e.g., in a shortcuts/common package) that implements Ready, Failed, Pending, FirstResult, PrimaryStatusCode and PrimaryStatusLabel as functions operating on a generic/slice-based result shape or an interface, then have wikiMoveTaskStatus and wikiMoveTaskQueryStatus call those helpers (keeping wikiMoveTaskResult as the typed struct for wiki and the map-based struct for drive) so you remove duplicated logic while preserving per-package types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/drive/drive_task_result.go`:
- Around line 54-67: The duplicated validation for the "--task-id" flag in the
"task_check" and "wiki_move" switch branches should be consolidated: extract the
check into a single helper (e.g., validateTaskIDOrReturnError) that calls
runtime.Str("task-id"), returns output.ErrValidation when empty, and invokes
validate.ResourceName(runtime.Str("task-id"), "--task-id") and returns any error
wrapped via output.ErrValidation("%s", err); then replace the duplicated blocks
in the "task_check" and "wiki_move" cases with a single call to that helper (or
run the helper once before the switch when the scenario is one of
{"task_check","wiki_move"}).
In `@shortcuts/wiki/wiki_move.go`:
- Around line 125-189: There is duplication of status logic between
wikiMoveTaskStatus/wikiMoveTaskResult and the drive equivalents
(wikiMoveTaskQueryStatus/wikiMoveTaskResultStatus); extract the shared behavior
by creating a small shared helper (e.g., in a shortcuts/common package) that
implements Ready, Failed, Pending, FirstResult, PrimaryStatusCode and
PrimaryStatusLabel as functions operating on a generic/slice-based result shape
or an interface, then have wikiMoveTaskStatus and wikiMoveTaskQueryStatus call
those helpers (keeping wikiMoveTaskResult as the typed struct for wiki and the
map-based struct for drive) so you remove duplicated logic while preserving
per-package types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7ea1d83-3a8a-438c-8bd6-c4d5d9cb4023
📒 Files selected for processing (12)
shortcuts/drive/drive_task_result.goshortcuts/drive/drive_task_result_test.goshortcuts/drive/drive_task_result_wiki.goshortcuts/drive/drive_task_result_wiki_test.goshortcuts/wiki/shortcuts.goshortcuts/wiki/wiki_move.goshortcuts/wiki/wiki_move_test.goshortcuts/wiki/wiki_node_create_test.goskills/lark-drive/references/lark-drive-move.mdskills/lark-drive/references/lark-drive-task-result.mdskills/lark-wiki/SKILL.mdskills/lark-wiki/references/lark-wiki-move.md
✅ Files skipped from review due to trivial changes (2)
- skills/lark-drive/references/lark-drive-move.md
- shortcuts/wiki/wiki_move_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- shortcuts/wiki/shortcuts.go
- shortcuts/drive/drive_task_result_test.go
- skills/lark-wiki/SKILL.md
- shortcuts/wiki/wiki_node_create_test.go
989fa22 to
373e6bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/drive/drive_task_result.go`:
- Around line 290-318: PrimaryStatusCode() and PrimaryStatusLabel() must not
just use MoveResults[0]; instead scan s.MoveResults to pick an aggregate
"primary" result: iterate MoveResults and if any entry has Status < 0 return
that Status (and use its StatusMsg for PrimaryStatusLabel); otherwise return the
first entry with Status > 0 (use its StatusMsg if non-empty); if none found fall
back to the Ready()/Failed() switch and default values. Update
PrimaryStatusCode() and PrimaryStatusLabel() to implement this aggregation logic
(reference types/functions: wikiMoveTaskQueryStatus, MoveResults, FirstResult,
PrimaryStatusCode, PrimaryStatusLabel, Ready(), Failed()).
- Around line 211-225: The helper validateDriveTaskResultScopes currently
swallows context cancellation errors from
runtime.Factory.Credential.ResolveToken; update the function to detect and
immediately return context.Canceled and context.DeadlineExceeded (use
errors.Is(err, context.Canceled) and errors.Is(err, context.DeadlineExceeded))
instead of treating them as non-errors, so that cancellation/timeout from
ResolveToken is preserved and execution does not continue to requireDriveScopes;
keep the existing behavior for non-context errors and for nil/empty
result.Scopes.
In `@shortcuts/wiki/wiki_move.go`:
- Around line 161-189: PrimaryStatusCode()/PrimaryStatusLabel() currently only
reflect the first move result causing later failures to be masked; update them
to aggregate across s.MoveResults: if s.Failed() is true, locate the first (or
most relevant) wikiMoveTaskResult in s.MoveResults with a failing Status and
return its Status for PrimaryStatusCode and its non-empty trimmed StatusMsg for
PrimaryStatusLabel; otherwise fall back to FirstResult() as before and then to
the Ready()/Failed()/processing switch. Reference: functions FirstResult,
PrimaryStatusCode, PrimaryStatusLabel, Failed(), Ready(), and the
wikiMoveTaskResult fields Status and StatusMsg.
- Around line 62-65: The Validate hook (Validate func) currently reads the spec
via readWikiMoveSpec and calls validateWikiMoveSpec but doesn't block bot
invocations that target the personal alias; update Validate to reject requests
where the runtime indicates a bot caller and the parsed spec's target space
equals "my_library": after spec := readWikiMoveSpec(runtime) check the runtime
identity (e.g., runtime indicates bot/`--as bot`) and if spec.TargetSpaceID ==
"my_library" return a validation error instead of calling validateWikiMoveSpec
so bot calls are blocked early.
🪄 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: e6695b56-68cf-4576-bd71-238180a9c41a
📒 Files selected for processing (10)
shortcuts/drive/drive_task_result.goshortcuts/drive/drive_task_result_test.goshortcuts/wiki/shortcuts.goshortcuts/wiki/wiki_move.goshortcuts/wiki/wiki_move_test.goshortcuts/wiki/wiki_node_create_test.goskills/lark-drive/references/lark-drive-move.mdskills/lark-drive/references/lark-drive-task-result.mdskills/lark-wiki/SKILL.mdskills/lark-wiki/references/lark-wiki-move.md
✅ Files skipped from review due to trivial changes (3)
- shortcuts/wiki/shortcuts.go
- skills/lark-wiki/SKILL.md
- shortcuts/wiki/wiki_move_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- shortcuts/wiki/wiki_node_create_test.go
- skills/lark-drive/references/lark-drive-task-result.md
- skills/lark-drive/references/lark-drive-move.md
Change-Id: Ie449390e7dcf745405479071f0694c7e37ad7a45
373e6bf to
afcdc43
Compare
…#436) Change-Id: I58400054e6c3c3c8e7b0cf72b874602b22fa287d
Summary
Add a new
wiki +moveshortcut to support moving existing wiki nodes and migrating Drive documents into Wiki. This also adds bounded async task polling, exposeswiki_moveviadrive +task_result, and documents how "My Document Library" /my_libraryshould resolve to the personal Wiki library instead of Drive root.Changes
wiki +movefor:drive +task_resultcommand when polling times outdrive +task_resultwith--scenario wiki_moveand normalized wiki task result outputmy_libraryand “My Document Library” should be treated as the personal Wiki library targetTest Plan
lark xxxcommand works as expectedCommands run:
go test ./shortcuts/wiki ./shortcuts/drivemake unit-testManual verification:
./lark-cli wiki spaces get --as user --params '{"space_id":"my_library"}'./lark-cli wiki +moveRelated Issues
Summary by CodeRabbit
New Features
wiki +movecommand to move existing Wiki nodes or migrate Drive documents into Wikidrive +task_resultwithwiki_movescenario to track asynchronous Wiki move operationsDocumentation
wiki +movefromdrive +movewith target semantic constraints and usage examples