feat(wiki): add +space-list, +node-list, +node-copy shortcuts#392
Open
herbertliu wants to merge 5 commits intomainfrom
Open
feat(wiki): add +space-list, +node-list, +node-copy shortcuts#392herbertliu wants to merge 5 commits intomainfrom
herbertliu wants to merge 5 commits intomainfrom
Conversation
github-actions
bot
added
domain/ccm
|
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:
📝 WalkthroughWalkthroughAdds three new wiki CLI shortcuts— Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI
participant API as "Lark API\n(open-apis/wiki)"
User->>CLI: run `wiki +space-list` or `wiki +node-list --space-id`
CLI->>API: GET /open-apis/wiki/v2/spaces[?page_token] or /spaces/{space_id}/nodes[?parent_node_token,page_token]
API-->>CLI: {items, has_more, next_page_token}
alt has_more == true
CLI->>API: GET ... (page_token=next_page_token)
API-->>CLI: next page
end
CLI->>CLI: normalize & aggregate items
CLI-->>User: print total and result payload
User->>CLI: run `wiki +node-copy --space-id X --node-token Y --target-space-id Z`
CLI->>CLI: validate flags (require target-space-id or target-parent-node-token)
CLI->>API: POST /open-apis/wiki/v2/spaces/X/nodes/Y/copy {target_space_id/target_parent_token,title}
API-->>CLI: {node (copied) details}
CLI-->>User: progress on stderr and returned node metadata on stdout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
Comment |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@ebd2136a2daff28dbaae8b60fe11501f25d786db🧩 Skill updatenpx skills add larksuite/cli#feat/wiki-organize-migrate -y -g |
Greptile SummaryThis PR adds three new wiki shortcuts (
Confidence Score: 5/5Safe to merge — all P1 findings from prior rounds are resolved; remaining findings are P2 style/test suggestions. All previously flagged issues (EncodePathSegment in DryRun, missing source-flag validation, unreachable guard) are fixed. The two remaining observations are a missing multi-page pagination test and an undocumented +node-move shortcut — both P2 and non-blocking. shortcuts/wiki/wiki_list_copy_test.go — pagination loop has no multi-page coverage. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([User invokes shortcut]) --> B{Which shortcut?}
B -->|+space-list| C[Validate: no flags]
B -->|+node-list| D[Validate space-id + parent-node-token]
B -->|+node-copy| E[Validate source flags + at-least-one-target]
B -->|+node-move| F[Validate source flags + at-least-one-target]
C --> G[GET /wiki/v2/spaces]
D --> H[GET /wiki/v2/spaces/spaceID/nodes]
E --> I[POST /wiki/v2/spaces/spaceID/nodes/nodeToken/copy]
F --> J[POST /wiki/v2/spaces/spaceID/nodes/nodeToken/move]
G -->|has_more=true| G
G -->|has_more=false| K[Output spaces + total]
H -->|has_more=true| H
H -->|has_more=false| L[Output nodes + total]
I --> M[Output copied node record]
J --> N[Output moved node record]
Reviews (7): Last reviewed commit: "feat(wiki): add +node-move shortcut" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
shortcuts/wiki/wiki_node_create_test.go (1)
100-105: Avoid asserting that+node-createmust stay at index 0.
Shortcuts()now returns multiple wiki shortcuts, and registration does not depend on their order. Keeping the test index-based makes harmless reordering fail the suite for no behavioral reason.♻️ Suggested test refactor
shortcuts := Shortcuts() - if len(shortcuts) == 0 { - t.Fatalf("len(Shortcuts()) = 0, want at least 1") - } - if shortcuts[0].Command != "+node-create" { - t.Fatalf("shortcut command = %q, want %q", shortcuts[0].Command, "+node-create") - } + found := false + for _, shortcut := range shortcuts { + if shortcut.Command == "+node-create" { + found = true + break + } + } + if !found { + t.Fatalf("Shortcuts() does not include %q", "+node-create") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/wiki/wiki_node_create_test.go` around lines 100 - 105, The test currently assumes Shortcuts() returns "+node-create" at index 0; instead iterate/search the returned slice (shortcuts := Shortcuts()) for an element whose Command field equals "+node-create" and assert that such an element exists (fail the test if not found). Update the assertion to check presence (e.g., loop over shortcuts and break when Command == "+node-create") rather than indexing shortcuts[0], keeping references to Shortcuts() and the Command field to locate the code to change.shortcuts/wiki/wiki_list_copy_test.go (1)
31-58:TestWikiSpaceListReturnsPaginatedSpacescurrently validates only one page.Line 42 sets
has_more: false, so pagination logic (page_tokenchaining) is not exercised. Consider adding a two-page fixture to verify continuation and aggregation behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/wiki/wiki_list_copy_test.go` around lines 31 - 58, The test TestWikiSpaceListReturnsPaginatedSpaces only returns a single page (has_more: false), so update the HTTP stubs for the GET "/open-apis/wiki/v2/spaces" used in that test to simulate two pages: register a first stub response with "has_more": true, a "next_page_token" (e.g., "token1") and one or more items, and register a second stub that expects the query/page_token value ("token1") and returns "has_more": false with the remaining items; ensure the test triggers the pagination path (so the code under test requests the second page) and then assert that the final result aggregates items from both pages (verifying both space_1 and space_2 are present).shortcuts/wiki/wiki_space_list.go (1)
27-51: Add a defensive guard for repeated pagination tokens.If the API ever returns
has_more=truewith the same non-emptypage_token, this loop can become unbounded. Add a seen-token guard (or max-page cap) to fail fast.Proposed hardening diff
Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { var spaces []map[string]interface{} pageToken := "" + seenTokens := map[string]struct{}{} for { params := map[string]interface{}{"page_size": 50} if pageToken != "" { params["page_token"] = pageToken } data, err := runtime.CallAPI("GET", "/open-apis/wiki/v2/spaces", params, nil) if err != nil { return err } @@ next, _ := data["page_token"].(string) hasMore, _ := data["has_more"].(bool) if !hasMore || next == "" { break } + if _, exists := seenTokens[next]; exists { + return fmt.Errorf("wiki space list pagination token repeated: %q", next) + } + seenTokens[next] = struct{}{} pageToken = next }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/wiki/wiki_space_list.go` around lines 27 - 51, The pagination loop in Execute (in wiki_space_list.go) can hang if the API returns has_more=true with the same non-empty page_token; add a defensive guard by tracking seen page tokens (e.g., a map[string]struct{} seenTokens) and/or a maxPageCount counter, check before setting pageToken or looping, and if you detect a repeated token or exceed max pages return a clear error; update the loop around runtime.CallAPI and the pageToken assignment/next handling to consult seenTokens (or increment/check maxPages) so the function fails fast instead of looping forever.
🤖 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/wiki/wiki_node_copy.go`:
- Around line 35-45: The Validate function currently only validates
targetSpaceID and targetParent; also trim and validate the source identifiers
coming from runtime.Str("space-id") and runtime.Str("node-token") inside the
same Validate block and return any validation errors. Specifically, inside
Validate (in wiki_node_copy.go) add sourceSpaceID :=
strings.TrimSpace(runtime.Str("space-id")) and sourceNodeToken :=
strings.TrimSpace(runtime.Str("node-token")) and call
validateOptionalResourceName(sourceSpaceID, "--space-id") and
validateOptionalResourceName(sourceNodeToken, "--node-token") (returning any
error) before returning; keep the existing target presence check and existing
calls for target flags.
In `@skills/lark-wiki/references/lark-wiki-node-copy.md`:
- Around line 7-15: Update the usage line for the lark-cli wiki +node-copy
example to reflect that the command accepts either --target-space-id or
--target-parent-node-token (not always both); modify the flags section for
--target-space-id and --target-parent-node-token so they are shown as an
either/or choice (e.g., "[--target-space-id <target_space_id> |
--target-parent-node-token <token>]" or a short note stating "one of
--target-space-id or --target-parent-node-token is required") to match the
actual CLI behavior and avoid implying --target-space-id is always required.
In `@skills/lark-wiki/references/lark-wiki-space-list.md`:
- Around line 35-39: Update the Notes bullet that currently reads "Returns all
spaces in a single call via auto-pagination" to avoid implying a single network
request; instead state that results are auto-paginated and may require multiple
API requests under the hood (which can affect latency/rate limits). Edit the
line in skills/lark-wiki/references/lark-wiki-space-list.md referring to
auto-pagination (the bullet mentioning "auto-pagination") to mention multiple
requests and recommend users consider pagination/ rate-limit implications when
troubleshooting or scripting with space_id/my_library.
---
Nitpick comments:
In `@shortcuts/wiki/wiki_list_copy_test.go`:
- Around line 31-58: The test TestWikiSpaceListReturnsPaginatedSpaces only
returns a single page (has_more: false), so update the HTTP stubs for the GET
"/open-apis/wiki/v2/spaces" used in that test to simulate two pages: register a
first stub response with "has_more": true, a "next_page_token" (e.g., "token1")
and one or more items, and register a second stub that expects the
query/page_token value ("token1") and returns "has_more": false with the
remaining items; ensure the test triggers the pagination path (so the code under
test requests the second page) and then assert that the final result aggregates
items from both pages (verifying both space_1 and space_2 are present).
In `@shortcuts/wiki/wiki_node_create_test.go`:
- Around line 100-105: The test currently assumes Shortcuts() returns
"+node-create" at index 0; instead iterate/search the returned slice (shortcuts
:= Shortcuts()) for an element whose Command field equals "+node-create" and
assert that such an element exists (fail the test if not found). Update the
assertion to check presence (e.g., loop over shortcuts and break when Command ==
"+node-create") rather than indexing shortcuts[0], keeping references to
Shortcuts() and the Command field to locate the code to change.
In `@shortcuts/wiki/wiki_space_list.go`:
- Around line 27-51: The pagination loop in Execute (in wiki_space_list.go) can
hang if the API returns has_more=true with the same non-empty page_token; add a
defensive guard by tracking seen page tokens (e.g., a map[string]struct{}
seenTokens) and/or a maxPageCount counter, check before setting pageToken or
looping, and if you detect a repeated token or exceed max pages return a clear
error; update the loop around runtime.CallAPI and the pageToken assignment/next
handling to consult seenTokens (or increment/check maxPages) so the function
fails fast instead of looping forever.
🪄 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: cb66315a-c1bf-435a-8ff8-30d83a393cc9
📒 Files selected for processing (10)
shortcuts/wiki/shortcuts.goshortcuts/wiki/wiki_list_copy_test.goshortcuts/wiki/wiki_node_copy.goshortcuts/wiki/wiki_node_create_test.goshortcuts/wiki/wiki_node_list.goshortcuts/wiki/wiki_space_list.goskills/lark-wiki/SKILL.mdskills/lark-wiki/references/lark-wiki-node-copy.mdskills/lark-wiki/references/lark-wiki-node-list.mdskills/lark-wiki/references/lark-wiki-space-list.md
herbertliu
added a commit
that referenced
this pull request
Apr 10, 2026
…arget - .gitleaks.toml: allowlist skills/*/references/*.md so placeholder tokens in documentation examples don't trigger generic-api-key rule - Makefile: add `make gitleaks` target for local pre-push scanning This prevents the false positive seen in PR #392 where example node_token values in lark-wiki reference docs triggered the gitleaks CI check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Line 11: The .PHONY declaration is missing the standard "all" target which
triggers minphony; update the .PHONY line to include all (i.e., add "all"
alongside build, vet, test, unit-test, integration-test, install, uninstall,
clean, fetch_meta, gitleaks) and ensure an "all" target exists as the default
entrypoint so make's default target is covered by .PHONY (look for the .PHONY
line and the "all" target in the Makefile).
🪄 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: d81be34f-e729-4e18-81d8-8c02dd89c213
📒 Files selected for processing (2)
.gitleaks.tomlMakefile
✅ Files skipped from review due to trivial changes (1)
- .gitleaks.toml
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Makefile (1)
11-11:⚠️ Potential issue | 🟡 Minor
allis still missing from.PHONY.Line 11 still omits
all, so thecheckmakeminphonywarning remains unresolved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 11, The .PHONY declaration is missing the all target which triggers the checkmake minphony warning; update the .PHONY line (the entry listing build vet test unit-test integration-test install uninstall clean fetch_meta gitleaks) to also include all so that the all target is treated as phony and the checkmake warning is resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check-doc-tokens.sh`:
- Around line 24-34: REALISTIC_TOKEN_RE currently only matches double-quoted
tokens and omits some documented prefixes, causing false negatives; update the
REALISTIC_TOKEN_RE to include the missing prefixes (docx, ou_, cli_) and broaden
the quoting capture to allow backtick and single-quote spans as well as double
quotes (use a character class for quotes/backtick or optional backtick group),
and ensure PLACEHOLDER_RE still excludes obvious fake tokens (keep
PLACEHOLDER_RE unchanged or adjust if needed); locate and modify the
REALISTIC_TOKEN_RE variable in the script to implement these changes so token
detection matches the documented prefixes and backtick spans.
---
Duplicate comments:
In `@Makefile`:
- Line 11: The .PHONY declaration is missing the all target which triggers the
checkmake minphony warning; update the .PHONY line (the entry listing build vet
test unit-test integration-test install uninstall clean fetch_meta gitleaks) to
also include all so that the all target is treated as phony and the checkmake
warning is resolved.
🪄 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: d707845b-21fd-439e-91a1-c116c79db935
📒 Files selected for processing (4)
.gitleaks.tomlMakefilescripts/check-doc-tokens.shskills/lark-wiki/references/lark-wiki-node-copy.md
✅ Files skipped from review due to trivial changes (2)
- .gitleaks.toml
- skills/lark-wiki/references/lark-wiki-node-copy.md
Implement three new wiki shortcuts for organizing and migrating wiki content: - `+space-list`: list all accessible wiki spaces with auto-pagination - `+node-list`: list nodes under a space or parent node with auto-pagination - `+node-copy`: copy a wiki node (and subtree) to a target space or parent node Also includes: - Reference docs under skills/lark-wiki/references/ (with _EXAMPLE_TOKEN placeholders) - Updated skills/lark-wiki/SKILL.md with new shortcuts table entries - 9 unit tests covering registration, pagination, validation, and copy scenarios - scripts/check-doc-tokens.sh: pre-push check that catches realistic-looking example tokens in reference docs and prompts use of _EXAMPLE_TOKEN placeholders - Makefile: add `make gitleaks` target (runs check-doc-tokens then gitleaks) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
herbertliu
force-pushed
the
feat/wiki-organize-migrate
branch
from
cb1547f to
427736b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Makefile (1)
11-11:⚠️ Potential issue | 🟡 MinorAdd the missing
allphony/default target (Line 11).
checkmake’sminphonywarning is still valid:allis missing from.PHONY, and there is noall:target.Suggested fix
-.PHONY: build vet test unit-test integration-test install uninstall clean fetch_meta gitleaks +.PHONY: all build vet test unit-test integration-test install uninstall clean fetch_meta gitleaks + +all: test🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 11, The Makefile is missing the default/phony target "all"; add "all" to the .PHONY list (alongside build, vet, test, etc.) and implement an all: target that invokes the intended default action (e.g., depends on build or runs the primary build/install steps). Update the .PHONY declaration to include "all" and add a simple all: rule that delegates to the existing build target (or the appropriate default target name) so make has a valid default/phony entry.
🧹 Nitpick comments (1)
shortcuts/wiki/wiki_list_copy_test.go (1)
31-58: Pagination code path is not exercised in the pagination-focused tests.Both list tests stub a single page (
has_more: false), so automatic pagination behavior is not validated.Suggested test shape (example for
+space-list)+reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/wiki/v2/spaces", + Body: map[string]interface{}{ + "code": 0, + "data": map[string]interface{}{ + "has_more": true, + "page_token": "p2", + "items": []interface{}{ ...page1... }, + }, + }, +}) +reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/wiki/v2/spaces?page_token=p2", + Body: map[string]interface{}{ + "code": 0, + "data": map[string]interface{}{ + "has_more": false, + "items": []interface{}{ ...page2... }, + }, + }, +})Also applies to: 98-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/wiki/wiki_list_copy_test.go` around lines 31 - 58, Update the test TestWikiSpaceListReturnsPaginatedSpaces to exercise the pagination path by registering two HTTP stubs for the "/open-apis/wiki/v2/spaces" endpoint: the first stub should return "has_more": true and include a "page_token" or "next_page_token" value in its data (and the page of items you want in page 1), and the second stub should expect that token as a query param (or in the request body) and return "has_more": false with the remaining items; ensure both stubs are registered via reg.Register and keep the same response shape (code/msg/data) so the list-pagination logic in the code under test (invoked by TestWikiSpaceListReturnsPaginatedSpaces) will follow the continuation token path and aggregate items across pages; apply the same two-page stub pattern to the other wiki list test referenced in the comment.
🤖 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/wiki/wiki_list_copy_test.go`:
- Around line 173-176: The test TestWikiNodeListPassesParentNodeToken currently
only checks response body; update it to assert that the outgoing request
actually carried the parent-node-token (e.g., as a header or query param
depending on implementation). Modify the httpmock.Stub setup (the stub variable
in TestWikiNodeListPassesParentNodeToken and the similar block at lines 198-223)
to capture the incoming request (use the stub's request inspector/callback or
inspect httpmock.ReceivedRequests) and add an assertion that the request
contains the expected parent-node-token value (matching the token passed to the
command), failing the test if the token is absent or different.
---
Duplicate comments:
In `@Makefile`:
- Line 11: The Makefile is missing the default/phony target "all"; add "all" to
the .PHONY list (alongside build, vet, test, etc.) and implement an all: target
that invokes the intended default action (e.g., depends on build or runs the
primary build/install steps). Update the .PHONY declaration to include "all" and
add a simple all: rule that delegates to the existing build target (or the
appropriate default target name) so make has a valid default/phony entry.
---
Nitpick comments:
In `@shortcuts/wiki/wiki_list_copy_test.go`:
- Around line 31-58: Update the test TestWikiSpaceListReturnsPaginatedSpaces to
exercise the pagination path by registering two HTTP stubs for the
"/open-apis/wiki/v2/spaces" endpoint: the first stub should return "has_more":
true and include a "page_token" or "next_page_token" value in its data (and the
page of items you want in page 1), and the second stub should expect that token
as a query param (or in the request body) and return "has_more": false with the
remaining items; ensure both stubs are registered via reg.Register and keep the
same response shape (code/msg/data) so the list-pagination logic in the code
under test (invoked by TestWikiSpaceListReturnsPaginatedSpaces) will follow the
continuation token path and aggregate items across pages; apply the same
two-page stub pattern to the other wiki list test referenced in the comment.
🪄 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: e8ecebb9-d068-4c95-bd20-8362b1fd7220
📒 Files selected for processing (13)
.gitleaks.tomlMakefilescripts/check-doc-tokens.shshortcuts/wiki/shortcuts.goshortcuts/wiki/wiki_list_copy_test.goshortcuts/wiki/wiki_node_copy.goshortcuts/wiki/wiki_node_create_test.goshortcuts/wiki/wiki_node_list.goshortcuts/wiki/wiki_space_list.goskills/lark-wiki/SKILL.mdskills/lark-wiki/references/lark-wiki-node-copy.mdskills/lark-wiki/references/lark-wiki-node-list.mdskills/lark-wiki/references/lark-wiki-space-list.md
✅ Files skipped from review due to trivial changes (8)
- .gitleaks.toml
- shortcuts/wiki/wiki_node_create_test.go
- shortcuts/wiki/shortcuts.go
- skills/lark-wiki/SKILL.md
- skills/lark-wiki/references/lark-wiki-node-list.md
- skills/lark-wiki/references/lark-wiki-space-list.md
- skills/lark-wiki/references/lark-wiki-node-copy.md
- shortcuts/wiki/wiki_node_copy.go
🚧 Files skipped from review as they are similar to previous changes (3)
- scripts/check-doc-tokens.sh
- shortcuts/wiki/wiki_space_list.go
- shortcuts/wiki/wiki_node_list.go
- wiki_node_list.go: use EncodePathSegment in DryRun URL; remove dead `if spaceID == ""` guard (cobra enforces Required: true before Execute) - wiki_node_copy.go: use EncodePathSegment in DryRun URL; add Validate checks for --space-id and --node-token (not just target flags) - wiki_list_copy_test.go: assert parent_node_token is forwarded in the outgoing request URL (not just in the response body) - check-doc-tokens.sh: extend regex to cover docx*, ou_, cli_ prefixes and backtick-quoted spans - lark-wiki-node-copy.md: clarify usage — target flags are mutual-exclusive (one of --target-space-id | --target-parent-node-token) - lark-wiki-space-list.md: fix misleading "single call" wording - Makefile: add `all` phony target Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
|
Thanks for adding the new wiki shortcuts, the coverage is moving in a good direction. I found a few issues that look worth fixing before merge though: |
| } | ||
| if err := validateOptionalResourceName(strings.TrimSpace(runtime.Str("node-token")), "--node-token"); err != nil { | ||
| return err | ||
| } |
Collaborator
There was a problem hiding this comment.
+node-copy is described as “target space or parent node”, but the validation only checks “at least one” and buildNodeCopyBody() will send both fields if both flags are provided. That creates an ambiguous request shape and can easily diverge from the documented behavior. I think we should reject the case where --target-space-id and --target-parent-node-token are both set, and add a test for that explicitly.
| Description: "List wiki nodes in a space or under a parent node", | ||
| Risk: "read", | ||
| Scopes: []string{"wiki:node:retrieve"}, | ||
| AuthTypes: []string{"user", "bot"}, |
Collaborator
There was a problem hiding this comment.
This shortcut says --space-id my_library is supported, but unlike +node-create it never resolves my_library to a real space_id, and it also does not reject that value for bot identity. As written, we will call /wiki/v2/spaces/my_library/nodes directly, which looks inconsistent with the existing wiki create flow and is likely to break for at least some callers. Can we reuse the same my_library resolution / bot-validation logic here so the behavior matches +node-create?
| # | ||
| # Excluded (clearly fake): | ||
| # - Values ending with EXAMPLE_TOKEN (e.g. wikcn_EXAMPLE_TOKEN) | ||
| # - Values that are all uppercase X (e.g. bascnXXXXXXXX) |
Collaborator
There was a problem hiding this comment.
I think this pattern/filter is too aggressive for the current docs set. Running the script on the repo already flags existing placeholder-ish examples like ou_manager / ou_director, so the new make gitleaks target fails even before this PR’s docs are considered. Could we narrow the detection or broaden the placeholder allowlist first, otherwise contributors will get a broken pre-push/local check immediately?
|
|
||
| ```bash | ||
| # 1. List nodes in the source space | ||
| lark-cli wiki +node-list --space-id source_space_id |
Collaborator
There was a problem hiding this comment.
This example uses a token-like value (wikcnSource) without an explicit placeholder suffix, and the new checker does not catch it because it only matches quoted/backticked forms. That means the PR introduces exactly the kind of “realistic-looking sample” the script is supposed to prevent. I’d switch this to something clearly fake like wikcn_EXAMPLE_TOKEN (or <source_node_token>) and consider teaching the checker to catch bare code-block values too.
Implements wiki node move using the native POST .../nodes/{token}/move API.
Unlike +node-copy, the node is removed from its original location.
- Flags: --space-id, --node-token, --target-space-id | --target-parent-node-token
- Scope: wiki:node:move
- 3 unit tests covering: target validation, move to space, move to parent
- Reference doc: skills/lark-wiki/references/lark-wiki-node-move.md
Also:
- check-doc-tokens.sh: require at least one digit in token suffix to avoid
false positives on plain-word fake names (e.g. ou_manager, ou_director)
- Fix pre-existing realistic example tokens in lark-minutes and lark-doc
reference docs (found by the updated script)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 participants
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
+space-list: List all wiki spaces accessible to the caller (auto-pagination)+node-list: List nodes under a space or parent node with auto-pagination; supports--space-id+ optional--parent-node-token+node-copy: Copy a wiki node (and its subtree) to a target space or parent node; requires--target-space-idor--target-parent-node-tokenAlso includes:
skills/lark-wiki/references/skills/lark-wiki/SKILL.mdwith new shortcuts table entriesMotivation
Enables wiki content organization and cross-space migration workflows directly from lark-cli, without requiring custom scripts or manual API calls.
Test plan
go test ./shortcuts/wiki/...— all tests passgo vet ./...— no issuesgolangci-lint run— 0 issues+space-list(199 spaces),+node-list(184 nodes),+node-copy(successful copy)node_tokenvalues in docs with clearly fake_EXAMPLE_TOKENplaceholders🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores