feat(wiki): add wiki +node-create shortcut#320
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:
📝 WalkthroughWalkthroughRegisters a new Changes
Sequence DiagramsequenceDiagram
participant CLI as User/CLI
participant Exec as Shortcut Executor
participant Val as Validator
participant Resolver as Space Resolver
participant WikiAPI as Wiki API
participant Out as Output Handler
CLI->>Exec: run +node-create (flags)
Exec->>Val: validate spec
Val-->>Exec: ok / error
alt valid
Exec->>Resolver: resolve target space
alt explicit --space-id
Resolver->>Resolver: use provided ID (resolve `my_library` alias if given)
Resolver-->>Exec: space_id, resolved_by
else --parent-node-token
Resolver->>WikiAPI: GET /open-apis/wiki/v2/spaces/get_node (parent token)
WikiAPI-->>Resolver: parent node (space_id)
Resolver-->>Exec: space_id, resolved_by
else user fallback
Resolver->>WikiAPI: GET /open-apis/wiki/v2/spaces/my_library
WikiAPI-->>Resolver: my_library space_id
Resolver-->>Exec: space_id, resolved_by
end
Exec->>WikiAPI: POST /open-apis/wiki/v2/spaces/{space_id}/nodes (body)
WikiAPI-->>Exec: created node response
Exec->>Out: print resolved_by, resolved_space_id, node_token, metadata
Out-->>CLI: success
else invalid
Val-->>CLI: validation error
end
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 docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds a Confidence Score: 5/5Safe to merge — no P0/P1 issues found and previously flagged concerns are resolved. All remaining findings are P2 or lower. The two P1-level issues from prior review threads (spurious my_library dry-run step and progress message ordering) are both correctly addressed in the current code. Logic for space resolution, validation, dry-run, and execution is well-tested. No files require special attention.
|
| Filename | Overview |
|---|---|
| shortcuts/wiki/wiki_node_create.go | Core implementation of the shortcut: spec parsing, validation, space resolution, dry-run, and execution — all correct and well-structured; previous issues are fixed. |
| shortcuts/wiki/wiki_node_create_test.go | Good coverage across validation, resolution paths, dry-run steps, and mounted execution; fake client behaviour for unknown tokens is safe. |
| shortcuts/wiki/shortcuts.go | Minimal registration file; correct. |
| shortcuts/register.go | Adds the wiki package to the aggregated shortcut list; straightforward one-liner change. |
| skills/lark-wiki/SKILL.md | Adds the +node-create shortcut entry to the wiki skill index; consistent with code. |
| skills/lark-wiki/references/lark-wiki-node-create.md | Reference doc covers all flags, resolution rules, bot constraints, dry-run behaviour, and output fields accurately. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[wiki +node-create] --> B{Validate spec}
B -->|invalid| ERR1[Return validation error]
B -->|valid| C{SpaceID set?}
C -->|Yes| D{SpaceID == my_library?}
D -->|Yes| E[GET /spaces/my_library\nresolve numeric space_id]
D -->|No| F{ParentNodeToken set?}
E --> F
F -->|Yes| G[GET /spaces/get_node\nvalidate parent space matches]
F -->|No| H[Use explicit SpaceID]
G --> I{spaces match?}
I -->|No| ERR2[Return mismatch error]
I -->|Yes| H
C -->|No| J{ParentNodeToken set?}
J -->|Yes| K[GET /spaces/get_node\ninfer space from parent]
J -->|No| L{identity == bot?}
L -->|Yes| ERR3[Return bot constraint error]
L -->|No| M[GET /spaces/my_library\npersonal library fallback]
H --> N[POST /spaces/:space_id/nodes\nCreate wiki node]
K --> N
M --> N
N --> O[Return resolved_space_id, node_token, obj_token ...]
Reviews (5): Last reviewed commit: "add wiki node create shortcut" | Re-trigger Greptile
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@91d94dbb75ac1b7f4c67a494a9c6b3f66cef94b1🧩 Skill updatenpx skills add larksuite/cli#feat/wiki_create_shortcut -y -g |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/wiki/wiki_node_create_test.go`:
- Around line 316-318: The test
TestWikiNodeCreateMountedExecuteWithExplicitSpaceID should isolate config state
before calling cmdutil.TestFactory: call t.Setenv("LARKSUITE_CLI_CONFIG_DIR",
t.TempDir()) at the start of the test (before invoking cmdutil.TestFactory or
any real auth/config plumbing) so the test uses an empty temporary config
directory and cannot leak or be affected by shared/local config; update the test
function to set this environment variable prior to creating the factory and
registering stubs (e.g., before registerWikiBotTokenStub or factory, stdout
assignment).
In `@shortcuts/wiki/wiki_node_create.go`:
- Around line 223-227: The current validation in wiki_node_create.go only
rejects bots when both location inputs are missing, but allows a bot to pass if
spec.ParentNodeToken is set while spec.SpaceID == wikiMyLibrarySpaceID; add an
explicit check using identity.IsBot() and spec.SpaceID to reject the reserved
wikiMyLibrarySpaceID for bots regardless of parent token. Specifically, add a
guard like "if identity.IsBot() && spec.SpaceID == wikiMyLibrarySpaceID { return
output.ErrValidation(...)" before calling GetSpace or the existing combined
validation so any request with identity.IsBot() and spec.SpaceID ==
wikiMyLibrarySpaceID is rejected up front.
- Around line 232-276: buildWikiNodeCreateDryRun currently shows a fake
my_library lookup and hides an explicit --space-id when a parent token is
provided because needsMyLibraryLookup returns true whenever SpaceID is empty
(even for parent-only flows) and dryRunWikiNodeCreateSpaceID suppresses a
provided SpaceID if ParentNodeToken != "". Fix by updating needsMyLibraryLookup
to only request a my_library lookup when SpaceID is empty AND ParentNodeToken is
empty (or when SpaceID == wikiMyLibrarySpaceID), and change
dryRunWikiNodeCreateSpaceID to return spec.SpaceID whenever it is non-empty
(regardless of ParentNodeToken) so an explicit --space-id is shown in the
dry-run; update references in buildWikiNodeCreateDryRun accordingly.
🪄 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: c2bf5f39-6f90-4159-a022-4039236f19a9
📒 Files selected for processing (6)
shortcuts/register.goshortcuts/wiki/shortcuts.goshortcuts/wiki/wiki_node_create.goshortcuts/wiki/wiki_node_create_test.goskills/lark-wiki/SKILL.mdskills/lark-wiki/references/lark-wiki-node-create.md
27360a2 to
759724b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
shortcuts/wiki/wiki_node_create.go (1)
274-279:⚠️ Potential issue | 🟠 MajorKeep the explicit
--space-idin dry-run URLs.When both
--space-idand--parent-node-tokenare set, execution still creates under the explicit space; the parent lookup only validates consistency. Line 275 turns that deterministic POST into/spaces/<resolved_space_id>/nodes, so--dry-runmisstates the actual request for the explicit-space + parent flow.Suggested fix
func dryRunWikiNodeCreateSpaceID(spec wikiNodeCreateSpec) string { - if spec.SpaceID != "" && spec.SpaceID != wikiMyLibrarySpaceID && spec.ParentNodeToken == "" { + if spec.SpaceID != "" && spec.SpaceID != wikiMyLibrarySpaceID { return spec.SpaceID } return "<resolved_space_id>" }Please also add a dry-run test for
--space-id space_123 --parent-node-token ...to lock this path down.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/wiki/wiki_node_create.go` around lines 274 - 279, The dry-run currently hides an explicit space when a parent token is provided; update dryRunWikiNodeCreateSpaceID to return spec.SpaceID whenever spec.SpaceID != "" (remove the ParentNodeToken=="" check) so the dry-run URL shows /spaces/<explicit_space_id>/nodes for the explicit-space + parent flow, and add a unit/integration dry-run test that invokes the CLI with --space-id space_123 and --parent-node-token <token> to assert the dry-run output contains /spaces/space_123/nodes.
🤖 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_create_test.go`:
- Around line 389-419: Test is failing because the real `--as bot` flow needs
the tenant bot auth endpoint; before calling mountAndRunWiki(WikiNodeCreate,
...) register a bot-auth stub (or call the package’s shared bot-auth helper)
immediately after cmdutil.TestFactory(t, wikiTestConfig()) so the
`/open-apis/auth/v3/tenant_access_token/internal` request is handled; keep
existing createStub/registration for the wiki create endpoint and ensure the
auth stub is registered on the same `reg` used for createStub.
In `@skills/lark-wiki/references/lark-wiki-node-create.md`:
- Around line 24-27: Update the docs for the lark-cli wiki +node_create example
to explicitly state that the flag value "--space-id my_library" refers to a
personal/user-only space and is rejected when run as the bot identity; change
the example text and nearby examples that show "--space-id my_library" (the
occurrences around the node_create usage and the later examples) to note
"user-only / not valid for bot identity" or show an alternative (e.g., omit
--space-id or use a valid space id) so the reference matches the actual
validation behavior.
---
Duplicate comments:
In `@shortcuts/wiki/wiki_node_create.go`:
- Around line 274-279: The dry-run currently hides an explicit space when a
parent token is provided; update dryRunWikiNodeCreateSpaceID to return
spec.SpaceID whenever spec.SpaceID != "" (remove the ParentNodeToken=="" check)
so the dry-run URL shows /spaces/<explicit_space_id>/nodes for the
explicit-space + parent flow, and add a unit/integration dry-run test that
invokes the CLI with --space-id space_123 and --parent-node-token <token> to
assert the dry-run output contains /spaces/space_123/nodes.
🪄 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: 1d549466-8009-496d-8a80-6c3b74ab05d7
📒 Files selected for processing (6)
shortcuts/register.goshortcuts/wiki/shortcuts.goshortcuts/wiki/wiki_node_create.goshortcuts/wiki/wiki_node_create_test.goskills/lark-wiki/SKILL.mdskills/lark-wiki/references/lark-wiki-node-create.md
✅ Files skipped from review due to trivial changes (3)
- shortcuts/wiki/shortcuts.go
- skills/lark-wiki/SKILL.md
- shortcuts/register.go
759724b to
53358f8
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/wiki/wiki_node_create.go (1)
274-279: Dry-run hides explicit--space-idwhen--parent-node-tokenis also provided.When both
--space-idand--parent-node-tokenare supplied,dryRunWikiNodeCreateSpaceIDreturns<resolved_space_id>instead of the explicit space ID. This makes the dry-run output less precise for AI agents, though functionally the space ID is validated at runtime.🔧 Optional fix to show explicit space-id in dry-run
func dryRunWikiNodeCreateSpaceID(spec wikiNodeCreateSpec) string { - if spec.SpaceID != "" && spec.SpaceID != wikiMyLibrarySpaceID && spec.ParentNodeToken == "" { + if spec.SpaceID != "" && spec.SpaceID != wikiMyLibrarySpaceID { return spec.SpaceID } return "<resolved_space_id>" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/wiki/wiki_node_create.go` around lines 274 - 279, dryRunWikiNodeCreateSpaceID currently hides an explicit spec.SpaceID when spec.ParentNodeToken is set, returning "<resolved_space_id>" which reduces dry-run precision; change the logic in dryRunWikiNodeCreateSpaceID so that if spec.SpaceID is non-empty and not equal to wikiMyLibrarySpaceID you return spec.SpaceID regardless of spec.ParentNodeToken (i.e., remove the ParentNodeToken check), keeping the fallback "<resolved_space_id>" only when no explicit SpaceID is provided.
🤖 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_node_create.go`:
- Around line 274-279: dryRunWikiNodeCreateSpaceID currently hides an explicit
spec.SpaceID when spec.ParentNodeToken is set, returning "<resolved_space_id>"
which reduces dry-run precision; change the logic in dryRunWikiNodeCreateSpaceID
so that if spec.SpaceID is non-empty and not equal to wikiMyLibrarySpaceID you
return spec.SpaceID regardless of spec.ParentNodeToken (i.e., remove the
ParentNodeToken check), keeping the fallback "<resolved_space_id>" only when no
explicit SpaceID is provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bb587b3e-a945-4d67-9de0-2181657f7b6a
📒 Files selected for processing (6)
shortcuts/register.goshortcuts/wiki/shortcuts.goshortcuts/wiki/wiki_node_create.goshortcuts/wiki/wiki_node_create_test.goskills/lark-wiki/SKILL.mdskills/lark-wiki/references/lark-wiki-node-create.md
✅ Files skipped from review due to trivial changes (2)
- shortcuts/wiki/shortcuts.go
- skills/lark-wiki/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- shortcuts/register.go
- shortcuts/wiki/wiki_node_create_test.go
53358f8 to
21cb6a2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
shortcuts/wiki/wiki_node_create_test.go (1)
415-445:⚠️ Potential issue | 🟠 MajorStub bot auth before executing the mounted command.
This still exercises the real
--as botpath, but only the wiki create endpoint is registered. The command will request/open-apis/auth/v3/tenant_access_token/internalfirst, so the test fails before it ever reachescreateStub. Please register the shared bot-auth stub, or stub that endpoint inline on the samereg, before callingmountAndRunWiki.🤖 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 415 - 445, The test fails because it never stubs the bot auth request used by the command; before calling mountAndRunWiki(t, WikiNodeCreate, ... , factory, stdout) register the shared bot-auth stub (or add an inline httpmock.Stub on reg for POST "/open-apis/auth/v3/tenant_access_token/internal") so the tenant access token request is handled; ensure the auth stub is registered on the same reg used for createStub so the command can proceed to the wiki create endpoint.
🧹 Nitpick comments (1)
shortcuts/wiki/wiki_node_create_test.go (1)
100-105: Avoid asserting that the wiki shortcut list has exactly one entry.This will fail as soon as the package grows another wiki shortcut, even if
+node_createis still registered. Checking membership keeps the test focused on the behavior it actually cares about.♻️ Suggested change
shortcuts := Shortcuts() - if len(shortcuts) != 1 { - t.Fatalf("len(Shortcuts()) = %d, want 1", len(shortcuts)) - } - if shortcuts[0].Command != "+node_create" { - t.Fatalf("shortcut command = %q, want %q", shortcuts[0].Command, "+node_create") + found := false + for _, candidate := range shortcuts { + if candidate.Command == "+node_create" { + found = true + break + } + } + if !found { + t.Fatalf("Shortcuts() missing %q: %#v", "+node_create", shortcuts) }🤖 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 asserts the Shortcuts() slice has length 1 and that shortcuts[0].Command == "+node_create", which will break as more shortcuts are added; update the test to assert membership instead: call Shortcuts(), iterate over the returned slice (or use a helper) and check for an element whose Command field equals "+node_create", and only fail the test with t.Fatalf if no such element is found, leaving any other entries unconstrained.
🤖 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_create.go`:
- Around line 57-62: The DryRun path for wiki node creation ignores runtime
identity and only uses flags, causing bot runtimes to produce user-style lookup
plans; update the DryRun func to pass runtime.As() into the dry-run builder so
it mirrors Validate's identity checks: change DryRun to call
buildWikiNodeCreateDryRun with both readWikiNodeCreateSpec(runtime) and
runtime.As() (or a new parameter representing the actor) and update
buildWikiNodeCreateDryRun (and any helper helpers it calls) to enforce the same
bot-only guards used by validateWikiNodeCreateSpec so the dry-run fails for bot
contexts the same way validation does.
---
Duplicate comments:
In `@shortcuts/wiki/wiki_node_create_test.go`:
- Around line 415-445: The test fails because it never stubs the bot auth
request used by the command; before calling mountAndRunWiki(t, WikiNodeCreate,
... , factory, stdout) register the shared bot-auth stub (or add an inline
httpmock.Stub on reg for POST "/open-apis/auth/v3/tenant_access_token/internal")
so the tenant access token request is handled; ensure the auth stub is
registered on the same reg used for createStub so the command can proceed to the
wiki create endpoint.
---
Nitpick comments:
In `@shortcuts/wiki/wiki_node_create_test.go`:
- Around line 100-105: The test currently asserts the Shortcuts() slice has
length 1 and that shortcuts[0].Command == "+node_create", which will break as
more shortcuts are added; update the test to assert membership instead: call
Shortcuts(), iterate over the returned slice (or use a helper) and check for an
element whose Command field equals "+node_create", and only fail the test with
t.Fatalf if no such element is found, leaving any other entries unconstrained.
🪄 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: 182d0eb4-ed1d-497b-ac85-ae6d658b14d0
📒 Files selected for processing (6)
shortcuts/register.goshortcuts/wiki/shortcuts.goshortcuts/wiki/wiki_node_create.goshortcuts/wiki/wiki_node_create_test.goskills/lark-wiki/SKILL.mdskills/lark-wiki/references/lark-wiki-node-create.md
✅ Files skipped from review due to trivial changes (1)
- skills/lark-wiki/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- shortcuts/register.go
- shortcuts/wiki/shortcuts.go
Change-Id: I4810fc541c31ae9e3e08539d4b1c91d01f53b7f5
21cb6a2 to
91d94db
Compare
Summary
This PR adds a
wiki +node-createshortcut so callers can create wiki nodes without resolving a numericspace_idfirst. It supports automatic space resolution frommy_libraryor a parent node, validates shortcut-specific inputs, and keeps dry-runand output behavior explicit for AI-agent workflows.
Changes
wikishortcut package and exposewiki +node_create--space-id,--parent-node-token, or the usermy_libraryfallback--node-type,--origin-node-token, bot identity constraints, and parent/space consistency checksresolved_space_id,resolved_by,node_token, andobj_tokenskills/lark-wikiand add a dedicated reference pageTest Plan
lark xxxcommand works as expected./lark-cli wiki +node_create --as user./lark-cli wiki +node_create --as user --node-type shortcut --origin-node-token JX0xxxxgo test ./shortcuts/wikicurrently fails locally withhttpmock: unmatched stub: /open-apis/auth/v3/tenant_access_token/internalRelated Issues
Summary by CodeRabbit
New Features
Documentation
Tests