fix(base): validate workflow message action shapes - #2092
Open
huarenmin13 wants to merge 1 commit into
Open
Conversation
1. Share LarkMessageAction preflight validation across workflow create and update
2. Validate required arrays and optional field types with field-specific typed errors
3. Keep field contracts in the workflow schema and cover unit, dry-run,
and embedded skill delivery paths
说明：
- Deliberately excludes case-specific unsupported-trigger guidance from PR larksuite#2078
```ai-signature
改动范围: Base workflow create/update 预检、LarkMessageAction schema、单测、dry-run E2E 与嵌入 skill 交付测试
思考过程: create 和 update 复用同一校验器，只校验已知消息动作的稳定结构；其他 workflow step 继续由服务端负责，避免不完整的客户端业务解释器
改动原因: 在请求前拦截 receiver/content 缺失和可选字段类型错误，同时移除单 case SOP 与 help 重复事实导致的过拟合方向
Break Change: 行为 breaking | 过去会发送的无效 LarkMessageAction 现在返回 typed validation error
```
Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local>
AI-SHA256: 9d629364e3a6b4db5c842bee23269dd7fdb275d78afbee8747efdaf18abf9979
📝 WalkthroughWalkthroughWorkflow create and update now perform semantic validation for ChangesWorkflow message validation
Estimated code review effort: 3 (Moderate) | ~20 minutes 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 |
github-actions
Bot
added
domain/base
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/base/workflow_message_validation.go (1)
41-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract helpers to remove duplicated required/optional field checks.
The receiver/content blocks (Lines 41-57) and the send_to_everyone/btn_list blocks (Lines 59-79) each repeat the same "assert type, build path, return typed error" pattern with only the field name/type/hint differing.
♻️ Proposed refactor
+func requireNonEmptyArray(data map[string]interface{}, dataPath, field string) error { + path := dataPath + "." + field + arr, ok := data[field].([]interface{}) + if !ok || len(arr) == 0 { + return workflowMessageActionShapeError( + path, + "a non-empty JSON array", + fmt.Sprintf("Set %s to a non-empty JSON array.", path), + ) + } + return nil +} + receiverPath := dataPath + ".receiver" - if receiver, ok := data["receiver"].([]interface{}); !ok || len(receiver) == 0 { - return workflowMessageActionShapeError( - receiverPath, - "a non-empty JSON array", - fmt.Sprintf("Set %s to a non-empty JSON array.", receiverPath), - ) + if err := requireNonEmptyArray(data, dataPath, "receiver"); err != nil { + return err } - contentPath := dataPath + ".content" - if content, ok := data["content"].([]interface{}); !ok || len(content) == 0 { - return workflowMessageActionShapeError( - contentPath, - "a non-empty JSON array", - fmt.Sprintf("Set %s to a non-empty JSON array.", contentPath), - ) + if err := requireNonEmptyArray(data, dataPath, "content"); err != nil { + return err }A similar helper (accepting a
func(interface{}) booltype-check and hint text) could unify thesend_to_everyone/btn_listoptional-field blocks.🤖 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/workflow_message_validation.go` around lines 41 - 79, Extract reusable validation helpers for the required receiver/content checks and optional send_to_everyone/btn_list checks in the workflow message validation flow. Parameterize each helper with the field name, type predicate or expected type, and hint text, while preserving non-empty-array requirements for receiver/content and allowing an empty btn_list. Update the surrounding validation logic to use these helpers and retain the existing typed errors and paths.
🤖 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.
Nitpick comments:
In `@shortcuts/base/workflow_message_validation.go`:
- Around line 41-79: Extract reusable validation helpers for the required
receiver/content checks and optional send_to_everyone/btn_list checks in the
workflow message validation flow. Parameterize each helper with the field name,
type predicate or expected type, and hint text, while preserving non-empty-array
requirements for receiver/content and allowing an empty btn_list. Update the
surrounding validation logic to use these helpers and retain the existing typed
errors and paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e1543b50-fa00-429f-88f0-b7e561663b97
📒 Files selected for processing (8)
shortcuts/base/workflow_create.goshortcuts/base/workflow_message_validation.goshortcuts/base/workflow_message_validation_test.goshortcuts/base/workflow_update.goskills/lark-base/references/lark-base-workflow-schema.mdtests/cli_e2e/base/coverage.mdtests/cli_e2e/base/workflow_dryrun_test.gotests/cli_e2e/base/workflow_skill_contract_test.go
5 tasks
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
1 participant
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
Add shared preflight validation for the known
LarkMessageActionrequest shape used by workflow create and update. This is a focused replacement for #2078: it keeps reusable schema validation while excluding case-specific unsupported-trigger guidance and duplicated field advice in command help.Changes
LarkMessageAction.dataas an object with non-emptyreceiverandcontentarrays for both create and update.send_to_everyoneandbtn_listoptional, but validate their boolean/array types when supplied.--jsonand the failing indexed path.Test Plan
make unit-testgo vet ./...gofmt -l .go mod tidy(no changes)go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=upstream/mainmake buildskills readE2E confirms the updated embedded workflow schema is deliveredLive workflow E2E was not run; this change is fully covered at the local preflight and dry-run boundaries and does not call the service.
Related Issues
Co-authored-by: BASE Infra Harness ai@base-infra-harness.noreply.local
AI-SHA256: 5367e32fc010a944e95b4109e107077d0b05e0216c2de20004a29c5a22b56bd3
Summary by CodeRabbit
New Features
send_to_everyoneandbtn_listfields.Bug Fixes
Documentation