Skip to content

feat(wiki): add +space-list, +node-list, +node-copy shortcuts#392

Open
herbertliu wants to merge 9 commits intomainfrom
feat/wiki-organize-migrate
Open

feat(wiki): add +space-list, +node-list, +node-copy shortcuts#392
herbertliu wants to merge 9 commits intomainfrom
feat/wiki-organize-migrate

Conversation

@herbertliu
Copy link
Copy Markdown
Collaborator

@herbertliu herbertliu commented Apr 10, 2026

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-id or --target-parent-node-token

Also includes:

  • Reference docs for each shortcut under skills/lark-wiki/references/
  • Updated skills/lark-wiki/SKILL.md with new shortcuts table entries
  • 9 unit tests covering registration, pagination, validation, and copy scenarios

Motivation

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 pass
  • go vet ./... — no issues
  • golangci-lint run — 0 issues
  • Real-world tested against a live wiki space: +space-list (199 spaces), +node-list (184 nodes), +node-copy (successful copy)
  • Fixed gitleaks false positive — replaced example node_token values in docs with clearly fake _EXAMPLE_TOKEN placeholders

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added wiki commands: +space-list, +node-list, +node-copy and exposed them in shortcuts.
  • Documentation

    • Added reference pages and skill docs for the three commands with usage, flags, examples, and responses.
  • Tests

    • Added tests covering space/node listing (pagination and parent scoping) and node-copy validation and execution.
  • Chores

    • Added a gitleaks target and a documentation token checker script to CI/tooling.

@github-actions github-actions bot added domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths labels Apr 10, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 10, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds three new wiki CLI shortcuts—+space-list, +node-list, and +node-copy—with flag validation, paginated API interactions, DryRun and Execute implementations, tests, documentation, and registry update to include them alongside +node-create.

Changes

Cohort / File(s) Summary
Shortcut Registry
shortcuts/wiki/shortcuts.go
Shortcuts() now returns WikiNodeCreate, WikiSpaceList, WikiNodeList, and WikiNodeCopy (expanded registry).
Core Shortcut Implementations
shortcuts/wiki/wiki_space_list.go, shortcuts/wiki/wiki_node_list.go, shortcuts/wiki/wiki_node_copy.go
New shortcuts: WikiSpaceList and WikiNodeList perform paginated GETs to /open-apis/wiki/v2/spaces and /open-apis/wiki/v2/spaces/{space_id}/nodes; WikiNodeCopy performs POST to /open-apis/wiki/v2/spaces/{space_id}/nodes/{node_token}/copy with destination validation, optional title, and DryRun/Execute flows.
Tests
shortcuts/wiki/wiki_list_copy_test.go, shortcuts/wiki/wiki_node_create_test.go
Added tests for space/node listing and node copy (validation, pagination, request body capture, stdout/stderr assertions). Relaxed node-create test to allow additional shortcuts.
Documentation / Skill Manifests
skills/lark-wiki/SKILL.md, skills/lark-wiki/references/lark-wiki-space-list.md, skills/lark-wiki/references/lark-wiki-node-list.md, skills/lark-wiki/references/lark-wiki-node-copy.md
Added skill entries and reference pages documenting usage, flags, output formats, pagination, example responses, and required OAuth scopes.
Tooling / Scripts
Makefile, scripts/check-doc-tokens.sh, .gitleaks.toml
Added gitleaks Make target that runs scripts/check-doc-tokens.sh then gitleaks detect; added scripts/check-doc-tokens.sh to scan skill reference docs for token-like literals; minor trailing blank line in .gitleaks.toml.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • fangshuyu-768
  • liangshuo-1

Poem

🐰 I hopped through spaces, nodes in tow,
I paged and listed where wiki winds blow,
I validated flags and copied a tree,
Progress whispered softly to stderr to me,
Resulting tokens tucked in stdout — such glee!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding three new wiki shortcuts (+space-list, +node-list, +node-copy). It matches the primary objective of the changeset.
Description check ✅ Passed The PR description comprehensively covers the required template sections with concrete details about each shortcut, motivation, and extensive test verification across unit tests, linting, and real-world scenarios.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/wiki-organize-migrate

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 10, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@e98a6df11a5b91a435d1e6011419419864150caf

🧩 Skill update

npx skills add larksuite/cli#feat/wiki-organize-migrate -y -g

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 10, 2026

Greptile Summary

This PR adds three new wiki shortcuts (+space-list, +node-list, +node-copy) with auto-pagination, full validate/dry-run/execute cycles, 9 unit tests, reference docs, and a gitleaks Makefile target with a token-scanning helper script. All issues raised in prior review rounds (raw spaceID in DryRun, missing source-flag validation in +node-copy, dead unreachable guard in +node-list) have been resolved.

Confidence Score: 5/5

  • Safe to merge; the sole remaining finding is a P2 documentation inconsistency about the my_library alias in the node-list reference doc.
  • All P0/P1 concerns from prior rounds are resolved. The one remaining finding — that lark-wiki-node-list.md documents my_library as a valid --space-id value while the implementation passes it raw (unlike +node-create which resolves it) — is a P2 documentation inconsistency that does not block the primary feature path.
  • skills/lark-wiki/references/lark-wiki-node-list.md — verify whether the Lark nodes API natively supports the my_library space ID alias before the doc note goes to users.

Important Files Changed

Filename Overview
shortcuts/wiki/wiki_space_list.go Implements +space-list with auto-pagination. Pagination loop guards against missing page_token and correctly exits when has_more is false or page_token is empty. No user-controlled path segments so no EncodePathSegment or Validate needed.
shortcuts/wiki/wiki_node_list.go Implements +node-list with proper Validate, DryRun (using EncodePathSegment), and paginated Execute. Previously raised issues (raw path interpolation in DryRun, dead spaceID == "" guard) are fixed. Minor doc inconsistency: lark-wiki-node-list.md suggests my_library is a valid alias but no resolution code is present.
shortcuts/wiki/wiki_node_copy.go Implements +node-copy with full validate/dry-run/execute cycle. All source and target flags are validated via validateOptionalResourceName, mutually-exclusive target check is correct, and EncodePathSegment is applied in both DryRun and Execute. All previously raised issues are resolved.
shortcuts/wiki/wiki_list_copy_test.go 9 tests covering registration, space-list, node-list (required-flag, single-page, parent-token filtering), and node-copy (validation, space target, parent-node target). Tests using t.Setenv correctly omit t.Parallel() to avoid races; pure-validation tests run in parallel. Good coverage.
scripts/check-doc-tokens.sh New script scanning reference docs for realistic-looking Lark token values. Regex covers major token prefixes (wikcn, doccn, ou_, cli_, etc.) and filters out clearly fake placeholders. Well-commented and uses set -euo pipefail. Minor note: docx[a-z] in the regex would miss docx_-prefixed tokens, but these don't match gitleaks patterns anyway.
skills/lark-wiki/references/lark-wiki-node-list.md The --space-id flag description says "Use my_library for personal document library", but the implementation passes this value directly to the API without resolution (unlike +node-create). The Tips section contradicts this by telling users to run +space-list first. May mislead users if the nodes endpoint does not support the alias.

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI
    participant LarkAPI

    Note over User,LarkAPI: space-list
    User->>CLI: +space-list
    loop Auto-pagination
        CLI->>LarkAPI: GET /open-apis/wiki/v2/spaces
        LarkAPI-->>CLI: items + has_more + page_token
    end
    CLI-->>User: spaces array with total count

    Note over User,LarkAPI: node-list
    User->>CLI: +node-list --space-id spaceID
    loop Auto-pagination
        CLI->>LarkAPI: GET /open-apis/wiki/v2/spaces/spaceID/nodes
        LarkAPI-->>CLI: items + has_more + page_token
    end
    CLI-->>User: nodes array with total count

    Note over User,LarkAPI: node-copy
    User->>CLI: +node-copy --space-id srcSpace --node-token srcNode --target-space-id dstSpace
    CLI->>LarkAPI: POST /open-apis/wiki/v2/spaces/srcSpace/nodes/srcNode/copy
    LarkAPI-->>CLI: copied node record
    CLI-->>User: space_id + node_token + title
Loading

Reviews (10): Last reviewed commit: "fix(wiki): address fangshuyu-768 review ..." | Re-trigger Greptile

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
shortcuts/wiki/wiki_node_create_test.go (1)

100-105: Avoid asserting that +node-create must 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: TestWikiSpaceListReturnsPaginatedSpaces currently validates only one page.

Line 42 sets has_more: false, so pagination logic (page_token chaining) 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=true with the same non-empty page_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

📥 Commits

Reviewing files that changed from the base of the PR and between 76fac11 and ce7849c.

📒 Files selected for processing (10)
  • shortcuts/wiki/shortcuts.go
  • shortcuts/wiki/wiki_list_copy_test.go
  • shortcuts/wiki/wiki_node_copy.go
  • shortcuts/wiki/wiki_node_create_test.go
  • shortcuts/wiki/wiki_node_list.go
  • shortcuts/wiki/wiki_space_list.go
  • skills/lark-wiki/SKILL.md
  • skills/lark-wiki/references/lark-wiki-node-copy.md
  • skills/lark-wiki/references/lark-wiki-node-list.md
  • skills/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>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 14f2371 and 120d920.

📒 Files selected for processing (2)
  • .gitleaks.toml
  • Makefile
✅ Files skipped from review due to trivial changes (1)
  • .gitleaks.toml

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
Makefile (1)

11-11: ⚠️ Potential issue | 🟡 Minor

all is still missing from .PHONY.

Line 11 still omits all, so the checkmake minphony warning 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

📥 Commits

Reviewing files that changed from the base of the PR and between 120d920 and cb1547f.

📒 Files selected for processing (4)
  • .gitleaks.toml
  • Makefile
  • scripts/check-doc-tokens.sh
  • skills/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 herbertliu force-pushed the feat/wiki-organize-migrate branch from cb1547f to 427736b Compare April 10, 2026 08:28
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
Makefile (1)

11-11: ⚠️ Potential issue | 🟡 Minor

Add the missing all phony/default target (Line 11).

checkmake’s minphony warning is still valid: all is missing from .PHONY, and there is no all: 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

📥 Commits

Reviewing files that changed from the base of the PR and between cb1547f and 427736b.

📒 Files selected for processing (13)
  • .gitleaks.toml
  • Makefile
  • scripts/check-doc-tokens.sh
  • shortcuts/wiki/shortcuts.go
  • shortcuts/wiki/wiki_list_copy_test.go
  • shortcuts/wiki/wiki_node_copy.go
  • shortcuts/wiki/wiki_node_create_test.go
  • shortcuts/wiki/wiki_node_list.go
  • shortcuts/wiki/wiki_space_list.go
  • skills/lark-wiki/SKILL.md
  • skills/lark-wiki/references/lark-wiki-node-copy.md
  • skills/lark-wiki/references/lark-wiki-node-list.md
  • skills/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

herbertliu and others added 2 commits April 10, 2026 16:42
- 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>
@fangshuyu-768
Copy link
Copy Markdown
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: +node-copy currently accepts both destination flags at the same time and will send both to the API, +node-list documents/supports my_library without resolving it the way +node-create already does, and the new doc-token check is not reliable yet because it both flags existing placeholder-like values and misses bare token-like examples such as wikcnSource. Once those are tightened up, this PR looks much safer to land.

}
if err := validateOptionalResourceName(strings.TrimSpace(runtime.Str("node-token")), "--node-token"); err != nil {
return err
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e98a6df — added a mutual-exclusion check in Validate: if both --target-space-id and --target-parent-node-token are set, the command now returns "--target-space-id and --target-parent-node-token are mutually exclusive; provide only one". Also added TestWikiNodeCopyRejectsBothTargetFlags to cover this case.

Description: "List wiki nodes in a space or under a parent node",
Risk: "read",
Scopes: []string{"wiki:node:retrieve"},
AuthTypes: []string{"user", "bot"},
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e98a6df — removed the misleading my_library tip from +node-list. The shortcut doesn't resolve the alias (unlike +node-create), so advertising it was incorrect. Flag description and Tips now point users to run +space-list first to discover real space IDs.

#
# Excluded (clearly fake):
# - Values ending with EXAMPLE_TOKEN (e.g. wikcn_EXAMPLE_TOKEN)
# - Values that are all uppercase X (e.g. bascnXXXXXXXX)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ou_manager / ou_director false-positive was already addressed in an earlier commit (e98a6df's predecessor): REALISTIC_TOKEN_RE now requires at least one digit in the token suffix, so pure-letter fake names like ou_manager are excluded. Running bash scripts/check-doc-tokens.sh on the current tree returns ✅ with no errors.


```bash
# 1. List nodes in the source space
lark-cli wiki +node-list --space-id source_space_id
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e98a6df — replaced wikcnSource with wikcn_EXAMPLE_TOKEN in the migration workflow example. Also switched the surrounding --space-id / --target-space-id values to <source_space_id> / <target_space_id> angle-bracket form for consistency.

herbertliu and others added 6 commits April 11, 2026 21:46
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>
- Merge main (abc374f) which adds WikiMove (+move) with async task polling
  and docs-to-wiki support
- Remove our WikiNodeMove (+node-move): superseded by main's richer +move
- Resolve test conflict: keep len(shortcuts) == 0 guard (not != 2) to
  allow the now-larger shortcuts list

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- wiki_node_copy.go: reject mutually exclusive --target-space-id and
  --target-parent-node-token; add test TestWikiNodeCopyRejectsBothTargetFlags
- wiki_node_list.go: remove misleading my_library tip (not resolved by
  this shortcut); point users to +space-list instead
- lark-wiki-node-copy.md: replace wikcnSource with wikcn_EXAMPLE_TOKEN
  in migration workflow example

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants