feat(doc): emit typed error envelopes across the doc domain#1346
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:
📝 WalkthroughWalkthroughMigrates shortcuts/doc from legacy output helpers to typed errs, updates lint to mark shortcuts/doc as migrated, adds wrapDocNetworkErr, converts clipboard/media/docs validation and execution errors to errs types, upgrades API calls to CallAPITyped, and adds validation contract tests. ChangesTyped Error Migration for shortcuts/doc
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1346 +/- ##
==========================================
+ Coverage 71.79% 71.90% +0.11%
==========================================
Files 690 691 +1
Lines 65578 65611 +33
==========================================
+ Hits 47080 47177 +97
+ Misses 14843 14789 -54
+ Partials 3655 3645 -10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@4f2d885d1f77cbb48128c6c3e5731535ed559fd2🧩 Skill updatenpx skills add larksuite/cli#feat/errs-migrate-doc -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/doc/clipboard.go`:
- Line 242: The validation error returned for the clipboard read should preserve
the original exec error like the other cases; replace the current return of
errs.NewValidationError(...) with the same call chained with .WithCause(err) so
the returned error includes the underlying `err` (matching the pattern used at
the Linux and Windows decode locations).
🪄 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: ec59d40f-3128-453d-9ecb-38dccb90c836
📒 Files selected for processing (16)
.golangci.ymllint/errscontract/rule_no_legacy_common_helper_call.golint/errscontract/rule_no_legacy_envelope_literal.golint/errscontract/rules_test.goshortcuts/doc/clipboard.goshortcuts/doc/doc_errors.goshortcuts/doc/doc_media_download.goshortcuts/doc/doc_media_insert.goshortcuts/doc/doc_media_preview.goshortcuts/doc/doc_media_upload.goshortcuts/doc/docs_create_v2.goshortcuts/doc/docs_fetch_v2.goshortcuts/doc/docs_search.goshortcuts/doc/docs_update_v2.goshortcuts/doc/helpers.goshortcuts/doc/v2_only.go
evandance
force-pushed
the
feat/errs-migrate-doc
branch
from
08803dc to
17490ef
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/doc/docs_search.go`:
- Around line 162-164: The JSON unmarshal error is being discarded; preserve the
original parse error by attaching it as the cause when returning the validation
error. In the block that calls json.Unmarshal([]byte(filterStr), &filter) and
returns errs.NewValidationError(...).WithParam("--filter"), chain the original
err using .WithCause(err) on the returned validation error (keeping the existing
message and .WithParam("--filter")) so the error chain and parse details
(line/column, type mismatch) are preserved.
- Around line 226-239: The validation errors returned when toUnixSeconds fails
should attach the original parse error as a cause; update the error
constructions in the start and end branches (the NewValidationError(...) calls
that are returned) to call .WithCause(err) before .WithParam("--filter") so the
error chain preserves the original toUnixSeconds error for both the start and
end handling (look for rangeMap["start"]/rangeMap["end"], toUnixSeconds,
NewValidationError, WithParam, and result assignments).
🪄 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: 0b03c7b2-3681-4c3a-a4d5-171f47f6bc73
📒 Files selected for processing (16)
.golangci.ymllint/errscontract/rule_no_legacy_common_helper_call.golint/errscontract/rule_no_legacy_envelope_literal.golint/errscontract/rules_test.goshortcuts/doc/clipboard.goshortcuts/doc/doc_errors.goshortcuts/doc/doc_media_download.goshortcuts/doc/doc_media_insert.goshortcuts/doc/doc_media_preview.goshortcuts/doc/doc_media_upload.goshortcuts/doc/docs_create_v2.goshortcuts/doc/docs_fetch_v2.goshortcuts/doc/docs_search.goshortcuts/doc/docs_update_v2.goshortcuts/doc/helpers.goshortcuts/doc/v2_only.go
✅ Files skipped from review due to trivial changes (1)
- lint/errscontract/rule_no_legacy_envelope_literal.go
🚧 Files skipped from review as they are similar to previous changes (14)
- lint/errscontract/rule_no_legacy_common_helper_call.go
- shortcuts/doc/docs_create_v2.go
- shortcuts/doc/doc_media_upload.go
- shortcuts/doc/doc_media_preview.go
- shortcuts/doc/doc_errors.go
- shortcuts/doc/docs_update_v2.go
- shortcuts/doc/clipboard.go
- shortcuts/doc/helpers.go
- lint/errscontract/rules_test.go
- shortcuts/doc/docs_fetch_v2.go
- shortcuts/doc/v2_only.go
- shortcuts/doc/doc_media_download.go
- .golangci.yml
- shortcuts/doc/doc_media_insert.go
evandance
force-pushed
the
feat/errs-migrate-doc
branch
3 times, most recently
from
0d0c88f to
0dc555d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/doc/doc_media_insert.go (1)
700-759:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn invalid-response errors for malformed ancestor walks.
When this walk stops because of a cycle, a self-parent, or a fetched block with no
parent_id, it falls through to the--selection-with-ellipsisvalidation error. That blames user input for a malformed doc/MCP response and flips the shell exit category from internal to validation. Keep the final validation fallback for a genuine “match is outside the root tree” case, but short-circuit the malformed-walk branches witherrs.NewInternalError(errs.SubtypeInvalidResponse, ...).🤖 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/doc/doc_media_insert.go` around lines 700 - 759, The ancestor-walk currently falls through to errs.NewValidationError even when the loop stopped because of a cycle or malformed parent info; update the loop to short-circuit those malformed-walk cases and return errs.NewInternalError(errs.SubtypeInvalidResponse, ...) instead. Concretely: detect the failure reasons (visited[cur] cycle, parent == cur self-parent, parent == "" after the runtime.CallAPITyped fetch, or exceeding maxDepth) using the existing symbols (visited, cur, parent, nextParent, walkDepth, maxDepth, runtime.CallAPITyped) and return an internal-invalid-response error with a clear message when any of those conditions occur; keep the existing errs.NewValidationError only for the genuine “match is outside the root tree” fallback when the block was properly traversed but not found in rootSet.
🤖 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/doc/doc_errors_test.go`:
- Around line 22-52: The test helper docValidateRuntime currently creates fresh
cobra flags with empty defaults, losing real-command defaults (e.g., the --type
default in shortcuts/doc/doc_media_insert.go) and causing false negatives;
modify docValidateRuntime to either register flags by copying the mounted
command's flags (use cmd.Flags() from the real command and Clone or Iterate to
attach defaults) or explicitly seed each flag's default value before calling
fs.Set — locate docValidateRuntime and change the flag registration logic so it
reads defaults from the real command's flag definitions (or accepts a map of
defaults) instead of always using ""/0/false so validators see the same defaults
as the production commands.
---
Outside diff comments:
In `@shortcuts/doc/doc_media_insert.go`:
- Around line 700-759: The ancestor-walk currently falls through to
errs.NewValidationError even when the loop stopped because of a cycle or
malformed parent info; update the loop to short-circuit those malformed-walk
cases and return errs.NewInternalError(errs.SubtypeInvalidResponse, ...)
instead. Concretely: detect the failure reasons (visited[cur] cycle, parent ==
cur self-parent, parent == "" after the runtime.CallAPITyped fetch, or exceeding
maxDepth) using the existing symbols (visited, cur, parent, nextParent,
walkDepth, maxDepth, runtime.CallAPITyped) and return an
internal-invalid-response error with a clear message when any of those
conditions occur; keep the existing errs.NewValidationError only for the genuine
“match is outside the root tree” fallback when the block was properly traversed
but not found in rootSet.
🪄 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: f044e6bf-15ae-4a93-90b0-3e2d1c1d6d3e
📒 Files selected for processing (17)
.golangci.ymllint/errscontract/rule_no_legacy_common_helper_call.golint/errscontract/rule_no_legacy_envelope_literal.golint/errscontract/rules_test.goshortcuts/doc/clipboard.goshortcuts/doc/doc_errors.goshortcuts/doc/doc_errors_test.goshortcuts/doc/doc_media_download.goshortcuts/doc/doc_media_insert.goshortcuts/doc/doc_media_preview.goshortcuts/doc/doc_media_upload.goshortcuts/doc/docs_create_v2.goshortcuts/doc/docs_fetch_v2.goshortcuts/doc/docs_search.goshortcuts/doc/docs_update_v2.goshortcuts/doc/helpers.goshortcuts/doc/v2_only.go
🚧 Files skipped from review as they are similar to previous changes (15)
- lint/errscontract/rule_no_legacy_envelope_literal.go
- shortcuts/doc/doc_errors.go
- shortcuts/doc/docs_search.go
- shortcuts/doc/helpers.go
- shortcuts/doc/doc_media_preview.go
- lint/errscontract/rule_no_legacy_common_helper_call.go
- shortcuts/doc/docs_create_v2.go
- shortcuts/doc/docs_update_v2.go
- shortcuts/doc/doc_media_download.go
- shortcuts/doc/clipboard.go
- .golangci.yml
- shortcuts/doc/docs_fetch_v2.go
- shortcuts/doc/doc_media_upload.go
- lint/errscontract/rules_test.go
- shortcuts/doc/v2_only.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/doc/doc_errors.go`:
- Line 15: Add a unit test that exercises the untyped-network wrapping branch by
calling wrapDocNetworkErr with a raw error value and verifying the returned
error is a NetworkError with errs.SubtypeNetworkTransport and that the original
error is preserved as the cause (i.e., the result of .WithCause(err) is
present); specifically, create a test that calls wrapDocNetworkErr(err) where
err is a plain errors.New(...) and assert the subtype equals
errs.SubtypeNetworkTransport and that errors.Is/Unwrap (or the
NetworkError.Cause accessor) returns the original error to prevent regressions
in the errs.NewNetworkError(...).WithCause(err) path.
🪄 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: 796db4c2-1cdc-4a4d-a1dc-178f91786281
📒 Files selected for processing (17)
.golangci.ymllint/errscontract/rule_no_legacy_common_helper_call.golint/errscontract/rule_no_legacy_envelope_literal.golint/errscontract/rules_test.goshortcuts/doc/clipboard.goshortcuts/doc/doc_errors.goshortcuts/doc/doc_errors_test.goshortcuts/doc/doc_media_download.goshortcuts/doc/doc_media_insert.goshortcuts/doc/doc_media_preview.goshortcuts/doc/doc_media_upload.goshortcuts/doc/docs_create_v2.goshortcuts/doc/docs_fetch_v2.goshortcuts/doc/docs_search.goshortcuts/doc/docs_update_v2.goshortcuts/doc/helpers.goshortcuts/doc/v2_only.go
✅ Files skipped from review due to trivial changes (3)
- lint/errscontract/rule_no_legacy_envelope_literal.go
- lint/errscontract/rule_no_legacy_common_helper_call.go
- .golangci.yml
🚧 Files skipped from review as they are similar to previous changes (8)
- shortcuts/doc/docs_fetch_v2.go
- shortcuts/doc/docs_update_v2.go
- shortcuts/doc/helpers.go
- shortcuts/doc/v2_only.go
- shortcuts/doc/docs_search.go
- lint/errscontract/rules_test.go
- shortcuts/doc/doc_errors_test.go
- shortcuts/doc/docs_create_v2.go
evandance
force-pushed
the
feat/errs-migrate-doc
branch
2 times, most recently
from
058fa5d to
dc3c63e
Compare
Emit structured validation, API, network, file, and internal error envelopes for Doc shortcuts so users and agents can recover from failed document workflows using stable type, subtype, param, and code fields. Add Doc domain errscontract and golangci guards to prevent legacy envelope and common helper regressions.
evandance
force-pushed
the
feat/errs-migrate-doc
branch
from
dc3c63e to
4f2d885
Compare
github-actions
Bot
added
size/L
SunPeiYang996
approved these changes
Jun 9, 2026
This was referenced Jun 9, 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 Doc shortcut domain. Scope is limited to
shortcuts/doc, Doc error-contract guards, and Doc guard tests.Changes
errs.*envelopes.CallAPITyped/ProblemOf..golangci.ymlandlint/errscontractso legacy envelopes and common helpers cannot regress.Test Plan
gofmt -l <changed go files>produced no outputgo vet ./shortcuts/doc/...go test ./shortcuts/doc/...go test -C lint ./...go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=origin/main ./shortcuts/doc/...go run -C lint . ..go build ./...Related Issues
Summary by CodeRabbit
Improvements
Bug Fixes
Tests