feat(base): support BaseApp application mode - #2231
Conversation
Implement the CLI layer of the BaseApp CLI/OpenAPI protocol design: 17 new shortcuts covering workspace entities, blank app creation, page CRUD and page block CRUD, plus skill references and dry-run E2E for each. The data_config validator moves to a neutral block_data_config.go with chart logic unchanged; list and richText dispatch are new branches, so dashboard behaviour is untouched. Command spaces stay separate — dashboard commands never take --app-token and app block commands never take --dashboard-id. The one exception is +app-block-get-data, which shares the dashboard endpoint, execute and dry-run hooks and therefore takes --base-token instead of --app-token. This phase ships no +app-block-delete and no page arrange command; both the help text and the skill docs spell out that a block type cannot be changed after creation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
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:
📝 WalkthroughWalkthroughThe CLI adds BaseApp and workspace commands for resource management. It adds block configuration validation, computed-data retrieval, BaseApp URL resolution, operating documentation, and broad unit, contract, dry-run, API, and pagination coverage. ChangesBaseApp application support
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
skills/lark-drive/references/lark-drive-delete.md (1)
54-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove lower-layer routing details from this caller reference.
Document
--type baseappas the supported CLI input. Do not expose the internalbitabletranslation or direct native API behavior in this reference.Based on learnings, avoid exposing internal implementation or compatibility details in
skills/lark-drive/references/*.md.🤖 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 `@skills/lark-drive/references/lark-drive-delete.md` around lines 54 - 72, Update the delete command reference to document baseapp only as a supported --type input and remove internal routing details, including base/baseapp shortcut normalization, bitable translation, and direct native Drive API behavior. Keep the user-facing deletion command, required parameters, and asynchronous polling behavior unchanged.Source: Learnings
shortcuts/base/base_execute_test.go (1)
374-374: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the test to name the command under test.
The test exercises
BaseAppBlockCreatewith--type list. The nameTestBaseAppListCreateOmitsUnspecifiedOptionalFieldsreads as a page or entity list create. UseTestBaseAppBlockCreateListOmitsUnspecifiedOptionalFields.🤖 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/base/base_execute_test.go` at line 374, Rename the test function from TestBaseAppListCreateOmitsUnspecifiedOptionalFields to TestBaseAppBlockCreateListOmitsUnspecifiedOptionalFields to identify the BaseAppBlockCreate command while preserving the test behavior.shortcuts/base/app_block_create.go (1)
52-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueParse
data-configone time.The list branch parses
data-configat Line 58. The common path parses the same value again at Line 80. Both calls use the samenewParseCtx(runtime)input. Hoist the parse above the type branch and pass the parsed map tovalidateAppListDataConfig. This removes the duplicate JSON decode and keeps one source of the parsed config.🤖 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/base/app_block_create.go` around lines 52 - 68, Hoist the trimmed non-empty data-config parsing using newParseCtx(runtime) before the block-type branch, retaining the no-validate guard as appropriate, and reuse the resulting parsed map in the list validation path. Remove the duplicate parseJSONObject call from the list branch while preserving existing validation and error behavior.
🤖 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/base/app_block_get_data.go`:
- Around line 47-66: Trim app-token and block-id once in both
dryRunAppBlockGetData and executeAppBlockGetData, then reuse those trimmed
values for the path parameters and app-token header. Keep base-token trimming
consistent with the existing behavior and ensure dry-run and execute requests
use identical normalized values.
In `@shortcuts/base/base_skill_contract_test.go`:
- Around line 6-17: Replace the direct os.ReadFile call in
TestBaseSkillContract_ReusedBlockConfigPreservesExplicitIntent with the
applicable internal/vfs read API, using the repository filesystem abstraction
for larkBaseSkillDoc. Remove the now-unused os import while preserving the
existing error handling and test behavior.
In `@shortcuts/base/baseapp_ops.go`:
- Around line 602-611: Update executeAppBlockUpdate to run the same
preconditions as executeAppBlockCreate before the PATCH: validate a non-empty
--name while excluding the current block ID from duplicate-name checks, retrieve
the current block type, and call validateListBaseWorkspace when the target block
is a list block. Add direct update tests asserting rejection of duplicate names
and list data_config bases outside the App workspace.
In `@shortcuts/base/baseapp_test.go`:
- Around line 675-682: Enhance TestNormalizeEntityType’s rejected “sheet” case
by capturing the error and asserting errs.ProblemOf reports the validation
category and subtype, errors.As identifies *errs.ValidationError, and Param
returns “--type”. Also assert the underlying cause is preserved so a plain error
implementation fails.
In `@shortcuts/base/block_data_config.go`:
- Around line 248-285: Replace the partial per-source normalization in the
data_sources handling with calls to the shared normalizeDataConfig function for
each source, preserving non-map entries unchanged. Ensure this applies the
default asc order for group_by sort types group and view, and add a BaseApp test
covering both types when order is absent.
In `@skills/lark-base/references/lark-base-baseapp-block-data-config.md`:
- Around line 59-63: Update the lark-cli base +app-block-update example’s
--data-config filter payload to satisfy the documented one-to-fifty condition
requirement by including a valid filter condition, or use the documented server
representation for clearing a filter if that behavior is intended.
In `@tests/cli_e2e/base/base_baseapp_dryrun_test.go`:
- Around line 221-225: Update the “update requires name or data_config” test to
parse the typed JSON validation envelope from result.Stderr instead of matching
the localized message fragment. Assert error.type, error.subtype, error.param,
and error.message, while preserving the expected validation-stage exit code 2.
---
Nitpick comments:
In `@shortcuts/base/app_block_create.go`:
- Around line 52-68: Hoist the trimmed non-empty data-config parsing using
newParseCtx(runtime) before the block-type branch, retaining the no-validate
guard as appropriate, and reuse the resulting parsed map in the list validation
path. Remove the duplicate parseJSONObject call from the list branch while
preserving existing validation and error behavior.
In `@shortcuts/base/base_execute_test.go`:
- Line 374: Rename the test function from
TestBaseAppListCreateOmitsUnspecifiedOptionalFields to
TestBaseAppBlockCreateListOmitsUnspecifiedOptionalFields to identify the
BaseAppBlockCreate command while preserving the test behavior.
In `@skills/lark-drive/references/lark-drive-delete.md`:
- Around line 54-72: Update the delete command reference to document baseapp
only as a supported --type input and remove internal routing details, including
base/baseapp shortcut normalization, bitable translation, and direct native
Drive API behavior. Keep the user-facing deletion command, required parameters,
and asynchronous polling behavior unchanged.
🪄 Autofix
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 Plus
Run ID: ac300aa8-4802-46e7-ae98-b7726fc95aac
📒 Files selected for processing (42)
internal/cmdutil/dryrun.gointernal/cmdutil/dryrun_test.gointernal/cmdutil/secheader.goshortcuts/base/app_block_create.goshortcuts/base/app_block_get.goshortcuts/base/app_block_get_data.goshortcuts/base/app_block_list.goshortcuts/base/app_block_update.goshortcuts/base/app_list_block_data_config.goshortcuts/base/base_copy.goshortcuts/base/base_execute_test.goshortcuts/base/base_resolve.goshortcuts/base/base_resolve_test.goshortcuts/base/base_shortcuts_test.goshortcuts/base/base_skill_contract_test.goshortcuts/base/baseapp_create.goshortcuts/base/baseapp_get.goshortcuts/base/baseapp_ops.goshortcuts/base/baseapp_page_create.goshortcuts/base/baseapp_page_delete.goshortcuts/base/baseapp_page_get.goshortcuts/base/baseapp_page_list.goshortcuts/base/baseapp_page_rename.goshortcuts/base/baseapp_test.goshortcuts/base/block_data_config.goshortcuts/base/helpers.goshortcuts/base/shortcuts.goshortcuts/base/workspace_create.goshortcuts/base/workspace_entity_list.goshortcuts/base/workspace_move_in.goshortcuts/common/resource_url.goshortcuts/common/resource_url_test.goshortcuts/drive/drive_delete.goshortcuts/drive/drive_delete_test.goskills/lark-base/SKILL.mdskills/lark-base/references/dashboard-block-data-config.mdskills/lark-base/references/lark-base-baseapp-block-data-config.mdskills/lark-base/references/lark-base-baseapp.mdskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-delete.mdtests/cli_e2e/base/base_baseapp_dryrun_test.gotests/cli_e2e/drive/drive_delete_dryrun_test.go
💤 Files with no reviewable changes (1)
- shortcuts/base/helpers.go
ba9b93a to
5c555e0
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
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 `@skills/lark-base/references/lark-base-baseapp.md`:
- Around line 117-119: Update the list guidance and the related sections at the
referenced locations to document the normalized multi-source data_config shape
and its validation constraints, rather than a scalar data_config.base_token.
Preserve the requirement that list sources resolve within one Workspace/BaseApp
context, and document how chart retrieval selects a single base_token when chart
data_config contains multiple sources. Align the examples and terminology with
the implemented normalization behavior in block data_config validation.
🪄 Autofix
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 Plus
Run ID: cca9c8d4-292c-486a-b7ef-53d89ffc48cb
📒 Files selected for processing (42)
internal/cmdutil/dryrun.gointernal/cmdutil/dryrun_test.gointernal/cmdutil/secheader.goshortcuts/base/app_block_create.goshortcuts/base/app_block_get.goshortcuts/base/app_block_get_data.goshortcuts/base/app_block_list.goshortcuts/base/app_block_update.goshortcuts/base/app_list_block_data_config.goshortcuts/base/base_copy.goshortcuts/base/base_execute_test.goshortcuts/base/base_resolve.goshortcuts/base/base_resolve_test.goshortcuts/base/base_shortcuts_test.goshortcuts/base/base_skill_contract_test.goshortcuts/base/baseapp_create.goshortcuts/base/baseapp_get.goshortcuts/base/baseapp_ops.goshortcuts/base/baseapp_page_create.goshortcuts/base/baseapp_page_delete.goshortcuts/base/baseapp_page_get.goshortcuts/base/baseapp_page_list.goshortcuts/base/baseapp_page_rename.goshortcuts/base/baseapp_test.goshortcuts/base/block_data_config.goshortcuts/base/helpers.goshortcuts/base/shortcuts.goshortcuts/base/workspace_create.goshortcuts/base/workspace_entity_list.goshortcuts/base/workspace_move_in.goshortcuts/common/resource_url.goshortcuts/common/resource_url_test.goshortcuts/drive/drive_delete.goshortcuts/drive/drive_delete_test.goskills/lark-base/SKILL.mdskills/lark-base/references/dashboard-block-data-config.mdskills/lark-base/references/lark-base-baseapp-block-data-config.mdskills/lark-base/references/lark-base-baseapp.mdskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-delete.mdtests/cli_e2e/base/base_baseapp_dryrun_test.gotests/cli_e2e/drive/drive_delete_dryrun_test.go
💤 Files with no reviewable changes (1)
- shortcuts/base/helpers.go
🚧 Files skipped from review as they are similar to previous changes (36)
- internal/cmdutil/dryrun.go
- shortcuts/base/block_data_config.go
- shortcuts/base/workspace_create.go
- shortcuts/base/baseapp_get.go
- shortcuts/base/baseapp_page_delete.go
- shortcuts/common/resource_url.go
- shortcuts/base/shortcuts.go
- shortcuts/base/app_block_get.go
- shortcuts/base/baseapp_page_create.go
- shortcuts/base/baseapp_page_get.go
- tests/cli_e2e/drive/drive_delete_dryrun_test.go
- shortcuts/common/resource_url_test.go
- shortcuts/base/base_copy.go
- shortcuts/base/app_block_list.go
- shortcuts/base/base_skill_contract_test.go
- shortcuts/base/workspace_move_in.go
- shortcuts/drive/drive_delete_test.go
- shortcuts/base/app_block_get_data.go
- tests/cli_e2e/base/base_baseapp_dryrun_test.go
- skills/lark-base/references/dashboard-block-data-config.md
- shortcuts/base/app_block_create.go
- shortcuts/drive/drive_delete.go
- skills/lark-drive/SKILL.md
- shortcuts/base/app_block_update.go
- internal/cmdutil/dryrun_test.go
- shortcuts/base/base_resolve.go
- shortcuts/base/baseapp_test.go
- shortcuts/base/base_execute_test.go
- skills/lark-base/SKILL.md
- shortcuts/base/baseapp_page_list.go
- skills/lark-base/references/lark-base-baseapp-block-data-config.md
- shortcuts/base/base_shortcuts_test.go
- skills/lark-drive/references/lark-drive-delete.md
- shortcuts/base/base_resolve_test.go
- shortcuts/base/app_list_block_data_config.go
- shortcuts/base/baseapp_ops.go
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/base/app_block_get_data.go (1)
60-66: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
runtime.CallAPITypedfor this read.This path bypasses the typed API error conversion by returning errors from
runtime.DoAPIandruntime.ClassifyAPIResponsedirectly. Route the request throughruntime.CallAPITyped, or useerrclass.BuildAPIErrorfor mapped failures, so transport and non-zero API errors satisfy the CLI error contract. Add tests for transport failures and non-zero API responses.🤖 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/base/app_block_get_data.go` around lines 60 - 66, Update the read flow containing runtime.DoAPI and runtime.ClassifyAPIResponse to use runtime.CallAPITyped so transport failures and non-zero API responses are converted to the CLI error contract; alternatively apply errclass.BuildAPIError consistently for mapped failures. Add coverage for both transport errors and non-zero API responses.Source: Coding guidelines
shortcuts/base/baseapp_ops.go (1)
133-139: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve the workspace response shape.
executeWorkspaceCreatereturns{"workspace": data, "created": true}while the created workspace result is documented as readable viaworkspace_tokenandurl. Keep the API response shape for machine consumers or add an explicitcreatedcontract.🤖 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/base/baseapp_ops.go` around lines 133 - 139, Update executeWorkspaceCreate to preserve the documented workspace response contract: ensure the returned workspace payload exposes workspace_token and url while retaining compatibility for machine consumers, or explicitly define and implement the created response contract. Keep the successful creation indicator consistent with the chosen API shape.
🤖 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/base/app_block_get_data.go`:
- Around line 60-66: Update the read flow containing runtime.DoAPI and
runtime.ClassifyAPIResponse to use runtime.CallAPITyped so transport failures
and non-zero API responses are converted to the CLI error contract;
alternatively apply errclass.BuildAPIError consistently for mapped failures. Add
coverage for both transport errors and non-zero API responses.
In `@shortcuts/base/baseapp_ops.go`:
- Around line 133-139: Update executeWorkspaceCreate to preserve the documented
workspace response contract: ensure the returned workspace payload exposes
workspace_token and url while retaining compatibility for machine consumers, or
explicitly define and implement the created response contract. Keep the
successful creation indicator consistent with the chosen API shape.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5a718d76-62ff-46c7-baee-26746876d295
📒 Files selected for processing (9)
shortcuts/base/app_block_get_data.goshortcuts/base/base_execute_test.goshortcuts/base/base_skill_contract_test.goshortcuts/base/baseapp_ops.goshortcuts/base/baseapp_test.goshortcuts/base/workspace_create.goskills/lark-base/SKILL.mdskills/lark-base/references/lark-base-baseapp.mdtests/cli_e2e/base/base_baseapp_dryrun_test.go
💤 Files with no reviewable changes (2)
- tests/cli_e2e/base/base_baseapp_dryrun_test.go
- skills/lark-base/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (5)
- shortcuts/base/workspace_create.go
- shortcuts/base/base_skill_contract_test.go
- shortcuts/base/base_execute_test.go
- skills/lark-base/references/lark-base-baseapp.md
- shortcuts/base/baseapp_test.go
|
Addressed the latest outside-diff findings in commit 7c7fddb:
Also resolved the remaining inline review threads. The multi-source-list suggestion was not applied because the current list protocol intentionally accepts one scalar data_config.base_token and at most one Base in the same Workspace. |
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/base/base_execute_test.go`:
- Around line 315-318: Update the test around errs.ProblemOf(err) to retain the
original connection-reset transport error in a variable, assert the returned
error wraps it with errors.Is, and verify problem.Subtype is populated in
addition to CategoryNetwork.
🪄 Autofix
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 Plus
Run ID: 9b56f1c1-3c8a-40fb-a0ed-677c3b1a4425
📒 Files selected for processing (9)
shortcuts/base/app_block_create.goshortcuts/base/app_block_get_data.goshortcuts/base/base_execute_test.goshortcuts/base/base_skill_contract_test.goshortcuts/base/baseapp_ops.goshortcuts/base/baseapp_test.goshortcuts/base/block_data_config.goskills/lark-base/references/lark-base-baseapp-block-data-config.mdtests/cli_e2e/base/base_baseapp_dryrun_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
- shortcuts/base/app_block_create.go
- tests/cli_e2e/base/base_baseapp_dryrun_test.go
- shortcuts/base/base_skill_contract_test.go
- shortcuts/base/baseapp_test.go
- skills/lark-base/references/lark-base-baseapp-block-data-config.md
- shortcuts/base/baseapp_ops.go
- shortcuts/base/block_data_config.go
7991c84 to
e807937
Compare
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@c97af9f6865fe94c9628fc0cb1f47e8aec5f5ecd🧩 Skill updatenpx skills add xiaomi-bytedance/cli#codex/baseapp-cli-upstream-pr-v2 -y -g |
AppMode 的文本组件此前对外叫 richText,发送时再映射成 wire 上的 text, 而读取方向没有反向映射:写进去用 richText、读回来是 text,同一个 CLI 表面自相矛盾,回填或幂等复建时会被枚举校验拒掉。 统一成 text,与 Dashboard 文本组件同名同义: - appBlockTypes/isAppBlockType/textBlockTypes 去掉 richText - 删除 appBlockBody 里的 richText → text 发送期映射 - help、枚举、示例、tips 与 baseapp block data_config reference 同步 - 新增回归测试,确保 richText 不再被接受也不再出现在枚举里 richText 不保留别名:+app-* 尚未随已发布版本对外,无存量调用方。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Migrate the net changes from bitable/base_cli!1308 onto the current BaseApp development branch.
Move the shortcut off the dashboard route and onto the dedicated BaseApp block-data endpoint: - URL: /open-apis/base/v3/base_apps/:app_token/blocks/:block_id/data - base_token is passed as a required query parameter per the new IDL - Refresh --block-id description and tips to list all producers of the chart_token (create/list/get) and note the cht… prefix - Update the dryrun test to expect the new URL Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Text blocks have no /data endpoint; calling +app-block-get-data on one returns a generic server 500. Point readers at +app-block-get, whose data_config.text carries the Markdown source. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1ea0a0a to
c97af9f
Compare
Summary
Add BaseApp application-mode support to the Base CLI, including Workspace, Page, and supported Block operations. Keep the public shortcut contracts and agent-facing skill guidance aligned with the BaseApp API protocol.
This PR preserves the development commits and authors after rebasing them onto the latest upstream
main; the final commit contains only upstream conflict-resolution and scope-cleanup adjustments.Changes
Test Plan
make unit-testgo vet ./...gofmt -l .produces no outputgo mod tidyproduces nogo.mod/go.sumchangesgolangci-lint v2.1.6 run --new-from-rev=upstream/mainreports 0 issuesRelated Issues
Summary by CodeRabbit
New Features
Documentation
Bug Fixes