feat(base): support button field workflow binding - #2234
Open
wanghm25 wants to merge 1 commit into
Open
Conversation
Normalize friendly button field JSON for +field-create, +field-update, and +base-create --fields, add button-specific validation and readback guidance, and cover the workflow-first flow with dry-run and live E2E tests. Co-authored-by: TRAE CLI <noreply@bytedance.com>
|
wanghaomin seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
github-actions
Bot
added
domain/base
📝 WalkthroughWalkthroughButton fields now support friendly JSON for creation and updates. The CLI normalizes button and automation trigger data into the low-level API schema, validates workflow bindings, detects button results, recommends readback, and documents the workflow. ChangesButton field support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant WorkflowAPI
participant BaseFieldAPI
participant FieldReadback
CLI->>WorkflowAPI: Create workflow
CLI->>BaseFieldAPI: Create button field with workflow_id
BaseFieldAPI-->>CLI: Return button field
CLI->>FieldReadback: Poll and validate configuration
CLI->>WorkflowAPI: Create replacement workflow
CLI->>BaseFieldAPI: Update complete button definition
BaseFieldAPI-->>CLI: Return updated field
CLI->>FieldReadback: Validate workflow binding
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
shortcuts/base/table_ops.go (1)
174-186: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared normalization loop and include the item index in both placeholders.
Lines 174-186 repeat the loop in
buildTableCreateBodyat lines 147-157. Only the error reporting differs. Line 177 also omits the item index, while line 182 includes it. A caller cannot tell which item is not an object.Extract one helper that normalizes the slice and returns an indexed error. Then both callers format that error for their own output channel.
♻️ Proposed refactor
+func normalizeFieldItems(fieldItems []interface{}) error { + for idx, item := range fieldItems { + fieldBody, ok := item.(map[string]interface{}) + if !ok { + return fmt.Errorf("item %d must be an object", idx+1) + } + normalized, err := normalizeFieldBody(fieldBody) + if err != nil { + return fmt.Errorf("item %d: %s", idx+1, err.Error()) + } + fieldItems[idx] = normalized + } + return nil +}- for idx, item := range fieldItems { - fieldBody, ok := item.(map[string]interface{}) - if !ok { - body["fields"] = "<invalid_fields_json>" - return body - } - normalized, normalizeErr := normalizeFieldBody(fieldBody) - if normalizeErr != nil { - body["fields"] = fmt.Sprintf("<invalid_fields_json: item %d: %s>", idx+1, normalizeErr.Error()) - return body - } - fieldItems[idx] = normalized - } + if err := normalizeFieldItems(fieldItems); err != nil { + body["fields"] = fmt.Sprintf("<invalid_fields_json: %s>", err.Error()) + return body + }Apply the matching change in
buildTableCreateBodywithbaseValidationErrorf("--fields %s", err.Error()).🤖 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/table_ops.go` around lines 174 - 186, Extract the duplicated field normalization loop from buildTableCreateBody and the shown caller into one helper that normalizes each item and returns an error containing its 1-based index for both invalid object types and normalization failures. Update both callers to format the shared error through their existing output channels, including buildTableCreateBody’s baseValidationErrorf("--fields %s", err.Error()) path, and ensure both invalid_fields_json placeholders include the item index.
🤖 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/field_ops.go`:
- Around line 357-396: Update normalizeButtonFieldBody to preserve all
caller-supplied trigger data when rebuilding property.trigger, including nested
trigger.config keys, or reject unsupported keys with baseValidationErrorf; do
not silently discard them. Also handle existing property.button and
property.trigger values without overwriting unhonored input—preserve compatible
values or return a typed validation error.
In `@shortcuts/base/helpers_test.go`:
- Around line 244-286: The normalizeFieldBody error tests must assert typed
metadata and preserve causes instead of checking only message text. Extend the
table with a missing or empty name case covering normalizeButtonFieldBody, then
use errs.ProblemOf in each subtest to verify the expected category, subtype, and
param, while also asserting the underlying cause is preserved.
---
Nitpick comments:
In `@shortcuts/base/table_ops.go`:
- Around line 174-186: Extract the duplicated field normalization loop from
buildTableCreateBody and the shown caller into one helper that normalizes each
item and returns an error containing its 1-based index for both invalid object
types and normalization failures. Update both callers to format the shared error
through their existing output channels, including buildTableCreateBody’s
baseValidationErrorf("--fields %s", err.Error()) path, and ensure both
invalid_fields_json placeholders include the item index.
🪄 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: 3c70a81a-5696-407a-99b9-cb66bb31a670
📒 Files selected for processing (13)
shortcuts/base/base_execute_test.goshortcuts/base/field_create.goshortcuts/base/field_ops.goshortcuts/base/field_update.goshortcuts/base/helpers_test.goshortcuts/base/table_ops.goskills/lark-base/references/lark-base-field-create.mdskills/lark-base/references/lark-base-field-json.mdskills/lark-base/references/lark-base-field-update.mdtests/cli_e2e/base/base_button_field_workflow_test.gotests/cli_e2e/base/base_create_dryrun_test.gotests/cli_e2e/base/base_field_dryrun_test.gotests/cli_e2e/base/base_field_update_dryrun_test.go
Comment on lines
+357
to
+396
| trigger, ok := body["trigger"].(map[string]interface{}) | ||
| if !ok { | ||
| return nil, baseValidationErrorf("button field requires object \"trigger\"") | ||
| } | ||
| trigger = cloneMap(trigger) | ||
| triggerType := normalizeFieldType(common.GetString(trigger, "type")) | ||
| if triggerType == "" { | ||
| triggerType = "automation" | ||
| } | ||
| if triggerType != "automation" { | ||
| return nil, baseValidationErrorf("button field only supports trigger.type %q", "automation") | ||
| } | ||
| workflowID := strings.TrimSpace(common.GetString(trigger, "workflow_id")) | ||
| if workflowID == "" { | ||
| config, ok := trigger["config"].(map[string]interface{}) | ||
| if ok { | ||
| workflowID = strings.TrimSpace(common.GetString(config, "id")) | ||
| } | ||
| } | ||
| if workflowID == "" { | ||
| return nil, baseValidationErrorf("button field requires non-empty string \"trigger.workflow_id\"") | ||
| } | ||
|
|
||
| normalized := cloneMap(body) | ||
| normalized["type"] = buttonFieldLowLevelType | ||
| normalized["fieldUIType"] = buttonFieldUIType | ||
| delete(normalized, "trigger") | ||
| delete(normalized, "button") | ||
|
|
||
| property, _ := normalized["property"].(map[string]interface{}) | ||
| property = cloneMap(property) | ||
| if property == nil { | ||
| property = map[string]interface{}{} | ||
| } | ||
| property["button"] = button | ||
| property["trigger"] = map[string]interface{}{ | ||
| "type": buttonTriggerAutomation, | ||
| "config": map[string]interface{}{"id": workflowID}, | ||
| } | ||
| normalized["property"] = property |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve or reject unknown trigger keys instead of dropping them.
normalizeButtonFieldBody rebuilds property.trigger from scratch with only type and config.id. Any other key the caller supplied under trigger (for example trigger.config.extra, or a future trigger.mode) is discarded without an error. The same applies to a caller-supplied property.button or property.trigger, which lines 391-395 overwrite silently.
The repository guideline requires a typed validation error when a requested behavior cannot be honored, instead of discarding writes. Reject unknown keys under trigger with baseValidationErrorf, or copy them through.
🛠️ Example: reject unrecognized trigger keys
trigger = cloneMap(trigger)
+ for key := range trigger {
+ switch key {
+ case "type", "workflow_id", "config":
+ default:
+ return nil, baseValidationErrorf("button field does not support trigger key %q", key)
+ }
+ }
triggerType := normalizeFieldType(common.GetString(trigger, "type"))Based on the coding guideline: "When transcribing input or transforming requests, preserve values faithfully; never silently coerce unsupported inputs, ignore unhonored options, default missing identities, or discard writes."
🤖 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/field_ops.go` around lines 357 - 396, Update
normalizeButtonFieldBody to preserve all caller-supplied trigger data when
rebuilding property.trigger, including nested trigger.config keys, or reject
unsupported keys with baseValidationErrorf; do not silently discard them. Also
handle existing property.button and property.trigger values without overwriting
unhonored input—preserve compatible values or return a typed validation error.
Source: Coding guidelines
Comment on lines
+244
to
+286
| tests := []struct { | ||
| name string | ||
| body map[string]interface{} | ||
| want string | ||
| }{ | ||
| { | ||
| name: "missing button", | ||
| body: map[string]interface{}{ | ||
| "name": "同步到 CRM", | ||
| "type": "button", | ||
| "trigger": map[string]interface{}{"workflow_id": "wkf_sync"}, | ||
| }, | ||
| want: `requires object "button"`, | ||
| }, | ||
| { | ||
| name: "missing workflow_id", | ||
| body: map[string]interface{}{ | ||
| "name": "同步到 CRM", | ||
| "type": "button", | ||
| "button": map[string]interface{}{}, | ||
| "trigger": map[string]interface{}{"type": "automation"}, | ||
| }, | ||
| want: `requires non-empty string "trigger.workflow_id"`, | ||
| }, | ||
| { | ||
| name: "unsupported trigger type", | ||
| body: map[string]interface{}{ | ||
| "name": "同步到 CRM", | ||
| "type": "button", | ||
| "button": map[string]interface{}{}, | ||
| "trigger": map[string]interface{}{"type": "url", "workflow_id": "wkf_sync"}, | ||
| }, | ||
| want: `only supports trigger.type "automation"`, | ||
| }, | ||
| } | ||
| for _, tc := range tests { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| _, err := normalizeFieldBody(tc.body) | ||
| if err == nil || !strings.Contains(err.Error(), tc.want) { | ||
| t.Fatalf("err=%v, want substring %q", err, tc.want) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Assert typed error metadata, and add a case for the missing name.
The table cases check only message substrings. The repository guideline requires error-path tests to assert category, subtype, and param through errs.ProblemOf, and to verify cause preservation. baseValidationErrorf produces a typed error, so a revert that changes the error category would still pass this test.
normalizeButtonFieldBody also rejects an empty name at field_ops.go line 339. No case covers that branch.
💚 Proposed additions
{
+ name: "missing name",
+ body: map[string]interface{}{
+ "type": "button",
+ "button": map[string]interface{}{},
+ "trigger": map[string]interface{}{"workflow_id": "wkf_sync"},
+ },
+ want: `requires non-empty string "name"`,
+ },
+ {
name: "missing button", _, err := normalizeFieldBody(tc.body)
if err == nil || !strings.Contains(err.Error(), tc.want) {
t.Fatalf("err=%v, want substring %q", err, tc.want)
}
+ problem, ok := errs.ProblemOf(err)
+ if !ok || problem.Category != errs.CategoryValidation {
+ t.Fatalf("problem=%#v ok=%v, want validation category", problem, ok)
+ }Based on the coding guideline: "Error-path tests must assert typed metadata through errs.ProblemOf (category, subtype, and param) and verify cause preservation rather than relying only on message substrings."
🤖 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/helpers_test.go` around lines 244 - 286, The
normalizeFieldBody error tests must assert typed metadata and preserve causes
instead of checking only message text. Extend the table with a missing or empty
name case covering normalizeButtonFieldBody, then use errs.ProblemOf in each
subtest to verify the expected category, subtype, and param, while also
asserting the underlying cause is preserved.
Source: Coding guidelines
This was referenced Aug 10, 2026
zgz2048
reviewed
Aug 11, 2026
| 默认值 / 约束： | ||
| - `button` 必填，结构是 `{title, color?}` | ||
| - `button.title` 默认回退到字段 `name` | ||
| - `button.color` 默认 `0` |
Collaborator
There was a problem hiding this comment.
button 颜色最好完整列出，不然就不要暴露这个配置
zgz2048
reviewed
Aug 11, 2026
| - `link`：必须有 `link_table`，可选 `bidirectional`、`bidirectional_link_field_name`。 | ||
| - `formula`：必须有 `expression`；先读 formula guide，再创建。 | ||
| - `lookup`：必须有 `from`、`select`、`where`；先读 lookup guide，再创建。 | ||
| - `button`：必须有 `button` 和 `trigger`；其中 `trigger.type` 只支持 `automation`，并且必须先创建 Workflow 再写入 `trigger.workflow_id`。 |
Collaborator
There was a problem hiding this comment.
文档里说 button.name 默认会 fallback 到 field name，为什么这里又必须有 button？
zgz2048
reviewed
Aug 11, 2026
| --table-id <table_id> \ | ||
| --json '{"name":"负责人","type":"user","multiple":false,"default_value":[{"$slot":"current_user"}],"description":"用于标记记录的直接负责人；协作约定可参考[团队字段约定](https://example.com/field-spec)"}' | ||
|
|
||
| lark-cli base +workflow-create \ |
Collaborator
There was a problem hiding this comment.
建议移动到 workflow 的文档中，并且在这个文档里引用 workflow 文档。不在这个文档中展示 workflow 相关指令
zgz2048
reviewed
Aug 11, 2026
|
|
||
| - ⚠️ 这是写入操作，执行前必须确认。 | ||
| - ⚠️ 当 `type` 是 `formula` 或 `lookup` 时，先读对应 guide，再创建。 | ||
| - ⚠️ 当 `type` 是 `button` 时，必须先创建 Workflow；不要猜测 `workflow_id`，也不要直接手写底层 `type:3001` 结构。 |
zgz2048
reviewed
Aug 11, 2026
| Object（对象字段）、Button（按钮字段）、Stage（流程字段）暂时都没有被 CLI 支持。这些字段会展示为 `not_support` 字段并被保护：不允许修改，不允许读取内容。 | ||
| Object（对象字段）和 Stage（流程字段）暂时都没有被 CLI 支持。这些字段会展示为 `not_support` 字段并被保护：不允许修改，不允许读取内容。 | ||
|
|
||
| Button（按钮字段）已经支持，按本文档的 friendly JSON 写法创建或更新即可；不要手写底层 `type:3001` / `fieldUIType:"Button"` 结构，除非是在调试服务端原始返回。 |
zgz2048
reviewed
Aug 11, 2026
| --base-token <base_token> \ | ||
| --table-id <table_id> \ | ||
| --field-id <field_id> \ | ||
| --json '{"name":"同步到 CRM","type":"button","button":{"title":"同步到 CRM","color":0},"trigger":{"type":"automation","workflow_id":"wkf_xxx"}}' \ |
Collaborator
There was a problem hiding this comment.
不要放在 field update 的 fewshot 中，button 是低频使用字段
zgz2048
reviewed
Aug 11, 2026
| } | ||
| ``` | ||
|
|
||
| **按钮字段重绑示例** |
Collaborator
There was a problem hiding this comment.
冗余了，field json 文档中有，就不需要在更新文档中重复说明
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
3 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\n- normalize friendly button field JSON into the backend button payload for +field-create, +field-update, and +base-create --fields\n- add button-specific validation, defaults, readback guidance, and docs/help updates\n- cover the workflow-first button field flow with dry-run tests and a live E2E scenario\n\n## Validation\n- go build -o ./lark-cli ./main.go\n- go test ./shortcuts/base -run 'Test(NormalizeButtonFieldBody|NormalizeButtonFieldBodyDefaultsAndErrors|FieldResultTypeRecognizesButtonField|BaseFieldExecuteFieldListAndGetAndCreateAndUpdate|BaseFieldExecuteUpdateButtonUsesLowLevelTriggerPayload)$' -count=1\n- env LARK_CLI_BIN=/data00/home/wanghaomin/.local/state/harness-agent/envs/env-dafb3e1ad5a4/generic-workers/generic-n251-235-140-5d4bed/base-tech-design/traex/01KZBRY7YHC73EHGRTPFABQHNF/worktrees/01kzbry7yhc73ehgrtpfabqhnf/lark/larksuite-cli/lark-cli go test ./tests/cli_e2e/base -run 'TestBase(ButtonFieldWorkflow|FieldCreateDryRunButtonNormalizesTriggerPayload|FieldUpdateButtonDryRunNormalizesTriggerPayload|CreateDryRunFieldsButtonNormalizesTriggerPayload)$' -count=1\n\n## Notes\n- the live workflow E2E is included and skips automatically when tenant credentials are unavailable in the environment
Summary by CodeRabbit
New Features
Documentation
Tests