feat(wiki): emit typed error envelopes across the wiki domain#1350
Merged
Conversation
evandance
added
size/M
|
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:
📝 WalkthroughWalkthroughMigrate shortcuts/wiki to typed errs: update lint exceptions, switch runtime.CallAPI -> CallAPITyped, convert validation/internal/API errors to errs.New* with parameter binding, refactor polling/retry hint propagation to use errs.ProblemOf, and update tests to assert typed problems and validation parameter hints. ChangesWiki Shortcuts Typed-Error Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size/L
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@c2ca5d86292a8ee5f2fc59ac0b3f8488e2ee3f1c🧩 Skill updatenpx skills add larksuite/cli#feat/errs-migrate-wiki -y -g |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1350 +/- ##
=======================================
Coverage 71.98% 71.98%
=======================================
Files 700 700
Lines 66293 66294 +1
=======================================
+ Hits 47721 47724 +3
Misses 14867 14867
+ Partials 3705 3703 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
evandance
force-pushed
the
feat/errs-migrate-wiki
branch
from
26172ff to
a98a591
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
shortcuts/wiki/wiki_node_list.go (1)
154-175:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn a typed invalid-response error when pagination fields are malformed.
At Lines 158-165, the unchecked assertions fall back to zero values, so a
code=0response with a baditems,has_more, orpage_tokenshape can be rendered as an empty/truncated list instead of surfacing the contract break. That leaves this wiki list path outside the typed invalid-response migration this PR is doing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/wiki/wiki_node_list.go` around lines 154 - 175, The code currently ignores type-assertion failures for data["items"], data["has_more"], and data["page_token"], silently using zero values; update the loop in the function that calls runtime.CallAPITyped to validate those fields' types and on any mismatch return a typed "invalid-response" error instead of continuing (i.e., check that data["items"] is []interface{} and each item is map[string]interface{} before calling wikiNodeListItem, and ensure data["has_more"] is bool and data["page_token"] is string); use the same error shape used elsewhere in the repo for malformed API responses so callers receive a clear contract-break error rather than an empty/truncated result.shortcuts/wiki/wiki_node_delete.go (1)
145-158:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate the delete response before interpreting
task_idas sync completion.
common.GetString(data, "task_id")collapses any non-stringtask_idto"", andrunWikiNodeDeletetreats""as a completed synchronous delete at Line 331. A malformed success payload would therefore be reported as finished instead of surfaced as a typed invalid-response error. Please branch on an explicitly validated response shape here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/wiki/wiki_node_delete.go` around lines 145 - 158, The code currently treats any non-string or missing "task_id" as "" by using common.GetString(data, "task_id"), which makes runWikiNodeDelete treat malformed success payloads as a completed sync delete; instead, after the CallAPITyped call in the same function (the block that calls wrapWikiNodeDeleteAPIError on err), explicitly validate that the response contains a "task_id" field of type string and non-empty (inspect data["task_id"] or use a typed accessor and an ok/type check) and if it is missing or not a string return a typed invalid-response error (do not return ""), otherwise return the validated task_id; reference the callers runWikiNodeDelete and the helper wrapWikiNodeDeleteAPIError/common.GetString to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@shortcuts/wiki/wiki_node_delete.go`:
- Around line 145-158: The code currently treats any non-string or missing
"task_id" as "" by using common.GetString(data, "task_id"), which makes
runWikiNodeDelete treat malformed success payloads as a completed sync delete;
instead, after the CallAPITyped call in the same function (the block that calls
wrapWikiNodeDeleteAPIError on err), explicitly validate that the response
contains a "task_id" field of type string and non-empty (inspect data["task_id"]
or use a typed accessor and an ok/type check) and if it is missing or not a
string return a typed invalid-response error (do not return ""), otherwise
return the validated task_id; reference the callers runWikiNodeDelete and the
helper wrapWikiNodeDeleteAPIError/common.GetString to locate the change.
In `@shortcuts/wiki/wiki_node_list.go`:
- Around line 154-175: The code currently ignores type-assertion failures for
data["items"], data["has_more"], and data["page_token"], silently using zero
values; update the loop in the function that calls runtime.CallAPITyped to
validate those fields' types and on any mismatch return a typed
"invalid-response" error instead of continuing (i.e., check that data["items"]
is []interface{} and each item is map[string]interface{} before calling
wikiNodeListItem, and ensure data["has_more"] is bool and data["page_token"] is
string); use the same error shape used elsewhere in the repo for malformed API
responses so callers receive a clear contract-break error rather than an
empty/truncated result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b06ba0e8-0b4e-4d85-bf51-a18af0e44764
📒 Files selected for processing (23)
.golangci.ymllint/errscontract/rule_no_legacy_common_helper_call.golint/errscontract/rule_no_legacy_envelope_literal.golint/errscontract/rules_test.goshortcuts/wiki/wiki_async_task.goshortcuts/wiki/wiki_async_task_test.goshortcuts/wiki/wiki_delete.goshortcuts/wiki/wiki_delete_test.goshortcuts/wiki/wiki_member_add.goshortcuts/wiki/wiki_member_helpers.goshortcuts/wiki/wiki_member_list.goshortcuts/wiki/wiki_member_remove.goshortcuts/wiki/wiki_move.goshortcuts/wiki/wiki_move_test.goshortcuts/wiki/wiki_node_copy.goshortcuts/wiki/wiki_node_create.goshortcuts/wiki/wiki_node_create_test.goshortcuts/wiki/wiki_node_delete.goshortcuts/wiki/wiki_node_delete_test.goshortcuts/wiki/wiki_node_get.goshortcuts/wiki/wiki_node_list.goshortcuts/wiki/wiki_space_create.goshortcuts/wiki/wiki_space_list.go
✅ Files skipped from review due to trivial changes (1)
- lint/errscontract/rule_no_legacy_common_helper_call.go
🚧 Files skipped from review as they are similar to previous changes (19)
- shortcuts/wiki/wiki_member_list.go
- shortcuts/wiki/wiki_member_helpers.go
- lint/errscontract/rule_no_legacy_envelope_literal.go
- shortcuts/wiki/wiki_delete.go
- shortcuts/wiki/wiki_delete_test.go
- shortcuts/wiki/wiki_async_task.go
- shortcuts/wiki/wiki_node_delete_test.go
- shortcuts/wiki/wiki_async_task_test.go
- lint/errscontract/rules_test.go
- shortcuts/wiki/wiki_space_list.go
- shortcuts/wiki/wiki_member_add.go
- shortcuts/wiki/wiki_space_create.go
- shortcuts/wiki/wiki_member_remove.go
- shortcuts/wiki/wiki_node_create_test.go
- shortcuts/wiki/wiki_move_test.go
- shortcuts/wiki/wiki_node_create.go
- shortcuts/wiki/wiki_move.go
- shortcuts/wiki/wiki_node_copy.go
- shortcuts/wiki/wiki_node_get.go
evandance
force-pushed
the
feat/errs-migrate-wiki
branch
from
a98a591 to
25a69d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/wiki/wiki_move_test.go (1)
207-225:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffIncomplete test coverage for typed error params.
The test cases on lines 207-225 validate error messages but don't assert the structured validation parameters, even though the production code (context snippet 1, lines 334, 350, 353, 356) sets
.WithParam()for these error paths:
- Line 207: Should assert
wantParams: []string{"--source-space-id"}- Line 212: Should assert
wantParams: []string{"--obj-type"}- Line 217: Should assert
wantParams: []string{"--obj-token"}- Line 222: Should assert
wantParams: []string{"--target-space-id"}As per coding guidelines, error-path tests must assert typed metadata (category/subtype/param) via
errs.ProblemOf, not message substrings alone.📋 Proposed fix to add missing param assertions
{ name: "source space requires node token", spec: wikiMoveSpec{SourceSpaceID: "space_src", ObjType: "sheet", ObjToken: "sheet_token", TargetSpaceID: "space_dst"}, wantErr: "can only be used with --node-token", + wantParams: []string{"--source-space-id"}, }, { name: "docs to wiki requires obj type", spec: wikiMoveSpec{ObjToken: "sheet_token", TargetSpaceID: "space_dst"}, wantErr: "--obj-type is required", + wantParams: []string{"--obj-type"}, }, { name: "docs to wiki requires obj token", spec: wikiMoveSpec{ObjType: "sheet", TargetSpaceID: "space_dst"}, wantErr: "--obj-token is required", + wantParams: []string{"--obj-token"}, }, { name: "docs to wiki requires target space", spec: wikiMoveSpec{ObjType: "sheet", ObjToken: "sheet_token"}, wantErr: "--target-space-id is required", + wantParams: []string{"--target-space-id"}, },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/wiki/wiki_move_test.go` around lines 207 - 225, The test cases in wiki_move_test.go that check validation errors need to also assert the structured error params set by the production code; update the four cases with names "source space requires node token", "docs to wiki requires obj type", "docs to wiki requires obj token", and "docs to wiki requires target space" by adding wantParams fields respectively: []string{"--source-space-id"}, []string{"--obj-type"}, []string{"--obj-token"}, and []string{"--target-space-id"} and change the verification to use errs.ProblemOf (or the existing helper that invokes errs.ProblemOf) to assert the Problem params match wantParams instead of only checking the error string.Source: Coding guidelines
🧹 Nitpick comments (1)
shortcuts/wiki/wiki_node_create_test.go (1)
132-150: ⚡ Quick winMake the shared validation helper lock the typed contract, not just param names.
Lines 132-150 only prove
erris a*errs.ValidationErrorand thatParamscontains the expected flags. The migrated callers still need message substrings to distinguish whether they got the right typed failure. Fold anerrs.ProblemOf(err)assertion forCategoryValidation/SubtypeInvalidArgumentinto this helper so the touched validation tests can stop keying on prose.As per coding guidelines,
*_test.goerror-path tests must assert typed metadata viaerrs.ProblemOfrather than message substrings alone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/wiki/wiki_node_create_test.go` around lines 132 - 150, The helper requireWikiValidationParams should also assert the typed problem metadata instead of relying on message substrings: after confirming err is a *errs.ValidationError, call errs.ProblemOf(err) and verify problem.Category == errs.CategoryValidation and problem.Subtype == errs.SubtypeInvalidArgument, failing the test with t.Fatalf if either check is false; keep the existing Params name checks (use the existing ve.Params loop) so callers no longer need to inspect error prose.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/wiki/wiki_move_test.go`:
- Around line 236-239: The test currently calls requireWikiValidationParams(t,
err, ...) which only asserts err is a *errs.ValidationError and checks
ve.Params; update the test to also assert typed metadata
(category/subtype/param) expectations: either extend requireWikiValidationParams
(defined in wiki_node_create_test.go) to accept and assert
errs.ProblemOf(category, subtype, param) and preserve error cause, or add
explicit assertions in wiki_move_test.go after the helper call that cast err to
*errs.ValidationError, check errs.ProblemOf(...) matches the error metadata and
that the original cause is preserved; reference the helper
requireWikiValidationParams, the *errs.ValidationError type, ve.Params, and
errs.ProblemOf when adding these checks.
In `@shortcuts/wiki/wiki_node_create.go`:
- Around line 484-488: The current requireWikiSpaceID function treats a missing
space.SpaceID as SubtypeInvalidArgument and suggests the caller change flags;
instead classify this as an invalid-response/internal error since GetSpace and
parseWikiSpaceRecord already validated caller input. Change the error
construction in requireWikiSpaceID to use the internal/invalid-response subtype
(e.g., errs.SubtypeInvalidResponse or errs.NewInternalError) rather than
errs.SubtypeInvalidArgument, and if you still want to surface the "--space-id"
escape hatch put it in a non-primary hint (e.g., .WithHint or similar) rather
than as the primary parameter; keep references to GetSpace and
parseWikiSpaceRecord in the comment/logic to indicate this is a
server/malformed-response issue.
---
Outside diff comments:
In `@shortcuts/wiki/wiki_move_test.go`:
- Around line 207-225: The test cases in wiki_move_test.go that check validation
errors need to also assert the structured error params set by the production
code; update the four cases with names "source space requires node token", "docs
to wiki requires obj type", "docs to wiki requires obj token", and "docs to wiki
requires target space" by adding wantParams fields respectively:
[]string{"--source-space-id"}, []string{"--obj-type"}, []string{"--obj-token"},
and []string{"--target-space-id"} and change the verification to use
errs.ProblemOf (or the existing helper that invokes errs.ProblemOf) to assert
the Problem params match wantParams instead of only checking the error string.
---
Nitpick comments:
In `@shortcuts/wiki/wiki_node_create_test.go`:
- Around line 132-150: The helper requireWikiValidationParams should also assert
the typed problem metadata instead of relying on message substrings: after
confirming err is a *errs.ValidationError, call errs.ProblemOf(err) and verify
problem.Category == errs.CategoryValidation and problem.Subtype ==
errs.SubtypeInvalidArgument, failing the test with t.Fatalf if either check is
false; keep the existing Params name checks (use the existing ve.Params loop) so
callers no longer need to inspect error prose.
🪄 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: d0f55e9e-f028-4d3f-a4b2-c755b736c234
📒 Files selected for processing (24)
.golangci.ymllint/errscontract/rule_no_legacy_common_helper_call.golint/errscontract/rule_no_legacy_envelope_literal.golint/errscontract/rules_test.goshortcuts/wiki/wiki_async_task.goshortcuts/wiki/wiki_async_task_test.goshortcuts/wiki/wiki_delete.goshortcuts/wiki/wiki_delete_test.goshortcuts/wiki/wiki_list_copy_test.goshortcuts/wiki/wiki_member_add.goshortcuts/wiki/wiki_member_helpers.goshortcuts/wiki/wiki_member_list.goshortcuts/wiki/wiki_member_remove.goshortcuts/wiki/wiki_move.goshortcuts/wiki/wiki_move_test.goshortcuts/wiki/wiki_node_copy.goshortcuts/wiki/wiki_node_create.goshortcuts/wiki/wiki_node_create_test.goshortcuts/wiki/wiki_node_delete.goshortcuts/wiki/wiki_node_delete_test.goshortcuts/wiki/wiki_node_get.goshortcuts/wiki/wiki_node_list.goshortcuts/wiki/wiki_space_create.goshortcuts/wiki/wiki_space_list.go
✅ Files skipped from review due to trivial changes (1)
- shortcuts/wiki/wiki_member_helpers.go
🚧 Files skipped from review as they are similar to previous changes (18)
- shortcuts/wiki/wiki_space_list.go
- lint/errscontract/rule_no_legacy_envelope_literal.go
- shortcuts/wiki/wiki_member_list.go
- shortcuts/wiki/wiki_space_create.go
- shortcuts/wiki/wiki_node_delete_test.go
- lint/errscontract/rule_no_legacy_common_helper_call.go
- shortcuts/wiki/wiki_delete.go
- lint/errscontract/rules_test.go
- shortcuts/wiki/wiki_member_add.go
- shortcuts/wiki/wiki_async_task.go
- .golangci.yml
- shortcuts/wiki/wiki_node_list.go
- shortcuts/wiki/wiki_node_get.go
- shortcuts/wiki/wiki_delete_test.go
- shortcuts/wiki/wiki_member_remove.go
- shortcuts/wiki/wiki_node_copy.go
- shortcuts/wiki/wiki_move.go
- shortcuts/wiki/wiki_async_task_test.go
evandance
force-pushed
the
feat/errs-migrate-wiki
branch
from
25a69d7 to
fb86c05
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/wiki/wiki_async_task_test.go`:
- Around line 101-107: The test currently only checks p.Hint text; update it to
also assert the returned problem subtype and that the original transport error
is preserved as the cause: call errs.ProblemOf(err) as before, then assert
p.Subtype (or the concrete typed value returned by ProblemOf) equals the
expected subtype for the fallback branch and use errors.Is(err, transportErr)
(or equivalent) to verify that lastErr/transportErr was preserved via WithCause;
keep the existing p.Hint assertions too so the test verifies subtype, cause
preservation, and hint content together.
In `@shortcuts/wiki/wiki_async_task.go`:
- Around line 103-106: Add a unit test that directly calls
parseWikiAsyncTaskStatus(taskID, nil, resultKey) and asserts it returns an empty
wikiAsyncTaskStatus and an error created via errs.NewInternalError with subtype
errs.SubtypeInvalidResponse; locate the function under test
(parseWikiAsyncTaskStatus) and add a test in the corresponding _test.go that
verifies the error type/subtype and message contract to lock down the new
invalid-response behavior.
- Around line 180-191: In the fallback branch where errs.ProblemOf(lastErr)
returns false (inside the poll error handling in wiki_async_task.go), don't
classify the wrapped error as errs.SubtypeSDKError; instead call
errs.NewInternalError with errs.SubtypeUnknown and preserve the hint and cause
(i.e., replace the NewInternalError(errs.SubtypeSDKError, ...) call with
NewInternalError(errs.SubtypeUnknown, ...) and keep the WithHint and
WithCause(lastErr) chaining) so unclassified lower-layer errors are not
over-classified; keep the existing logic that appends p.Hint when ProblemOf
succeeds and match the behavior of wrapWikiNodeDeleteAPIError.
🪄 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: d748a802-bef5-4761-bcaf-468f9fc832fe
📒 Files selected for processing (24)
.golangci.ymllint/errscontract/rule_no_legacy_common_helper_call.golint/errscontract/rule_no_legacy_envelope_literal.golint/errscontract/rules_test.goshortcuts/wiki/wiki_async_task.goshortcuts/wiki/wiki_async_task_test.goshortcuts/wiki/wiki_delete.goshortcuts/wiki/wiki_delete_test.goshortcuts/wiki/wiki_list_copy_test.goshortcuts/wiki/wiki_member_add.goshortcuts/wiki/wiki_member_helpers.goshortcuts/wiki/wiki_member_list.goshortcuts/wiki/wiki_member_remove.goshortcuts/wiki/wiki_move.goshortcuts/wiki/wiki_move_test.goshortcuts/wiki/wiki_node_copy.goshortcuts/wiki/wiki_node_create.goshortcuts/wiki/wiki_node_create_test.goshortcuts/wiki/wiki_node_delete.goshortcuts/wiki/wiki_node_delete_test.goshortcuts/wiki/wiki_node_get.goshortcuts/wiki/wiki_node_list.goshortcuts/wiki/wiki_space_create.goshortcuts/wiki/wiki_space_list.go
✅ Files skipped from review due to trivial changes (1)
- shortcuts/wiki/wiki_member_helpers.go
🚧 Files skipped from review as they are similar to previous changes (20)
- shortcuts/wiki/wiki_space_list.go
- shortcuts/wiki/wiki_delete_test.go
- shortcuts/wiki/wiki_delete.go
- lint/errscontract/rules_test.go
- lint/errscontract/rule_no_legacy_common_helper_call.go
- shortcuts/wiki/wiki_list_copy_test.go
- shortcuts/wiki/wiki_node_copy.go
- shortcuts/wiki/wiki_member_remove.go
- shortcuts/wiki/wiki_node_delete_test.go
- shortcuts/wiki/wiki_node_list.go
- lint/errscontract/rule_no_legacy_envelope_literal.go
- shortcuts/wiki/wiki_node_create_test.go
- shortcuts/wiki/wiki_node_get.go
- shortcuts/wiki/wiki_move_test.go
- shortcuts/wiki/wiki_space_create.go
- shortcuts/wiki/wiki_member_add.go
- shortcuts/wiki/wiki_node_delete.go
- shortcuts/wiki/wiki_node_create.go
- .golangci.yml
- shortcuts/wiki/wiki_move.go
evandance
force-pushed
the
feat/errs-migrate-wiki
branch
from
fb86c05 to
7113a61
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/wiki/wiki_node_list.go (1)
169-176:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a non-advancing cursor guard in node auto-pagination.
With
--page-alland--page-limit 0, a buggy API that returns the samepage_tokenwithhas_more=truecan loop forever.fetchWikiMembersalready protects this case;fetchWikiNodesshould mirror it.Suggested fix
if !lastHasMore || lastPageToken == "" { break } + if lastPageToken == pageToken { + fmt.Fprintf(runtime.IO().ErrOut, "Stopping pagination: server returned a non-advancing page_token.\n") + break + } if pageLimit > 0 && page+1 >= pageLimit { break }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/wiki/wiki_node_list.go` around lines 169 - 176, The auto-pagination loop in fetchWikiNodes can enter an infinite loop if the API returns the same page_token with has_more=true; add a non-advancing cursor guard like in fetchWikiMembers: track the previous pageToken (e.g., lastPageToken) and after computing lastPageToken compare it to the current pageToken, and break the loop if they are equal (or if lastPageToken == ""). Ensure you update/assign lastPageToken appropriately inside the loop before checking lastHasMore/pageLimit and set pageToken = lastPageToken only when it advanced; mirror the same guard logic used in fetchWikiMembers to prevent infinite loops.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@shortcuts/wiki/wiki_node_list.go`:
- Around line 169-176: The auto-pagination loop in fetchWikiNodes can enter an
infinite loop if the API returns the same page_token with has_more=true; add a
non-advancing cursor guard like in fetchWikiMembers: track the previous
pageToken (e.g., lastPageToken) and after computing lastPageToken compare it to
the current pageToken, and break the loop if they are equal (or if lastPageToken
== ""). Ensure you update/assign lastPageToken appropriately inside the loop
before checking lastHasMore/pageLimit and set pageToken = lastPageToken only
when it advanced; mirror the same guard logic used in fetchWikiMembers to
prevent infinite loops.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c7285bd-8a34-4d0c-ab15-445e14e7409d
📒 Files selected for processing (24)
.golangci.ymllint/errscontract/rule_no_legacy_common_helper_call.golint/errscontract/rule_no_legacy_envelope_literal.golint/errscontract/rules_test.goshortcuts/wiki/wiki_async_task.goshortcuts/wiki/wiki_async_task_test.goshortcuts/wiki/wiki_delete.goshortcuts/wiki/wiki_delete_test.goshortcuts/wiki/wiki_list_copy_test.goshortcuts/wiki/wiki_member_add.goshortcuts/wiki/wiki_member_helpers.goshortcuts/wiki/wiki_member_list.goshortcuts/wiki/wiki_member_remove.goshortcuts/wiki/wiki_move.goshortcuts/wiki/wiki_move_test.goshortcuts/wiki/wiki_node_copy.goshortcuts/wiki/wiki_node_create.goshortcuts/wiki/wiki_node_create_test.goshortcuts/wiki/wiki_node_delete.goshortcuts/wiki/wiki_node_delete_test.goshortcuts/wiki/wiki_node_get.goshortcuts/wiki/wiki_node_list.goshortcuts/wiki/wiki_space_create.goshortcuts/wiki/wiki_space_list.go
🚧 Files skipped from review as they are similar to previous changes (22)
- shortcuts/wiki/wiki_member_helpers.go
- lint/errscontract/rule_no_legacy_envelope_literal.go
- shortcuts/wiki/wiki_space_list.go
- shortcuts/wiki/wiki_async_task_test.go
- shortcuts/wiki/wiki_member_remove.go
- lint/errscontract/rule_no_legacy_common_helper_call.go
- shortcuts/wiki/wiki_move_test.go
- shortcuts/wiki/wiki_delete.go
- shortcuts/wiki/wiki_node_delete_test.go
- shortcuts/wiki/wiki_list_copy_test.go
- shortcuts/wiki/wiki_space_create.go
- shortcuts/wiki/wiki_member_add.go
- lint/errscontract/rules_test.go
- shortcuts/wiki/wiki_delete_test.go
- shortcuts/wiki/wiki_node_create_test.go
- shortcuts/wiki/wiki_async_task.go
- shortcuts/wiki/wiki_node_copy.go
- shortcuts/wiki/wiki_node_get.go
- .golangci.yml
- shortcuts/wiki/wiki_move.go
- shortcuts/wiki/wiki_node_delete.go
- shortcuts/wiki/wiki_node_create.go
evandance
force-pushed
the
feat/errs-migrate-wiki
branch
2 times, most recently
from
1489037 to
905858f
Compare
Emit structured validation, API, network, file, and internal error envelopes for Wiki shortcuts so users and agents can recover from failed wiki workflows using stable type, subtype, param, and code fields. Add Wiki domain errscontract and golangci guards to prevent legacy envelope and common helper regressions.
evandance
force-pushed
the
feat/errs-migrate-wiki
branch
from
905858f to
c2ca5d8
Compare
liujinkun2025
approved these changes
Jun 11, 2026
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
Migrates typed error envelopes across the Wiki shortcut domain. Scope is limited to
shortcuts/wiki, Wiki error-contract guards, and Wiki guard tests.Changes
errs.*envelopes..golangci.ymlandlint/errscontractso legacy envelopes and common helpers cannot regress.Test Plan
gofmt -l <changed go files>produced no outputgo vet ./shortcuts/wiki/...go test ./shortcuts/wiki/...go test -C lint ./...go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=origin/maingo run -C lint . ..go build ./...Related Issues
Summary by CodeRabbit
Bug Fixes
Chores / Refactor
Tests