Feat/button workflow - #2314
Conversation
|
zhangbinkai.zbk 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds Base button-rule bind, get, and unbind shortcuts. It also adds endpoint-domain overrides and validated extra request headers through environment variables, with tests and updated Base field guidance. ChangesBase button workflow commands
Environment routing and request headers
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ButtonRuleShortcut
participant BaseAPI
CLI->>ButtonRuleShortcut: invoke button-rule command
ButtonRuleShortcut->>ButtonRuleShortcut: validate arguments
ButtonRuleShortcut->>BaseAPI: send GET or PUT request
BaseAPI-->>ButtonRuleShortcut: return workflow rule
ButtonRuleShortcut-->>CLI: print response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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.
Actionable comments posted: 20
🧹 Nitpick comments (8)
env/codex-dev-lark.sh (1)
233-238: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the lane override behavior with
env/claude-dev-lark.sh.Line 235 pins
LARK_LANE="$lane"at generation time. The sibling shim inenv/claude-dev-lark.shline 224 usesLARK_LANE="\${LARK_LANE:-$lane}"and lets the caller override the lane per invocation. Both launchers document the same--laneflag, so the two shims should resolve the lane the same way.♻️ Proposed fix
exec env \\ LARK_CLI_ENV_BIN="$bin_dir" \\ - LARK_LANE="$lane" \\ + LARK_LANE="\${LARK_LANE:-$lane}" \\🤖 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 `@env/codex-dev-lark.sh` around lines 233 - 238, Update the LARK_LANE assignment in the launcher’s exec environment to use the caller-provided LARK_LANE when set, falling back to the generated lane value otherwise, matching the behavior of env/claude-dev-lark.sh. Keep the existing --lane handling and other environment assignments unchanged.internal/envvars/read.go (1)
34-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not reuse
agentNameMaxLenas the header value limit.Line 34 bounds arbitrary header values with
agentNameMaxLen. That constant defines the limit for the agent-name value. The two limits are unrelated. If the agent-name limit changes later, header values are silently accepted or rejected at a different length.Declare a dedicated constant, for example
extraHeaderValueMaxLen, and use it here.🤖 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 `@internal/envvars/read.go` at line 34, In the header-value sanitization path, replace the agentNameMaxLen argument used by sanitizeSingleLine with a dedicated extraHeaderValueMaxLen constant. Define the new limit alongside the existing environment-value limits, keeping the agent-name limit exclusively for agent-name validation.env/larkenv (2)
101-117: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDisable globbing during the unquoted split.
Line 107 iterates
$rawunquoted so thatIFS=';'splits the entries. Bash also applies pathname expansion to that unquoted expansion. If a header value contains*or?, the loop can replace the value with matching filenames from the current directory.♻️ Proposed fix
local item result="" local IFS=';' + local reset_glob=0 + case "$-" in *f*) ;; *) reset_glob=1; set -f ;; esac for item in $raw; do item="${item#"${item%%[![:space:]]*}"}" item="${item%"${item##*[![:space:]]}"}" [ -n "$item" ] || continue [ "$item" = "$target" ] && continue if [ -n "$result" ]; then result="$result; $item" else result="$item" fi done + [ "$reset_glob" -eq 0 ] || set +f🤖 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 `@env/larkenv` around lines 101 - 117, Update remove_extra_header so pathname expansion is disabled while iterating over the unquoted raw value used for semicolon splitting, preventing header characters such as * and ? from expanding to filenames. Preserve the existing trimming, target removal, and result reconstruction behavior.
270-278: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRestore terminal echo if the prompt is interrupted.
Line 272 disables echo with
stty -echo. If the user presses Ctrl-C duringread, line 274 never runs and the terminal stays without echo. Bashread -rshandles the restore itself, including on interrupt.♻️ Proposed fix
printf 'App Secret (输入不回显): ' >&2 - stty -echo 2>/dev/null || true - read -r secret - stty echo 2>/dev/null || true + read -rs secret printf '\n' >&2🤖 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 `@env/larkenv` around lines 270 - 278, Update the secret prompt in the three-argument branch to use Bash’s `read -rs` instead of manually toggling terminal echo with `stty -echo` and `stty echo`. Remove the explicit stty calls while preserving hidden input, interruption-safe echo restoration, newline output, and subsequent `config init --app-secret-stdin` handling.internal/cmdutil/secheader_test.go (1)
265-272: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd an assertion that extra headers cannot replace CLI-owned headers.
The test proves that a new key is added. It does not constrain the case where the extra header collides with a header the CLI sets itself. Add that case together with the fix proposed on
internal/cmdutil/secheader.golines 62-66.💚 Proposed test
func TestBaseSecurityHeaders_ExtraHeadersDoNotOverrideCLIHeaders(t *testing.T) { t.Setenv(envvars.CliExtraHeaders, "X-Cli-Source: spoofed; X-TT-ENV: boe_bitable_bk") h := BaseSecurityHeaders() if got := h.Get(HeaderSource); got != SourceValue { t.Fatalf("%s = %q, want %q", HeaderSource, got, SourceValue) } if got := h.Get("X-TT-ENV"); got != "boe_bitable_bk" { t.Fatalf("X-TT-ENV = %q, want boe_bitable_bk", got) } }🤖 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 `@internal/cmdutil/secheader_test.go` around lines 265 - 272, Extend the security-header tests around BaseSecurityHeaders to cover collisions with CLI-owned headers: configure CliExtraHeaders with both a spoofed HeaderSource and a new X-TT-ENV header, then assert the CLI’s SourceValue remains authoritative while the non-conflicting extra header is preserved. Apply the corresponding protection in BaseSecurityHeaders so extra headers cannot override CLI-set headers.internal/core/types_test.go (1)
76-83: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winExtend the rejection coverage to a table.
The test covers only the
https://prefix. The override guard also rejects/?#@, and it currently accepts values that should be rejected, such as a value with a port. Table-driven negative cases document the intended contract and fail if the guard is loosened.💚 Proposed test
func TestResolveEndpoints_RejectsInvalidEndpointDomainOverride(t *testing.T) { for _, raw := range []string{ "https://open.feishu-boe.cn", "open.feishu-boe.cn/path", "open.feishu-boe.cn?a=b", "user@open.feishu-boe.cn", "open.feishu-boe.cn:8080", " ", } { t.Run(raw, func(t *testing.T) { t.Setenv(envvars.CliEndpointDomain, raw) ep := ResolveEndpoints(BrandFeishu) if ep.Open != "https://open.feishu.cn" { t.Errorf("Open = %q, want default endpoint for override %q", ep.Open, raw) } if ep.Accounts != "https://accounts.feishu.cn" { t.Errorf("Accounts = %q, want default endpoint for override %q", ep.Accounts, raw) } }) } }The
:8080case fails against the current implementation. It documents the gap raised oninternal/core/types.golines 115-123.🤖 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 `@internal/core/types_test.go` around lines 76 - 83, Expand TestResolveEndpoints_RejectsURLAsEndpointDomainOverride into a table-driven negative test covering URL schemes, path, query, userinfo, ports, and whitespace-only overrides; rename it to reflect invalid endpoint domain overrides. For each case, assert ResolveEndpoints(BrandFeishu) falls back to both the default Open and Accounts endpoints, including the port case currently accepted by the guard.env/claude-dev-lark.sh (1)
137-139: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winMake the permission bypass opt-in.
Line 138 always passes
--allow-dangerously-skip-permissions. The script provides no way to launch Claude Code with permission prompts enabled. The sibling launcherenv/codex-dev-lark.shgates the equivalent bypass behind an explicit--cxflag plus a required environment variable (lines 79-82 and 128-131).Add a flag such as
--safeor invert the default, so the bypass requires an explicit opt-in.🤖 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 `@env/claude-dev-lark.sh` around lines 137 - 139, Update launch_claude so Claude starts with permission prompts enabled by default; only add --allow-dangerously-skip-permissions when the user explicitly opts in via a dedicated flag and the required environment-variable guard, following the existing opt-in pattern in the sibling launcher.internal/envvars/read_test.go (1)
147-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the empty result contract.
ExtraHeadersreturnsnilwhen no valid header remains. No test asserts that branch. A revert to returning an empty non-nilhttp.Headerwould pass the current tests.Add a case for an unset variable and a case where every entry is invalid.
💚 Proposed test
func TestExtraHeaders_ReturnsNilWhenNoValidHeaders(t *testing.T) { t.Setenv(CliExtraHeaders, "") if h := ExtraHeaders(); h != nil { t.Fatalf("ExtraHeaders() = %v, want nil for empty value", h) } t.Setenv(CliExtraHeaders, "no-colon; Bad Header: nope; : empty-name") if h := ExtraHeaders(); h != nil { t.Fatalf("ExtraHeaders() = %v, want nil when all entries are invalid", h) } }Based on learnings, every behavior change requires a nearby regression test that fails when the implementation is reverted.
🤖 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 `@internal/envvars/read_test.go` around lines 147 - 160, Add a regression test near TestExtraHeaders_RejectsHeaderInjection covering ExtraHeaders with an empty or unset CliExtraHeaders value and with a value containing only invalid entries; assert the result is nil in both cases, preserving the contract that no valid headers returns nil.Source: Learnings
🤖 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 `@A2A_WEB_PARITY_DISCUSSION.md`:
- Around line 210-217: Update the unknown-event fallback described in the stable
core type/provider extension proposal to avoid forwarding arbitrary raw
structures. Represent unrecognized data with a redacted or opaque extension by
default, and only preserve raw fields after explicit visibility validation and
allowlist checks; apply the same rule to the corresponding section around the
additional referenced content.
- Around line 421-429: Update section “7.2 推荐方案” to distinguish standard A2A/MCP
capabilities from Lark-specific recovery semantics: label
`SubscribeTask(after_cursor=...)` and cursor-expiry recovery as Lark-specific
extensions, explicitly pin the relevant A2A and MCP protocol versions, and
remove any implication that A2A `SubscribeToTask` or MCP Tasks define standard
event replay.
In `@cmd/agents/lark-cli-a2a-web-experience-parity-discussion.md`:
- Around line 419-438: Establish one versioned, normative public event
vocabulary before implementation, centered on the event model in section 5.2.1
or an explicitly selected alternative. Add a compatibility mapping covering
output.*, task.error, task.snapshot, and task.stream_end to the canonical
equivalents, and update the referenced design documents, reducers, and
contract-test expectations to use that mapping consistently.
In `@cmd/agents/task-stream-phase1-design.md`:
- Around line 156-190: 更新“结束事件”及对应错误表，明确 task.stream_end 中 ok
表示传输成功而非任务成功，并增加独立的任务结果字段以区分 completed、failed、rejected、canceled 等状态。为 API
错误、内容安全拦截和写入失败补充终止行为：stdout
仍可写时发送不含敏感信息的错误结束事件；写入失败时说明只能依赖退出码，但将退出码保留为辅助信号而非唯一任务状态来源。
- Around line 242-269: Extend the task-stream design around snapshot
digest/output handling to define explicit maximum serialized snapshot size,
cumulative transfer, and polling/output frequency limits, including oversized
single-message or artifact cases. Ensure the implementation checks these limits
before emitting each snapshot and returns a typed truncation or size error
rather than emitting incomplete state; apply the same safeguards to the later
output path referenced by the comment.
- Around line 114-115: Update the task polling flow described in the design so
--timeout is enforced as a hard observation deadline, using a deadline-aware
context for polling and GetTask plus an interruptible timer during backoff.
Explicitly define whether the initial GetTask may finish after the deadline, and
add coverage where the timeout is shorter than the next backoff interval to
verify prompt termination without cancelling the remote task.
In `@env/codex-dev-lark.sh`:
- Around line 95-99: The `--` handling replaces previously collected forwarding
arguments instead of preserving them. In env/codex-dev-lark.sh lines 95-99,
update the `codex_args` assignment to append the remaining positional arguments;
in env/claude-dev-lark.sh lines 81-86, append to `claude_args` and increment
`claude_arg_count` by the remaining argument count rather than resetting it.
- Around line 136-138: Add *.bak.* to the repository’s .gitignore so timestamped
skill backup files created by link_skill under the existing skill backup paths
are ignored, while preserving the current bin ignore rules.
- Line 263: Update the launcher’s final exec command to safely handle an empty
codex_args array while set -u is enabled, including plain invocation and -- with
no arguments on Bash 3.2. Preserve all existing argument passing behavior when
codex_args contains values, and keep codex_launch_args expansion unchanged.
In `@env/larkenv`:
- Around line 157-176: Confirm whether feishu-boe.cn and feishu-pre.cn are
publicly accessible and approved for inclusion in the public CLI distribution;
if not, remove the hardcoded domains and internal lane values from the
environment switch and source them from an untracked developer-provided
configuration or environment variables, while preserving the boe, pre, ppe, and
online behavior.
In `@internal/cmdutil/secheader.go`:
- Around line 62-66: Update the extra-header application in BaseSecurityHeaders
in internal/cmdutil/secheader.go:62-66 to skip keys already present in h using
canonical header names, and use h.Add for each value so multi-value headers are
preserved. Add a test case in internal/cmdutil/secheader_test.go:265-272
covering a colliding X-Cli-Source header whose CLI value remains intact and a
non-colliding header that is still added.
In `@internal/core/types.go`:
- Line 8: Remove the direct os.Getenv usage from ResolveEndpoints in
internal/core and add an envvars.EndpointDomain() string accessor that reads and
validates envvars.CliEndpointDomain alongside ExtraHeaders and the other
environment accessors. Update ResolveEndpoints to use EndpointDomain(), reusing
the existing validation machinery and keeping environment ownership within
internal/envvars.
- Around line 115-123: Strengthen endpointDomainOverride validation by rejecting
any value containing “:” and requiring a dotted domain whose labels each match
[a-z0-9]([a-z0-9-]*[a-z0-9])?. Preserve the existing trimming, lowercasing,
URL-delimiter rejection, and trailing-dot normalization while ensuring invalid
values such as ports, spaces, and empty labels return an empty override.
- Around line 106-111: Harden endpoint domain overrides across
internal/core/types.go: at lines 106-111, prevent endpointDomainOverride from
redirecting Accounts and ensure override-derived hosts are excluded from
platformEndpointHosts; at lines 115-123, replace the denylist validation with
positive domain-shape validation or an explicit development-domain allowlist; at
internal/core/types_test.go lines 76-83, convert the rejection test into a table
covering ports, inner spaces, and single-label values.
In `@shortcuts/base/workflow_execute_test.go`:
- Around line 190-196: Update
TestBaseButtonRuleValidateRejectsInternalWorkflowID to assert that the returned
error is an *errs.ValidationError, then verify its invalid-argument subtype and
associated parameter is --workflow-id. Replace the message-only validation while
preserving the existing rejection scenario and ensure the validation cause
metadata is checked rather than relying on matching error text.
- Around line 136-151: The tests in shortcuts/base/workflow_execute_test.go at
lines 136-151 and 172-188 only validate response output; update both
TestBaseButtonRuleExecuteBind and the corresponding unbind test to assert the
outbound PUT payload, requiring workflow_id "wkf_1" for bind and workflow_id ""
for unbind.
In `@shortcuts/base/workspace.go`:
- Around line 12-112: Add self-contained live E2E coverage for
BaseWorkspaceCreate, BaseWorkspaceEntityList, BaseWorkspaceEntityAdd, and
BaseWorkspaceEntityRemove, covering the complete create, list, add, and remove
workflow. Introduce an exposed cleanup shortcut or equivalent cleanup workflow
for the workspace created by the test, and ensure cleanup runs even when earlier
assertions or operations fail so no persistent workspace state is leaked.
In `@skills/lark-base/references/baseapp-protocol-design.md`:
- Line 731: Remove the developer-specific local path and internal code.byted.org
host from the backend-core evidence description in the referenced documentation
section. Replace it with repository-neutral wording such as “backend-core source
review,” without changing the surrounding evidence or contract content.
In `@skills/lark-base/SKILL.md`:
- Line 153: Update the routing section in SKILL.md to add entries for
+button-rule-bind, +button-rule-get, and +button-rule-unbind, directing these
requests to lark-base-field-json.md and its field-creation sequence. Keep the
existing button-field recovery guidance unchanged and ensure the new entries
provide domain routing and cross-command workflow coverage.
In `@tests/cli_e2e/base/base_button_rule_dryrun_test.go`:
- Around line 15-79: Extend TestBaseButtonRuleDryRun with self-contained live
E2E coverage for the bind, get, and unbind shortcuts, following the HTTP-mock
setup used by workflow_execute_test.go. Create the required base, table, field,
and workflow fixtures, verify get returns the bound workflow and then confirms
the field is unbound, and register cleanup for every created resource so
teardown runs even when assertions fail.
---
Nitpick comments:
In `@env/claude-dev-lark.sh`:
- Around line 137-139: Update launch_claude so Claude starts with permission
prompts enabled by default; only add --allow-dangerously-skip-permissions when
the user explicitly opts in via a dedicated flag and the required
environment-variable guard, following the existing opt-in pattern in the sibling
launcher.
In `@env/codex-dev-lark.sh`:
- Around line 233-238: Update the LARK_LANE assignment in the launcher’s exec
environment to use the caller-provided LARK_LANE when set, falling back to the
generated lane value otherwise, matching the behavior of env/claude-dev-lark.sh.
Keep the existing --lane handling and other environment assignments unchanged.
In `@env/larkenv`:
- Around line 101-117: Update remove_extra_header so pathname expansion is
disabled while iterating over the unquoted raw value used for semicolon
splitting, preventing header characters such as * and ? from expanding to
filenames. Preserve the existing trimming, target removal, and result
reconstruction behavior.
- Around line 270-278: Update the secret prompt in the three-argument branch to
use Bash’s `read -rs` instead of manually toggling terminal echo with `stty
-echo` and `stty echo`. Remove the explicit stty calls while preserving hidden
input, interruption-safe echo restoration, newline output, and subsequent
`config init --app-secret-stdin` handling.
In `@internal/cmdutil/secheader_test.go`:
- Around line 265-272: Extend the security-header tests around
BaseSecurityHeaders to cover collisions with CLI-owned headers: configure
CliExtraHeaders with both a spoofed HeaderSource and a new X-TT-ENV header, then
assert the CLI’s SourceValue remains authoritative while the non-conflicting
extra header is preserved. Apply the corresponding protection in
BaseSecurityHeaders so extra headers cannot override CLI-set headers.
In `@internal/core/types_test.go`:
- Around line 76-83: Expand
TestResolveEndpoints_RejectsURLAsEndpointDomainOverride into a table-driven
negative test covering URL schemes, path, query, userinfo, ports, and
whitespace-only overrides; rename it to reflect invalid endpoint domain
overrides. For each case, assert ResolveEndpoints(BrandFeishu) falls back to
both the default Open and Accounts endpoints, including the port case currently
accepted by the guard.
In `@internal/envvars/read_test.go`:
- Around line 147-160: Add a regression test near
TestExtraHeaders_RejectsHeaderInjection covering ExtraHeaders with an empty or
unset CliExtraHeaders value and with a value containing only invalid entries;
assert the result is nil in both cases, preserving the contract that no valid
headers returns nil.
In `@internal/envvars/read.go`:
- Line 34: In the header-value sanitization path, replace the agentNameMaxLen
argument used by sanitizeSingleLine with a dedicated extraHeaderValueMaxLen
constant. Define the new limit alongside the existing environment-value limits,
keeping the agent-name limit exclusively for agent-name 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: f04d9270-6d77-4fcd-a728-05350d30cbfa
⛔ Files ignored due to path filters (2)
.codex-dev/tmp/boe-base-button-auth-20260812-2.pngis excluded by!**/*.png.codex-dev/tmp/boe-base-button-auth.pngis excluded by!**/*.png
📒 Files selected for processing (53)
.agents/skills/lark-approval.agents/skills/lark-apps.agents/skills/lark-attendance.agents/skills/lark-base.agents/skills/lark-calendar.agents/skills/lark-contact.agents/skills/lark-doc.agents/skills/lark-drive.agents/skills/lark-event.agents/skills/lark-im.agents/skills/lark-mail.agents/skills/lark-markdown.agents/skills/lark-minutes.agents/skills/lark-note.agents/skills/lark-okr.agents/skills/lark-openapi-explorer.agents/skills/lark-shared.agents/skills/lark-sheets.agents/skills/lark-skill-maker.agents/skills/lark-slides.agents/skills/lark-task.agents/skills/lark-vc.agents/skills/lark-vc-agent.agents/skills/lark-whiteboard.agents/skills/lark-wiki.agents/skills/lark-workflow-meeting-summary.agents/skills/lark-workflow-standup-reportA2A_WEB_PARITY_DISCUSSION.mdcmd/agents/lark-cli-a2a-web-experience-parity-discussion.mdcmd/agents/task-stream-phase1-design.mdenv/claude-dev-lark.shenv/codex-dev-lark.shenv/larkenvinternal/cmdutil/secheader.gointernal/cmdutil/secheader_test.gointernal/core/types.gointernal/core/types_test.gointernal/envvars/envvars.gointernal/envvars/read.gointernal/envvars/read_test.goshortcuts/base/base_dryrun_ops_test.goshortcuts/base/base_shortcuts_test.goshortcuts/base/button_rule.goshortcuts/base/shortcuts.goshortcuts/base/workflow_execute_test.goshortcuts/base/workspace.goshortcuts/base/workspace_ops.goshortcuts/base/workspace_test.goskills/lark-base/SKILL.mdskills/lark-base/references/baseapp-protocol-design.mdskills/lark-base/references/lark-base-field-json.mdtests/cli_e2e/base/base_button_rule_dryrun_test.gotests/cli_e2e/base/base_workspace_dryrun_test.go
| #### 方案二：稳定核心类型 + provider extension | ||
|
|
||
| 这是推荐方案。公共协议只定义少量稳定的生命周期和内容类型： | ||
|
|
||
| - 生命周期由 `output.created/delta/updated/completed` 表达； | ||
| - 内容使用 `text`、`markdown`、`table`、`chart`、`file`、`form`、`action`、`data` 等通用类型； | ||
| - Base 或其他 provider 的特殊信息放在带命名空间的 `extensions` 中； | ||
| - 对暂未认识的类型，保留原始结构并生成可读 fallback，不能静默丢弃。 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not forward arbitrary raw data for unknown public events.
The proposed fallback preserves the original structure for unknown types. This can expose internal tool parameters, prompts, resource identifiers, or other sensitive fields.
Use a redacted or opaque extension for unknown data. Permit raw data only after an explicit visibility and allowlist check.
Also applies to: 254-270
🤖 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 `@A2A_WEB_PARITY_DISCUSSION.md` around lines 210 - 217, Update the
unknown-event fallback described in the stable core type/provider extension
proposal to avoid forwarding arbitrary raw structures. Represent unrecognized
data with a redacted or opaque extension by default, and only preserve raw
fields after explicit visibility validation and allowlist checks; apply the same
rule to the corresponding section around the additional referenced content.
| ### 7.2 推荐方案 | ||
|
|
||
| - Task 快照补充 `status_message`、`progress`、`error`、`last_event_cursor`； | ||
| - error 至少包含稳定 `code`、`category`、`retryable`、`hint` 和必要的服务端 ground truth； | ||
| - `GetTask` 永远提供权威当前状态，事件流用于低延迟更新； | ||
| - `SubscribeTask(after_cursor=...)` 用于恢复；游标过期时返回明确错误和最新快照； | ||
| - context list 直接提供 `active_task_id`、`awaiting_input`、`last_state` 等一致的 rollup； | ||
| - 对回答、确认、取消等有副作用的动作使用 idempotency key； | ||
| - CodeAgent 在重试前先读回当前任务状态，避免重复执行。 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- referenced document sections ---'
sed -n '400,530p' A2A_WEB_PARITY_DISCUSSION.md
printf '%s\n' '--- all protocol references in the document ---'
rg -n -C 2 'A2A|MCP|SubscribeToTask|after_cursor|cursor|2025-11-25|whats-new' A2A_WEB_PARITY_DISCUSSION.mdRepository: larksuite/cli
Length of output: 260
🌐 Web query:
A2A Protocol v1 whats new SubscribeToTask cursor pagination task listing after_cursor
💡 Result:
The A2A Protocol v1.0, an open standard for communication between AI agents, introduced several key changes to task management, specifically regarding task subscriptions and pagination [1][2][3]. SubscribeToTask In v1.0, the task subscription operation was officially renamed from tasks/resubscribe to SubscribeToTask [1]. This method establishes a streaming connection to receive updates for a specific task that is not in a terminal state [4][5]. Key updates to this method include: - Formal specification of the streaming subscription lifecycle [1]. - Clarified behavior for stream closure when a task reaches a terminal state (completed, failed, canceled, or rejected) [4][1]. - Support for multiple concurrent subscriptions per task [1]. - The operation is required to return the current state of the task as the first event in the stream, which avoids potential information loss between a standard GetTask call and the start of the subscription [4][5]. Task Listing and Cursor Pagination A major change in v1.0 is the mandate to use cursor-based pagination for the ListTasks operation to ensure scalability and performance [1][5]. This replaces the legacy page-based (offset) pagination used in previous versions (e.g., v0.3.0) [1]. - Mechanism: The API uses a pageToken (request) and next_page_token (response) to navigate results [4][6]. - Sorting: Implementations must return tasks sorted by their status timestamp in descending order (most recently updated tasks first) [6][7]. - Implementation Note: Clients are expected to store the next_page_token from the response to retrieve subsequent pages in a loop, rather than relying on page numbers [1]. These changes were part of a broader effort in v1.0 to improve protocol maturity, clarify specifications, and address enterprise deployment requirements [1][3].
Citations:
- 1: https://a2a-protocol.org/latest/whats-new-v1/
- 2: https://a2a-protocol.org/v1.0.0/
- 3: https://github.com/a2aproject/A2A/blob/main/docs/announcing-1.0.md
- 4: https://a2a-protocol.org/v1.0.0/specification/
- 5: https://github.com/a2aproject/A2A/blob/main/docs/specification.md
- 6: https://a2a-protocol.org/latest/specification/
- 7: https://github.com/google/A2A/blob/main/docs/specification.md
🌐 Web query:
MCP Tasks 2025-11-25 experimental task subscription replay cursor after_cursor
💡 Result:
In the Model Context Protocol (MCP) specification version 2025-11-25, tasks were introduced as an experimental feature that utilized cursor-based pagination for the tasks/list method [1][2]. For the tasks/list request, the pagination mechanism functions as follows: - Requestors can include an optional cursor field in the request parameters to specify their position in a paginated list [1][2]. - The server responds with a list of tasks and a nextCursor field if additional tasks are available [1][2]. - Requestors must treat these cursors as opaque tokens and are not to attempt to parse or modify them [1][2]. While the term "after_cursor" is common in general API design for pagination (often used as a parameter key or path) [3][4], the official 2025-11-25 MCP specification uses cursor as the input parameter and nextCursor as the response field for task pagination [1][2]. References to "after_cursor" in MCP-related codebases or discussions [5][6] generally refer to the broader implementation pattern rather than a mandatory field name defined in the 2025-11-25 protocol specification itself. As of August 2026, the experimental task feature has evolved significantly, having been moved out of the core protocol and into an official, versioned extension (io.modelcontextprotocol/tasks) [7][8][9]. New implementations are expected to follow this current extension specification rather than the legacy 2025-11-25 design [10][7].
Citations:
- 1: https://modelcontextprotocol.io/specification/2025-11-25/basic/utilities/tasks
- 2: https://modelcontextprotocol.io/specification/2025-11-25/basic/utilities/tasks.md
- 3: https://docs.port.io/context-lake/ingestion/ingest-data-into-port/custom-integration/ocean-custom-integration/configuration/
- 4: https://docs.sim.ai/integrations/zendesk
- 5: https://gitcode.com/Jason_Daut/hermeshermes-agent-agent/blob/main/mcp_serve.py
- 6: https://github.com/chirino/memory-service/blob/main/AGENTS.md
- 7: https://www.ainews.tech/blog/mcp-stateless-migration-checklist
- 8: https://py.sdk.modelcontextprotocol.io/v2/whats-new/
- 9: https://modelcontextprotocol.io/specification/draft/basic/utilities/tasks
- 10: https://modelcontextprotocol.io/seps/2663-tasks-extension.md
🏁 Script executed:
#!/bin/bash
set -e
file="$(git ls-files | awk 'tolower($0) ~ /(^|\/)a2a_web_parity_discussion\.md$/ {print; exit}')"
if [ -z "$file" ]; then
echo "A2A_WEB_PARITY_DISCUSSION.md not found in tracked files" >&2
exit 1
fi
printf 'FILE=%s\n' "$file"
printf '%s\n' '--- referenced document sections ---'
sed -n '400,530p' "$file"
printf '%s\n' '--- all protocol references in the document ---'
rg -n -C 2 'A2A|MCP|SubscribeToTask|after_cursor|cursor|2025-11-25|whats-new' "$file"Repository: larksuite/cli
Length of output: 207
🏁 Script executed:
#!/bin/bash
set -e
file="$(git ls-files | awk 'tolower($0) ~ /(^|\/)a2a_web_parity_discussion\.md$/ {print; exit}')"
if [ -z "$file" ]; then
echo "A2A_WEB_PARITY_DISCUSSION.md not found in tracked files" >&2
exit 1
fi
printf 'FILE=%s\n' "$file"
sed -n '400,530p' "$file"
rg -n -C 2 'A2A|MCP|SubscribeToTask|after_cursor|cursor|2025-11-25|whats-new' "$file"Repository: larksuite/cli
Length of output: 207
🌐 Web query:
site:a2a-protocol.org/v1.0.0/specification SubscribeToTask pageToken next_page_token replay cursor
💡 Result:
The terms SubscribeToTask, pageToken, next_page_token, and cursor are associated with the A2A Protocol, which utilizes cursor-based pagination for listing and managing task data [1]. In this specification, pageToken and next_page_token are used to implement cursor-based pagination for retrieving lists of tasks [1]. This approach is preferred over offset-based pagination because it provides better performance and consistency for large datasets, avoiding the performance degradation associated with deep pagination [1]. The SubscribeToTask operation is a streaming method designed to provide real-time updates on a specific task [1]. To ensure no information is lost during the transition between requesting current state and subscribing to future updates, the protocol requires that the operation return the current state of the task as the first event in the stream [1]. While pagination tokens are standard for List-style methods (like ListTasks), the subscription mechanism focuses on maintaining a live connection to the task state rather than paginating results [1].
Citations:
🌐 Web query:
site:modelcontextprotocol.io/specification/2025-11-25/basic/utilities/tasks experimental tasks/list cursor nextCursor SubscribeTask
💡 Result:
The tasks/list operation is an experimental feature of the Model Context Protocol (MCP) introduced in version 2025-11-25 [1]. It uses cursor-based pagination to manage task retrieval [1]. When a requestor calls tasks/list, they can provide an optional cursor parameter in the request params [1]. The server's response includes a nextCursor field, which the server must provide if additional tasks are available [1]. Requestors are required to treat these cursors as opaque tokens and must not attempt to parse or modify them [1]. Regarding SubscribeTask, there is no evidence in the provided documentation of a feature or method by that name within the MCP experimental tasks specification [1]. Example of the tasks/list request and response structure: { "jsonrpc": "2.0", "id": 5, "method": "tasks/list", "params": { "cursor": "optional-cursor-value" } } { "jsonrpc": "2.0", "id": 5, "result": { "tasks": [...], "nextCursor": "next-page-cursor" } }
Citations:
Separate standard capabilities from Lark-specific recovery semantics.
Mark after_cursor as a Lark-specific extension. Pin the A2A and MCP protocol versions. Do not cite A2A SubscribeToTask or MCP Tasks as evidence that standard event replay is defined.
🤖 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 `@A2A_WEB_PARITY_DISCUSSION.md` around lines 421 - 429, Update section “7.2
推荐方案” to distinguish standard A2A/MCP capabilities from Lark-specific recovery
semantics: label `SubscribeTask(after_cursor=...)` and cursor-expiry recovery as
Lark-specific extensions, explicitly pin the relevant A2A and MCP protocol
versions, and remove any implication that A2A `SubscribeToTask` or MCP Tasks
define standard event replay.
Source: MCP tools
| #### 5.2.1 定义 provider-neutral 的公开事件模型 | ||
|
|
||
| 建议至少支持以下事件类别： | ||
|
|
||
| | 事件 | 语义 | | ||
| |---|---| | ||
| | `task.started` | 任务已受理，返回 task/context 标识 | | ||
| | `task.status` | submitted/working/阶段摘要变化 | | ||
| | `message.created` | 创建一个公开消息对象 | | ||
| | `message.delta` | 向已有消息追加公开文本 | | ||
| | `message.updated` | 替换或完成已有消息对象 | | ||
| | `tool.started` | 可公开的工具类别开始执行 | | ||
| | `tool.completed` | 工具完成及脱敏摘要 | | ||
| | `artifact.updated` | 产物 processing/ready/failed 变化 | | ||
| | `input.required` | 结构化问题组，任务暂停 | | ||
| | `auth.required` | 需要用户授权，任务暂停 | | ||
| | `task.completed` | 成功终态及最终结果定位 | | ||
| | `task.failed` | 失败终态及稳定错误分类 | | ||
| | `stream.heartbeat` | 连接存活，不代表业务进度 | | ||
| | `stream.end` | 本次观察结束及 reason | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Choose one normative public event vocabulary.
This document defines message.*, tool.*, task.completed, task.failed, and stream.end. A2A_WEB_PARITY_DISCUSSION.md defines output.* and task.error, while cmd/agents/task-stream-phase1-design.md defines task.snapshot and task.stream_end.
Add a versioned mapping from phase-one compatibility events to the target event model, or select one canonical vocabulary before implementation. Otherwise, reducers and contract tests can implement incompatible protocols.
🤖 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 `@cmd/agents/lark-cli-a2a-web-experience-parity-discussion.md` around lines 419
- 438, Establish one versioned, normative public event vocabulary before
implementation, centered on the event model in section 5.2.1 or an explicitly
selected alternative. Add a compatibility mapping covering output.*, task.error,
task.snapshot, and task.stream_end to the canonical equivalents, and update the
referenced design documents, reducers, and contract-test expectations to use
that mapping consistently.
| | `--timeout` | `5m` | 最长观察时间；`0` 表示无界，超时不取消远端任务 | | ||
| | `--format` | `ndjson` | 一期只接受 `ndjson`；其它值返回 typed validation error | |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file=$(fd -t f '^task-stream-phase1-design\.md$' . | head -n 1)
printf 'FILE=%s\n' "$file"
wc -l "$file"
sed -n '190,235p' "$file"
sed -n '100,125p' "$file"
rg -n -C 3 -- '--timeout|deadline|backoff|GetTask|sleep|timer|poll' "$file"Repository: larksuite/cli
Length of output: 191
🏁 Script executed:
#!/bin/bash
set -eu
git ls-files | rg '(^|/)task-stream-phase1-design\.md$|cmd/agents'Repository: larksuite/cli
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- root ---'
pwd
ls -la
printf '%s\n' '--- candidate paths ---'
find . -type f -name 'task-stream-phase1-design.md' -print
printf '%s\n' '--- repository status ---'
git status --short 2>&1 || trueRepository: larksuite/cli
Length of output: 2907
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- cmd/agents files ---'
find cmd/agents -maxdepth 3 -type f -print 2>/dev/null | sort
printf '%s\n' '--- matching design text ---'
rg -n -i -C 3 '最长观察时间|一期只接受|GetTask|backoff|--timeout|ndjson' cmd .github README* 2>/dev/null || trueRepository: larksuite/cli
Length of output: 19133
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- branches and HEAD ---'
git branch --all --no-color
git rev-parse HEAD
printf '%s\n' '--- tracked cmd/agents entries ---'
git ls-tree -r --name-only HEAD -- cmd/agents
printf '%s\n' '--- recent commits mentioning task stream ---'
git log --oneline --all -- cmd/agents | head -20Repository: larksuite/cli
Length of output: 446
🏁 Script executed:
#!/bin/bash
set -eu
path=cmd/agents/task-stream-phase1-design.md
printf '%s\n' '--- current commit change ---'
git show --stat --oneline HEAD
git show --format= --name-status HEAD -- "$path"
printf '%s\n' '--- file in parent commit ---'
if git cat-file -e "HEAD^:$path" 2>/dev/null; then
git show "HEAD^:$path" | nl -ba | sed -n '100,125p'
git show "HEAD^:$path" | nl -ba | sed -n '198,235p'
else
printf '%s\n' 'not present in parent'
fiRepository: larksuite/cli
Length of output: 1087
🏁 Script executed:
#!/bin/bash
set -eu
path=cmd/agents/task-stream-phase1-design.md
printf '%s\n' '--- CLI contract ---'
git show "HEAD^:$path" | sed -n '105,122p'
printf '%s\n' '--- polling state machine ---'
git show "HEAD^:$path" | sed -n '198,235p'
printf '%s\n' '--- timeout and error rules ---'
git show "HEAD^:$path" | sed -n '275,325p'
printf '%s\n' '--- tests related to timeout ---'
git show "HEAD^:$path" | sed -n '455,510p'Repository: larksuite/cli
Length of output: 4044
🏁 Script executed:
#!/bin/bash
set -eu
path=cmd/agents/task-stream-phase1-design.md
git show "HEAD^:$path" | sed -n '315,365p'Repository: larksuite/cli
Length of output: 1467
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from dataclasses import dataclass
`@dataclass`
class Poll:
request_duration: float
sleep_after: float
def documented_elapsed(polls, timeout):
elapsed = 0.0
for poll in polls:
elapsed += poll.request_duration
# The design checks timeout only after a completed poll and before
# starting the next request; its sleep is not deadline-aware.
if elapsed >= timeout:
return elapsed, "timeout"
elapsed += poll.sleep_after
return elapsed, "still polling"
cases = {
"sleep exceeds timeout": [Poll(0.1, 5.0)],
"request exceeds timeout": [Poll(6.0, 0.0)],
}
for name, polls in cases.items():
elapsed, result = documented_elapsed(polls, 1.0)
print(f"{name}: result={result}, elapsed={elapsed:.1f}s, deadline=1.0s")
PYRepository: larksuite/cli
Length of output: 293
Enforce --timeout as a hard observation deadline.
The polling flow can exceed the configured timeout during backoff or GetTask. Use a deadline-aware context and interruptible timer. Define whether the initial GetTask must complete before the deadline. Add a test with a timeout shorter than the next backoff interval.
🤖 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 `@cmd/agents/task-stream-phase1-design.md` around lines 114 - 115, Update the
task polling flow described in the design so --timeout is enforced as a hard
observation deadline, using a deadline-aware context for polling and GetTask
plus an interruptible timer during backoff. Explicitly define whether the
initial GetTask may finish after the deadline, and add coverage where the
timeout is shorter than the next backoff interval to verify prompt termination
without cancelling the remote task.
| ### 6.3 结束事件 | ||
|
|
||
| 当观察窗口结束时输出一条控制事件： | ||
|
|
||
| ```json | ||
| { | ||
| "ok": true, | ||
| "identity": "user", | ||
| "event": { | ||
| "type": "task.stream_end", | ||
| "sequence": 4, | ||
| "observed_at": "2026-08-03T10:26:24Z", | ||
| "reason": "completed" | ||
| }, | ||
| "data": { | ||
| "task_id": "task_123", | ||
| "state": "completed", | ||
| "is_terminal": true | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| `reason` 枚举： | ||
|
|
||
| | reason | 含义 | 远端任务是否停止 | | ||
| |---|---|---| | ||
| | `completed` | 成功终态 | 是 | | ||
| | `failed` | 失败终态 | 是 | | ||
| | `rejected` | 拒绝终态 | 是 | | ||
| | `canceled` | 取消终态 | 是 | | ||
| | `input_required` | 等待用户回答 | 暂停 | | ||
| | `auth_required` | 等待授权 | 暂停 | | ||
| | `timeout` | CLI 观察窗口到期 | 否 | | ||
|
|
||
| 结束事件不重复完整 messages/artifacts，只携带任务标识和状态。完整内容已经由结束前最后一条 `task.snapshot` 输出。 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make stream termination self-describing on failures.
The task.stream_end example uses "ok": true even when reason is failed, rejected, or canceled. The error table also omits a stream_end event for API errors, content-safety blocks, and write failures.
Define whether ok means transport success or task success. Prefer a separate task outcome field. Emit a non-sensitive error end event when stdout is still writable. If writing fails, document that the exit code is the only terminal signal. Keep the exit code as a secondary signal, not the sole task-status indicator.
Also applies to: 328-346
🤖 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 `@cmd/agents/task-stream-phase1-design.md` around lines 156 - 190,
更新“结束事件”及对应错误表，明确 task.stream_end 中 ok 表示传输成功而非任务成功，并增加独立的任务结果字段以区分
completed、failed、rejected、canceled 等状态。为 API 错误、内容安全拦截和写入失败补充终止行为：stdout
仍可写时发送不含敏感信息的错误结束事件；写入失败时说明只能依赖退出码，但将退出码保留为辅助信号而非唯一任务状态来源。
| func TestBaseButtonRuleValidateRejectsInternalWorkflowID(t *testing.T) { | ||
| factory, stdout, _ := newExecuteFactory(t) | ||
| err := runShortcut(t, BaseButtonRuleBind, []string{"+button-rule-bind", "--base-token", "app_x", "--table-id", "tbl_1", "--field-id", "fld_1", "--workflow-id", "123456"}, factory, stdout) | ||
| if err == nil || !strings.Contains(err.Error(), "public wkf workflow ID") { | ||
| t.Fatalf("err=%v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert typed validation metadata.
This test only matches the error message. An untyped error with the same message passes.
Assert *errs.ValidationError metadata, including the invalid-argument subtype and --workflow-id parameter.
As per coding guidelines, “Error tests must assert typed metadata and cause preservation rather than message text alone.”
🤖 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_execute_test.go` around lines 190 - 196, Update
TestBaseButtonRuleValidateRejectsInternalWorkflowID to assert that the returned
error is an *errs.ValidationError, then verify its invalid-argument subtype and
associated parameter is --workflow-id. Replace the message-only validation while
preserving the existing rejection scenario and ensure the validation cause
metadata is checked rather than relying on matching error text.
Source: Coding guidelines
| var BaseWorkspaceCreate = common.Shortcut{ | ||
| Service: "base", | ||
| Command: "+workspace-create", | ||
| Description: "Create a Base workspace", | ||
| Risk: "write", | ||
| Scopes: []string{"base:workspace:create"}, | ||
| AuthTypes: authTypes(), | ||
| Flags: []common.Flag{ | ||
| {Name: "name", Desc: "workspace name", Required: true}, | ||
| {Name: "icon", Desc: "workspace icon"}, | ||
| }, | ||
| Tips: []string{ | ||
| `Example: lark-cli base +workspace-create --name "Sales Workspace"`, | ||
| "Record the returned workspace_token; workspace entity commands need it.", | ||
| }, | ||
| Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { | ||
| return validateWorkspaceCreate(runtime) | ||
| }, | ||
| DryRun: dryRunWorkspaceCreate, | ||
| Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { | ||
| return executeWorkspaceCreate(runtime) | ||
| }, | ||
| } | ||
|
|
||
| var BaseWorkspaceEntityList = common.Shortcut{ | ||
| Service: "base", | ||
| Command: "+workspace-entity-list", | ||
| Description: "List Base or BaseApp entities in a workspace", | ||
| Risk: "read", | ||
| Scopes: []string{"base:workspace:read"}, | ||
| AuthTypes: authTypes(), | ||
| Flags: []common.Flag{ | ||
| workspaceTokenFlag(true), | ||
| workspaceEntityTypeFlag(false), | ||
| {Name: "page-size", Type: "int", Default: "100", Desc: "page size per request, range 1-100"}, | ||
| {Name: "page-token", Desc: "pagination token from the previous response"}, | ||
| }, | ||
| Tips: []string{ | ||
| `Example: lark-cli base +workspace-entity-list --workspace-token <workspace_token> --type base --page-size 100`, | ||
| "Use entity_id from this output when removing an entity from the workspace tree.", | ||
| }, | ||
| Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { | ||
| return validateWorkspaceEntityList(runtime) | ||
| }, | ||
| DryRun: dryRunWorkspaceEntityList, | ||
| Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { | ||
| return executeWorkspaceEntityList(runtime) | ||
| }, | ||
| } | ||
|
|
||
| var BaseWorkspaceEntityAdd = common.Shortcut{ | ||
| Service: "base", | ||
| Command: "+workspace-entity-add", | ||
| Description: "Add a Base or BaseApp entity to a workspace", | ||
| Risk: "write", | ||
| Scopes: []string{"base:workspace:write"}, | ||
| AuthTypes: authTypes(), | ||
| Flags: []common.Flag{ | ||
| workspaceTokenFlag(true), | ||
| workspaceEntityTypeFlag(true), | ||
| {Name: "token", Desc: "base_token or app_token of the entity to add", Required: true}, | ||
| {Name: "prev-entity-id", Desc: "insert after this workspace entity ID"}, | ||
| {Name: "to-last", Type: "bool", Desc: "append the entity to the end of the workspace"}, | ||
| }, | ||
| Tips: []string{ | ||
| `Example: lark-cli base +workspace-entity-add --workspace-token <workspace_token> --type base --token <base_token> --to-last`, | ||
| "`--token` is the underlying Base/BaseApp token, not workspace entity_id.", | ||
| }, | ||
| Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { | ||
| return validateWorkspaceEntityAdd(runtime) | ||
| }, | ||
| DryRun: dryRunWorkspaceEntityAdd, | ||
| Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { | ||
| return executeWorkspaceEntityAdd(runtime) | ||
| }, | ||
| } | ||
|
|
||
| var BaseWorkspaceEntityRemove = common.Shortcut{ | ||
| Service: "base", | ||
| Command: "+workspace-entity-remove", | ||
| Description: "Remove an entity relation from a workspace tree", | ||
| Risk: "high-risk-write", | ||
| Scopes: []string{"base:workspace:write"}, | ||
| AuthTypes: authTypes(), | ||
| Flags: []common.Flag{ | ||
| workspaceTokenFlag(true), | ||
| {Name: "entity-id", Desc: "workspace entity ID to remove", Required: true}, | ||
| }, | ||
| Tips: []string{ | ||
| `Example: lark-cli base +workspace-entity-remove --workspace-token <workspace_token> --entity-id <entity_id> --yes`, | ||
| "This only removes the entity from the workspace tree; it does not delete the underlying Base or BaseApp resource.", | ||
| baseHighRiskYesTip, | ||
| }, | ||
| Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { | ||
| return validateWorkspaceEntityRemove(runtime) | ||
| }, | ||
| DryRun: dryRunWorkspaceEntityRemove, | ||
| Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { | ||
| return executeWorkspaceEntityRemove(runtime) | ||
| }, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Add self-contained live E2E coverage for the new workspace shortcuts.
The supplied tests cover dry-run behavior only. These shortcuts include persistent write operations. +workspace-create has no exposed cleanup command, so a simple live test would leak Workspace state.
Add a cleanup workflow and live coverage for create, list, add, remove, and cleanup. Do not merge a persistent live test that cannot clean up after failure.
As per coding guidelines: “new shortcuts require live E2E coverage” and “Live E2E tests must be self-contained: create, use, and clean up their resources even after failure.”
🤖 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/workspace.go` around lines 12 - 112, Add self-contained live
E2E coverage for BaseWorkspaceCreate, BaseWorkspaceEntityList,
BaseWorkspaceEntityAdd, and BaseWorkspaceEntityRemove, covering the complete
create, list, add, and remove workflow. Introduce an exposed cleanup shortcut or
equivalent cleanup workflow for the workspace created by the test, and ensure
cleanup runs even when earlier assertions or operations fail so no persistent
workspace state is leaked.
Source: Coding guidelines
| | `1254045` 字段名不存在 | 重新 `+field-list`，使用真实字段名或字段 ID；注意空格、大小写和跨表字段 | | ||
| | `1254015` 字段值类型不匹配 | 先 `+field-list`，再按 [lark-base-cell-value.md](references/lark-base-cell-value.md) 构造 CellValue | | ||
| | `Invalid discriminator value`（字段写入缺 `type`） | 按完整提交规则读取当前字段，只改目标内容后提交；不要只补 `type` 重试 | | ||
| | `Invalid discriminator value`（字段写入） | 先确认字段 JSON 有合法 `type`。若请求已是 `{"type":"button",...}`，通常表示当前调用的 Base OpenAPI 后端或 `bitable.node.mcp` 仍未部署按钮字段 schema；不要往字段 JSON 里追加 `workflow_id`、内部 Workflow ID 或底层 `property.trigger`，应等待对应环境升级后用同一最小 JSON 重试 | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a button-rule routing entry to SKILL.md.
The recovery rule names button fields but does not route direct binding requests. Add a quick-routing entry for +button-rule-bind, +button-rule-get, and +button-rule-unbind. Link it to lark-base-field-json.md for the field-creation sequence.
As per coding guidelines: skills/<name>/SKILL.md must contain domain routing and cross-command workflows.
🤖 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-base/SKILL.md` at line 153, Update the routing section in
SKILL.md to add entries for +button-rule-bind, +button-rule-get, and
+button-rule-unbind, directing these requests to lark-base-field-json.md and its
field-creation sequence. Keep the existing button-field recovery guidance
unchanged and ensure the new entries provide domain routing and cross-command
workflow coverage.
Source: Coding guidelines
| func TestBaseButtonRuleDryRun(t *testing.T) { | ||
| setBaseDryRunConfigEnv(t) | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
| t.Cleanup(cancel) | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| args []string | ||
| wantMethod string | ||
| wantBody string | ||
| }{ | ||
| { | ||
| name: "bind", | ||
| args: []string{ | ||
| "base", "+button-rule-bind", | ||
| "--base-token", "app_x", | ||
| "--table-id", "tbl_x", | ||
| "--field-id", "fld_x", | ||
| "--workflow-id", "wkf_x", | ||
| "--dry-run", | ||
| }, | ||
| wantMethod: "PUT", | ||
| wantBody: "wkf_x", | ||
| }, | ||
| { | ||
| name: "get", | ||
| args: []string{ | ||
| "base", "+button-rule-get", | ||
| "--base-token", "app_x", | ||
| "--table-id", "tbl_x", | ||
| "--field-id", "fld_x", | ||
| "--dry-run", | ||
| }, | ||
| wantMethod: "GET", | ||
| }, | ||
| { | ||
| name: "unbind", | ||
| args: []string{ | ||
| "base", "+button-rule-unbind", | ||
| "--base-token", "app_x", | ||
| "--table-id", "tbl_x", | ||
| "--field-id", "fld_x", | ||
| "--dry-run", | ||
| }, | ||
| wantMethod: "PUT", | ||
| wantBody: "", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| result, err := clie2e.RunCmd(ctx, clie2e.Request{Args: tt.args, DefaultAs: "bot"}) | ||
| require.NoError(t, err) | ||
| result.AssertExitCode(t, 0) | ||
|
|
||
| out := result.Stdout | ||
| require.Equal(t, "/open-apis/base/v3/bases/app_x/tables/tbl_x/fields/fld_x/button_rule", clie2e.DryRunGet(out, "api.0.url").String(), out) | ||
| require.Equal(t, tt.wantMethod, clie2e.DryRunGet(out, "api.0.method").String(), out) | ||
| if tt.wantMethod == "PUT" { | ||
| require.Equal(t, tt.wantBody, clie2e.DryRunGet(out, "api.0.body.workflow_id").String(), out) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Add self-contained live E2E coverage for the new shortcuts.
This file verifies dry-run output only. shortcuts/base/workflow_execute_test.go uses HTTP mocks.
Add live bind, get, and unbind cases. Create fixtures, verify bound and unbound readback, and clean up all resources after failure.
As per coding guidelines, “new shortcuts require live E2E coverage.”
🤖 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 `@tests/cli_e2e/base/base_button_rule_dryrun_test.go` around lines 15 - 79,
Extend TestBaseButtonRuleDryRun with self-contained live E2E coverage for the
bind, get, and unbind shortcuts, following the HTTP-mock setup used by
workflow_execute_test.go. Create the required base, table, field, and workflow
fixtures, verify get returns the bound workflow and then confirms the field is
unbound, and register cleanup for every created resource so teardown runs even
when assertions fail.
Sources: Coding guidelines, Learnings
Change-Id: I3f05245da74f75e06414a50c60019cb81863e489
023361f to
4f0feca
Compare
github-actions
Bot
added
size/M
2 participants
Summary
Changes
Test Plan
lark-cli <domain> <command>flow works as expectedRelated Issues
Summary by CodeRabbit