fix(base): align workflow validation and agent guidance - #2078
fix(base): align workflow validation and agent guidance#2078huarenmin13 wants to merge 16 commits into
Conversation
|
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:
📝 WalkthroughWalkthroughWorkflow create and update now perform structured workflow-definition validation, including message-action fields and unsupported steps. Guidance and schema references describe the updated contracts, while unit and CLI tests cover validation errors, dry-run output, and request-body preservation. ChangesWorkflow contracts and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant BaseWorkflowCreate
participant validateWorkflowDefinition
participant WorkflowAPI
CLI->>BaseWorkflowCreate: submit --json workflow body
BaseWorkflowCreate->>validateWorkflowDefinition: validate workflow definition
validateWorkflowDefinition-->>BaseWorkflowCreate: validation result
BaseWorkflowCreate->>WorkflowAPI: send validated JSON request
Possibly related PRs
Suggested labels: 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 review |
✅ Action performedReview finished.
|
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/base/workflow_execute_test.go`:
- Around line 169-212: The TestBaseWorkflowExecuteUpdatePreservesValidRequests
cases need stronger request-body assertions. In the “valid message action”
subtest, inspect the captured JSON from decodeCapturedJSONBody and verify the
preserved receiver and content values, not just the steps length; in the “empty
steps” subtest, assert that the captured body contains steps as an empty array
while retaining the existing dispatch verification.
In `@tests/cli_e2e/base/workflow_skill_contract_test.go`:
- Around line 32-45: The workflow skill contract test’s contracts table only
covers DeleteRecordTrigger text and must also validate the changed
LarkMessageAction schema. Extend the contracts in the test to assert non-empty
receiver and content fields, while covering optional send_to_everyone and
btn_list fields, using the existing contract assertion flow.
🪄 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 Plus
Run ID: e21b0491-5417-4af6-ac57-8c3831d258f8
📒 Files selected for processing (11)
shortcuts/base/base_shortcuts_test.goshortcuts/base/workflow_create.goshortcuts/base/workflow_execute_test.goshortcuts/base/workflow_get.goshortcuts/base/workflow_message_validation.goshortcuts/base/workflow_update.goskills/lark-base/SKILL.mdskills/lark-base/references/lark-base-workflow-schema.mdtests/cli_e2e/base/url_resolution_skill_contract_test.gotests/cli_e2e/base/workflow_skill_contract_test.gotests/cli_e2e/base/workflow_update_dryrun_test.go
1. Document form-share support in resolver help and recovery hints 2. Assert exact workflow request bodies and LarkMessageAction schema contracts ```ai-signature 改动范围: Base URL resolver 的命令描述、参数提示、错误恢复提示,以及 workflow 请求体和 schema 契约测试 思考过程: 按当前两个 review thread 做最小修复,复用请求捕获辅助函数并比较完整 JSON,避免只验证数组长度而漏掉字段丢失 改动原因: PR larksuite#2078 的帮助文本漏列 form-share,现有测试未完整固定合法请求、空 steps 和文档字段契约 Break Change: 否 ``` Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local> AI-SHA256: 4538bb15e1079d6ca9c48275b205a10ca68a03dad1000c0beff1b5981e29787f
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/base/base_resolve_test.go`:
- Line 234: Update the unrelated Base URL case in the relevant resolve test to
assert the complete typed error contract: verify Category, Subtype, and the
preserved cause using errs.ProblemOf with errors.Is or errors.As, and extract
Param directly from the *errs.ValidationError instead of errs.ProblemOf. Retain
the existing message and Hint assertions.
- Around line 283-285: Add a success-path test for the form-share resolver
alongside the existing BaseURLResolve assertions, exercising a
/share/base/form/<token> request and verifying the returned input_type,
resource_type, and share_token. Keep the description assertion, but ensure the
execution test confirms routing selects the form-share behavior rather than the
generic Base branch.
In `@shortcuts/base/base_resolve.go`:
- Line 18: Update classifyBaseURL so the specific /share/base/form/ pattern is
checked before the generic /base/ pattern, ensuring form-share URLs reach
resolveFormShareURL instead of the generic base resolution. Add a regression
test covering successful classification and resolution of a form-share URL.
🪄 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 Plus
Run ID: 654d0113-d33f-4048-bd8b-d6bbba9569be
📒 Files selected for processing (4)
shortcuts/base/base_resolve.goshortcuts/base/base_resolve_test.goshortcuts/base/workflow_execute_test.gotests/cli_e2e/base/workflow_skill_contract_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/base/workflow_execute_test.go
1. assert exact workflow create request preservation 2. strengthen resolver routing and typed error contracts ```ai-signature 改动范围: shortcuts/base/base_resolve.go、base_resolve_test.go、workflow_execute_test.go 思考过程: 用完整请求体深比较锁住 create 转发;用 resource_type 证明 form-share 真实分支;仅在 URL 解析失败时保留底层 cause 改动原因: 关闭两个 P2 和三条 CodeRabbit 反馈,同时按用户决定将 live E2E 保持为非阻塞项 Break Change: 否 ``` Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local> AI-SHA256: 3a6f6ad6ac1520fe5967c052c8de1f0a31f67facb80767dd52383721975d271b
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_resolve_test.go`:
- Around line 260-265: Strengthen error assertions in
shortcuts/base/base_resolve_test.go at lines 260-265, 415-418, and 429-432: in
the malformed-URL case, verify the unwrapped cause is specifically *url.Error
rather than only non-nil; in both semantic validation cases, assert
CategoryValidation, SubtypeInvalidArgument, and a nil cause using the existing
typed-error assertion patterns.
🪄 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 Plus
Run ID: 01b9a56d-56d5-4371-8208-234c09a60325
📒 Files selected for processing (3)
shortcuts/base/base_resolve.goshortcuts/base/base_resolve_test.goshortcuts/base/workflow_execute_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/base/workflow_execute_test.go
1. require malformed URL failures to preserve a url.Error cause 2. verify title validation metadata and nil semantic causes ```ai-signature 改动范围: shortcuts/base/base_resolve_test.go 思考过程: 按错误来源区分解析失败与语义校验;前者断言具体 url.Error,后者明确断言无底层 cause 改动原因: 关闭 CodeRabbit 增量复审新增的 typed-error 合同测试缺口 Break Change: 否 ``` Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local> AI-SHA256: 16558d9f4339a7d2660d3e398d33ea19ce9eac04538f4fa2f0a5d44a97b11381
1. Propagate the selected alias flag through structured resolver validation errors. 2. Align embedded workflow guidance with title and steps while managing status separately. 3. Cover alias errors and delivered guidance with unit and real-binary regression tests. ```ai-signature 改动范围: shortcuts/base resolver 实现及单元测试,并更新 lark-base workflow 指南与真实二进制 E2E 合同测试。 思考过程: 保留既有请求和解析行为,仅让输入读取结果携带实际 flag 名;指南测试经 LARK_CLI_BIN 执行 skills read,直接覆盖二进制交付面。 改动原因: 隐藏 alias 的结构化错误会误报 canonical 参数,且内嵌指南错误地把 status 放入 create/update 写入契约。 Break Change: 否 ``` Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local> AI-SHA256: b0cf1649b3fb329b4910eff6cbc7d8135cc6e0e34b7bbb17bcca4761c5be9fdc
Keep this PR scoped to DeleteRecordTrigger guidance and LarkMessageAction validation.
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@b6e49aeff132b9e729323ba0cd1b9c616cb716f7🧩 Skill updatenpx skills add huarenmin13/cli#codex/base-workflow-contracts -y -g |
c53cc4c to
3b49167
Compare
3b49167 to
c53cc4c
Compare
1. Move the workflow validation matrix to the shared validator and keep representative create and up
date wiring cases.
2. Reuse existing dry-run and embedded-reference helpers while preserving the 014 and 026 regression
contracts.
```ai-signature
改动范围: shortcuts/base/workflow_execute_test.go 以及两个 Base workflow CLI E2E 测试文件
思考过程: 将字段组合放在共享 validator 层验证,仅保留 create 和 update 的接线回归,并复用仓库现有 dry-run helper,避免重复命令执行样板
改动原因: PR2078 的有效 diff 中测试占比过高,重复断言和环境初始化降低可审阅性,需要在不改变生产行为与两题契约覆盖的前提下压缩
Break Change: 否
```
Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local>
AI-SHA256: 3865fb792cd29489fbafb179fd107cd3949b3235a62b26267266706dc6de7cf9
1. Reject non-object workflow step entries and non-string step types with indexed validation errors.
2. Cover malformed step shapes while preserving unknown string step types for server-side validation
.
```ai-signature
改动范围: shortcuts/base/workflow_message_validation.go 和 shortcuts/base/workflow_execute_test.go
思考过程: 在现有共享 workflow validator 的解析边界返回索引化错误,只收紧畸形 step 结构,不建立封闭类型白名单,也不改变未知字符串 step 的透传
改动原因: 当前 workflowSteps 会把非对象元素和非字符串 type 清零后静默跳过,导致明显畸形输入仍发往服务端,增加代理重试与错误恢复成本
Break Change: 否
```
Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local>
AI-SHA256: 0374d62cc198d61578a71c746b97a20165e3730060d7bcd68179b0fa7c238275
1. Require every workflow step type to be a non-empty string before sending a request. 2. Cover missing, empty, and whitespace-only types while preserving unknown non-empty string types. ```ai-signature 改动范围: shortcuts/base/workflow_message_validation.go 和 shortcuts/base/workflow_execute_test.go 思考过程: 复用共享 workflowSteps 解析边界收紧 type 的结构契约,仅要求非空字符串,不增加类型白名单,也不改变 FutureTrigger 等未来字符串类型的透传行为 改动原因: 当前解析器允许缺失、空串或纯空白 type 以零值继续进入请求,与 workflow schema 中 type 必填且为 string 的契约不一致 Break Change: 否 ``` Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local> AI-SHA256: 9255a69f2935afab109cbfc183a946504a1dc172f52eefc84404a1886affb97c
|
@coderabbitai review |
✅ Action performedReview finished.
|
1. Reject case-variant aliases for validated workflow and message-action fields. 2. Preserve unknown future fields and step types. 3. Add unit and branch-binary dry-run regressions. 4. Sync the Base workflow E2E coverage inventory. ```ai-signature 改动范围: workflow_message_validation.go、workflow_execute_test.go、workflow_update_dryrun_test.go 和 Base E2E coverage.md 思考过程: 将已知契约字段解析改为 map 精确取键,并只拒绝大小写等价别名;未知字段和未来 step type 继续透传,避免把修复扩成严格白名单 改动原因: encoding/json 对 struct 字段进行大小写不敏感匹配,导致错误大小写字段通过校验后按原键外发,可能绕过 workflow full-replacement 防护 Break Change: 否 ``` Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local> AI-SHA256: c466c5e718ec6aa51cf55955fcadc90b151acc20072e5bf2f985bc87c2392a00
1. Keep preflight checks limited to DeleteRecordTrigger and LarkMessageAction contracts 2. Move shared validation and E2E coverage into existing workflow files 3. Restore unrelated update and coverage behavior to the pre-PR baseline ```ai-signature 改动范围: shortcuts/base 的 workflow create/update 校验、既有测试文件与 lark-base workflow 指南 思考过程: 保留 014 和 026 的发送前防线,删除顶层 schema 校验和重复解析,并把原新增文件内容合并到现有文件以缩小有效 diff 改动原因: PR 仅需修复不支持的删除触发器与消息动作缺字段问题,现有实现扩大了契约和文件数量 Break Change: 否 ``` Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local> AI-SHA256: 55da68b4a04ec4c0250365527417936252760795db3044e7a68e2664d33c606b
This reverts commit 13948ee.
Summary
Fix Base workflow cases 014 and 026 without adding a general workflow schema validator.
DeleteRecordTriggeras known unsupported. When a requested capability is unavailable, agents must preserve the original intent, stop writes, and execute alternatives only after explicit user selection.LarkMessageAction.databefore workflow create/update:datamust be an object,receiverandcontentmust be non-empty arrays, and optionalsend_to_everyone/btn_listfields are type-checked when present.Changes
LarkMessageActioncontract and return typed--jsonfield errors before transport.Validation
make buildgo test ./shortcuts/base -count=1go vet ./shortcuts/base ./tests/cli_e2e/basegofmtandgit diff --checkThe opt-in live workflow round-trip was not run.
Related
Base workflow evaluation cases 014 and 026.