feat(todo): harden skill workflows and write safety by Freda0909 · Pull Request #1042 · DingTalk-Real-AI/dingtalk-workspace-cli

Freda0909 · 2026-08-18T12:50:29Z

Summary

harden Todo write receipts, read-back verification, and execution-started error semantics
add executable constraints and local dry-run plans for Todo write shortcuts
refresh the Todo Skill routes and make batch, daily-summary, and overdue scripts bounded and fail-closed
add a release fragment and regression coverage for runtime and Skill scripts

Risk tier

High-risk: internal shortcut declarations feed the live command tree and Schema assembly

Verification

DWS_PACKAGE_VERSION=0.0.0-test go test ./...
make test-schema-agent-examples
make generate-schema
./scripts/policy/check-generated-drift.sh
./scripts/policy/check-schema-catalog.sh
./scripts/policy/check-runtime-confirmation-truth.sh
python3 test/scripts/todo_skill_scripts_test.py
git diff --check
Release-seal validation: N/A (ordinary release-fragment PR; the release-seal content-only check is intentionally not applicable)

Behavior evidence

Todo writes reject malformed success receipts and mark post-dispatch validation failures as execution_started=true.
Todo write shortcuts publish safe dry-run plans without remote reads.
Batch creation records verified, unverified, unknown, and failed outcomes and does not blindly retry ambiguous non-idempotent writes.

Notes

No live DingTalk write was executed during validation.

wxianfeng

开源 DWS Codex 自动 CR：不通过，需修复

feat(todo): harden skill workflows and write safety

评审 SHA：ced81db4108b6afd58e1c2a678ca76f52472a81f
CI 状态：未通过（30 通过，4 跳过，1 未通过）
冲突状态：无冲突
Agent 测试报告：不通过，缺少“Agent 测试报告”截图（todo）
指令 CI 集成测试：不涉及
自动 CR 结论：不通过，需修复
代码评审结论：不通过，需修复

变更总结

本次变更加强了 Todo 写回执、读回验证、分页完整性和文档中的安全示例，并为多个写 Shortcut 增加本地 dry-run；但新批量创建脚本无法通过写命令的用户确认门，导致其核心创建流程在非交互子进程中失败。

CI 单测结果

✅ AI Behavior — 通过
✅ CLI Smoke — 通过
❌ Coverage — 失败
✅ Coverage (baseline) — 通过
✅ Coverage (current) — 跳过
✅ Coverage (current: app) — 通过
✅ Coverage (current: cli) — 通过
✅ Coverage (current: generators) — 通过
✅ Coverage (current: helpers) — 通过
✅ Coverage (current: remaining) — 通过
✅ Coverage (macOS) — 跳过
✅ Coverage (supporting) — 通过
✅ Coverage (Windows) — 跳过
✅ Edition — 通过
✅ Interface Integrity — 通过
✅ Lint — 通过
✅ Mock MCP — 通过
✅ Policy — 通过
✅ route — 通过
✅ Test — 通过
✅ Test (cross-platform compile) — 通过
✅ Test (focused: ${{ matrix.shard }}) — 跳过
✅ Test (macOS auth/keychain) — 通过
✅ Test (race: app-a-b) — 通过
✅ Test (race: app-c) — 通过
✅ Test (race: app-d-r) — 通过
✅ Test (race: app-s-z-example-fuzz) — 通过
✅ Test (race: app-schema) — 通过
✅ Test (race: cli) — 通过
✅ Test (race: generators) — 通过
✅ Test (race: helpers) — 通过
✅ Test (race: remaining) — 通过
✅ Test (race: smoke) — 通过
✅ Test (Windows) — 通过
✅ Test (workflow and release contracts) — 通过

结论说明

Codex 静态审查发现 P0/P1 阻断问题，需要修复后提交新 commit 重新 CR。

代码问题

[P1 | 必须修改] 批量创建脚本无法完成写操作确认 — skills/multi/dingtalk-todo/scripts/todo_batch_create.py:213
- 说明：脚本通过 subprocess.run(..., capture_output=True) 调用受 user_required 保护的 dws todo task create，既没有在脚本层取得一次明确确认，也没有在确认后向子命令传递确认凭据。由于子进程输出被捕获，运行时确认提示对用户不可见；在非交互模式下通常会直接拒绝，若仍读取继承的 stdin 则会一直等待到 120 秒超时。因此文档推荐的 python scripts/todo_batch_create.py todos.json 实际无法可靠创建任何待办。应在写入前显式请求批次确认，确认后对每个精确创建调用传递运行时认可的确认方式，并补充“确认前无工具调用、确认后精确调用”的测试。

此结论已作为 GitHub CHANGES_REQUESTED 提交；不会自动合并。

wxianfeng · 2026-08-18T13:42:09Z

开源 DWS Codex 自动 CR：不通过，需修复

feat(todo): harden skill workflows and write safety

评审 SHA：ced81db4108b6afd58e1c2a678ca76f52472a81f
CI 状态：未通过（30 通过，4 跳过，1 未通过）
冲突状态：无冲突
Agent 测试报告：不通过，缺少“Agent 测试报告”截图（todo）
指令 CI 集成测试：不涉及
自动 CR 结论：不通过，需修复
代码评审结论：不通过，需修复

变更总结

本次变更加强了 Todo 写回执、读回验证、分页完整性和文档中的安全示例，并为多个写 Shortcut 增加本地 dry-run；但新批量创建脚本无法通过写命令的用户确认门，导致其核心创建流程在非交互子进程中失败。

CI 单测结果

✅ AI Behavior — 通过
✅ CLI Smoke — 通过
❌ Coverage — 失败
✅ Coverage (baseline) — 通过
✅ Coverage (current) — 跳过
✅ Coverage (current: app) — 通过
✅ Coverage (current: cli) — 通过
✅ Coverage (current: generators) — 通过
✅ Coverage (current: helpers) — 通过
✅ Coverage (current: remaining) — 通过
✅ Coverage (macOS) — 跳过
✅ Coverage (supporting) — 通过
✅ Coverage (Windows) — 跳过
✅ Edition — 通过
✅ Interface Integrity — 通过
✅ Lint — 通过
✅ Mock MCP — 通过
✅ Policy — 通过
✅ route — 通过
✅ Test — 通过
✅ Test (cross-platform compile) — 通过
✅ Test (focused: ${{ matrix.shard }}) — 跳过
✅ Test (macOS auth/keychain) — 通过
✅ Test (race: app-a-b) — 通过
✅ Test (race: app-c) — 通过
✅ Test (race: app-d-r) — 通过
✅ Test (race: app-s-z-example-fuzz) — 通过
✅ Test (race: app-schema) — 通过
✅ Test (race: cli) — 通过
✅ Test (race: generators) — 通过
✅ Test (race: helpers) — 通过
✅ Test (race: remaining) — 通过
✅ Test (race: smoke) — 通过
✅ Test (Windows) — 通过
✅ Test (workflow and release contracts) — 通过

结论说明

Codex 静态审查发现 P0/P1 阻断问题，需要修复后提交新 commit 重新 CR。

代码问题

[P1 | 必须修改] 批量创建脚本无法完成写操作确认 — skills/multi/dingtalk-todo/scripts/todo_batch_create.py:213
- 说明：脚本通过 subprocess.run(..., capture_output=True) 调用受 user_required 保护的 dws todo task create，既没有在脚本层取得一次明确确认，也没有在确认后向子命令传递确认凭据。由于子进程输出被捕获，运行时确认提示对用户不可见；在非交互模式下通常会直接拒绝，若仍读取继承的 stdin 则会一直等待到 120 秒超时。因此文档推荐的 python scripts/todo_batch_create.py todos.json 实际无法可靠创建任何待办。应在写入前显式请求批次确认，确认后对每个精确创建调用传递运行时认可的确认方式，并补充“确认前无工具调用、确认后精确调用”的测试。

此结论已作为 GitHub CHANGES_REQUESTED 提交；不会自动合并。

wxianfeng

开源 DWS Codex 自动 CR：不通过，需修复

feat(todo): harden skill workflows and write safety

评审 SHA：bdf0cd5de950c082959622e8376a7aefe09da093
CI 状态：通过（31 通过，4 跳过）
冲突状态：无冲突
Agent 测试报告：不通过，缺少“Agent 测试报告”截图（todo）
指令 CI 集成测试：不涉及
自动 CR 结论：不通过，需修复
代码评审结论：不通过，需修复

变更总结

本次修改强化了 Todo Shortcut 的参数校验、dry-run 声明、写后状态标记和分页完整性，并移除了文档中危险的 --yes 示例；但新设为 Golden Route 的批量创建脚本没有实现可用的批次级确认流程，在捕获子进程输出的同时直接调用受确认门保护的创建命令，导致标准用法无法可靠执行。

CI 单测结果

✅ AI Behavior — 通过
✅ CLI Smoke — 通过
✅ Coverage — 通过
✅ Coverage (baseline) — 通过
✅ Coverage (current) — 跳过
✅ Coverage (current: app) — 通过
✅ Coverage (current: cli) — 通过
✅ Coverage (current: generators) — 通过
✅ Coverage (current: helpers) — 通过
✅ Coverage (current: remaining) — 通过
✅ Coverage (macOS) — 跳过
✅ Coverage (supporting) — 通过
✅ Coverage (Windows) — 跳过
✅ Edition — 通过
✅ Interface Integrity — 通过
✅ Lint — 通过
✅ Mock MCP — 通过
✅ Policy — 通过
✅ route — 通过
✅ Test — 通过
✅ Test (cross-platform compile) — 通过
✅ Test (focused: ${{ matrix.shard }}) — 跳过
✅ Test (macOS auth/keychain) — 通过
✅ Test (race: app-a-b) — 通过
✅ Test (race: app-c) — 通过
✅ Test (race: app-d-r) — 通过
✅ Test (race: app-s-z-example-fuzz) — 通过
✅ Test (race: app-schema) — 通过
✅ Test (race: cli) — 通过
✅ Test (race: generators) — 通过
✅ Test (race: helpers) — 通过
✅ Test (race: remaining) — 通过
✅ Test (race: smoke) — 通过
✅ Test (Windows) — 通过
✅ Test (workflow and release contracts) — 通过

结论说明

Codex 静态审查发现 P0/P1 阻断问题，需要修复后提交新 commit 重新 CR。

代码问题

[P1 | 必须修改] 批量创建 Golden Route 无法完成运行时确认 — skills/multi/dingtalk-todo/scripts/todo_batch_create.py:198
- 说明：脚本把 dws todo task create 的 stdout/stderr 全部捕获，却既没有在批次开始前取得明确确认，也没有在确认后为子命令传递确认结果。该写命令受 user_required 运行时门保护：交互提示会被捕获而用户不可见，自动化环境则会在确认前拒绝或等待至超时，因此文档推荐的 python .../todo_batch_create.py todos.json 无法可靠创建任务，并可能把确认前拒绝误记为 unknown。应实现两阶段流程：确认前只验证并展示完整计划；得到明确批次确认后才执行精确的创建 argv，并增加测试覆盖“确认前零调用”和“确认后传给 dws 的精确调用”。

此结论已作为 GitHub CHANGES_REQUESTED 提交；不会自动合并。

wxianfeng · 2026-08-18T14:09:07Z

开源 DWS Codex 自动 CR：不通过，需修复

feat(todo): harden skill workflows and write safety

评审 SHA：bdf0cd5de950c082959622e8376a7aefe09da093
CI 状态：通过（31 通过，4 跳过）
冲突状态：无冲突
Agent 测试报告：不通过，缺少“Agent 测试报告”截图（todo）
指令 CI 集成测试：不涉及
自动 CR 结论：不通过，需修复
代码评审结论：不通过，需修复

变更总结

本次修改强化了 Todo Shortcut 的参数校验、dry-run 声明、写后状态标记和分页完整性，并移除了文档中危险的 --yes 示例；但新设为 Golden Route 的批量创建脚本没有实现可用的批次级确认流程，在捕获子进程输出的同时直接调用受确认门保护的创建命令，导致标准用法无法可靠执行。

CI 单测结果

✅ AI Behavior — 通过
✅ CLI Smoke — 通过
✅ Coverage — 通过
✅ Coverage (baseline) — 通过
✅ Coverage (current) — 跳过
✅ Coverage (current: app) — 通过
✅ Coverage (current: cli) — 通过
✅ Coverage (current: generators) — 通过
✅ Coverage (current: helpers) — 通过
✅ Coverage (current: remaining) — 通过
✅ Coverage (macOS) — 跳过
✅ Coverage (supporting) — 通过
✅ Coverage (Windows) — 跳过
✅ Edition — 通过
✅ Interface Integrity — 通过
✅ Lint — 通过
✅ Mock MCP — 通过
✅ Policy — 通过
✅ route — 通过
✅ Test — 通过
✅ Test (cross-platform compile) — 通过
✅ Test (focused: ${{ matrix.shard }}) — 跳过
✅ Test (macOS auth/keychain) — 通过
✅ Test (race: app-a-b) — 通过
✅ Test (race: app-c) — 通过
✅ Test (race: app-d-r) — 通过
✅ Test (race: app-s-z-example-fuzz) — 通过
✅ Test (race: app-schema) — 通过
✅ Test (race: cli) — 通过
✅ Test (race: generators) — 通过
✅ Test (race: helpers) — 通过
✅ Test (race: remaining) — 通过
✅ Test (race: smoke) — 通过
✅ Test (Windows) — 通过
✅ Test (workflow and release contracts) — 通过

结论说明

Codex 静态审查发现 P0/P1 阻断问题，需要修复后提交新 commit 重新 CR。

代码问题

[P1 | 必须修改] 批量创建 Golden Route 无法完成运行时确认 — skills/multi/dingtalk-todo/scripts/todo_batch_create.py:198
- 说明：脚本把 dws todo task create 的 stdout/stderr 全部捕获，却既没有在批次开始前取得明确确认，也没有在确认后为子命令传递确认结果。该写命令受 user_required 运行时门保护：交互提示会被捕获而用户不可见，自动化环境则会在确认前拒绝或等待至超时，因此文档推荐的 python .../todo_batch_create.py todos.json 无法可靠创建任务，并可能把确认前拒绝误记为 unknown。应实现两阶段流程：确认前只验证并展示完整计划；得到明确批次确认后才执行精确的创建 argv，并增加测试覆盖“确认前零调用”和“确认后传给 dws 的精确调用”。

此结论已作为 GitHub CHANGES_REQUESTED 提交；不会自动合并。

haofeng0705

Gate decision

REQUEST CHANGES — one small PR-introduced defect: this PR removes Markdown table-pipe escaping that the base file had, which silently drops the "关键结果 / 边界" column from two Golden Routes in a shipped skill document. Everything else holds up under independent reproduction — all 8 claimed commands pass, plus 3 more I ran that were not claimed — and the prior auto-CR's P1 against the batch script does not reproduce, so please do not act on it (details below).

Review provenance

[PASS] Reviewed at PR head bdf0cd5d in a clean dedicated detached checkout
[PASS] Base revision c15480c4 (merge-base with origin/main), built separately for before/after comparison
[PASS] Head re-checked during review — drift detected and handled: analysis started at ced81db4, author pushed bdf0cd5d (test(todo): cover validation branches) mid-review. Fast-forward confirmed via git merge-base --is-ancestor. That commit touches implementation (assign.go −4, remind.go −3), so conclusions were not carried forward on hash-equality; the affected checks were re-run at bdf0cd5d. All findings below are stated at bdf0cd5d.

PR description vs repository template

Template graded against: .github/pull_request_template.md
[PASS] T1 Summary — four bullets covering write receipts/read-back, executable constraints + dry-run plans, Skill route refresh with bounded fail-closed scripts, fragment + regression coverage. Accurate against the diff, with one wording nit under F6.
[PASS] T2 Risk tier — exactly one box, High-risk, with the reason "internal shortcut declarations feed the live command tree and Schema assembly". Correct: the PR adds Contract.DryRun declarations that flow into Schema assembly.
[PASS] T3 Release fragment — .changes/todo-skill-safe-workflows.md added; CHANGELOG.md untouched.
[NOT VERIFIED] T4 Release-seal validation — the item is absent from the Verification section. The template asks for it "(otherwise N/A)", so it should carry an explicit N/A. No actual work is implied: check-changelog-pr.sh --content-only requires the PR to have modified CHANGELOG.md and therefore errors by design on an ordinary fragment PR. Recording N/A closes this.
[PASS] T5 Targeted test/check commands and results — 8 commands listed; every one reproduced below.
[PASS] T6 Behavior evidence — three concrete behavioral statements, all independently reproduced (B1–B3).
[PASS] T7 Conditional checks — check-generated-drift.sh recorded and reproduced. check-command-surface.sh --strict was not recorded although flag help text and contract declarations changed; I ran it anyway and it passes (exit 0), so this is an observation rather than a defect. verify-package-managers.sh is N/A (no packaging surface touched). The documentation-rendering item is N/A by the template's own parenthetical, which scopes it to documentation-only PRs.
[PASS] T8 Notes — "No live DingTalk write was executed during validation." Consistent with what the evidence can show.
Observations outside the template, explicitly not counted as defects: local OS/arch/Go version (never requested; go.mod pins Go and CI uses go-version-file: go.mod); a requirement-to-test mapping table; a dedicated limitations section; reproducing the full CI suite locally (the template explicitly says not to); and "upstream MCP contract is not locally verifiable", which is a property shared by all 1152 tool bindings.

Author validation reproduction

ID	Acceptance criterion	Author claim / exact step	Documented environment	Independent result	Match	Evidence
V1	Full suite green	`DWS_PACKAGE_VERSION=0.0.0-test go test ./...`	none stated	PASS	Yes	exit 0, 0 `FAIL`/`panic` lines, 111 `ok` packages; re-run in full at `bdf0cd5d`
V2	Agent dry-run examples green	`make test-schema-agent-examples`	none stated	PASS	Yes	exit 0; includes `TestAgentExamplesDryRun/todo/shortcut_{create,remind,reminder,reopen,todo_done,update}`
V3	Schema regenerates	`make generate-schema`	none stated	PASS	Yes	covered by V4 sandboxed determinism run; working tree clean afterwards
V4	No generated drift	`./scripts/policy/check-generated-drift.sh`	none stated	PASS	Yes	exit 0; `schema assembly determinism check: ok`; identical `registry_hash` across both passes
V5	Catalog intact	`./scripts/policy/check-schema-catalog.sh`	none stated	PASS	Yes	exit 0; `27 products, 1152 tools`
V6	Confirmation homology	`./scripts/policy/check-runtime-confirmation-truth.sh`	none stated	PASS	Yes	exit 0; `runtime confirmation truth ok (live Contract SafetySpec homology)`
V7	Skill script tests green	`python3 test/scripts/todo_skill_scripts_test.py`	none stated	PASS	Yes	`Ran 8 tests ... OK`
V8	No whitespace damage	`git diff --check`	none stated	PASS	Yes	exit 0
V9	(not claimed) command surface intact	`./scripts/policy/check-command-surface.sh --strict`	—	PASS	n/a	exit 0; `command surface check: ok`
V10	(not claimed) fragment gate	`./scripts/policy/check-release-fragments.sh <base> HEAD`	—	PASS	n/a	exit 0
V11	(not claimed) affected packages at new head	`go test ./internal/shortcut/... ./test/unit/...`	—	PASS	n/a	all `ok`, incl. `shortcut/smart`, `shortcut/todo`, `test/unit`

Behavioral and regression checks

[PASS] B1 write receipts mark execution_started — requireTodoWriteReceipt now separates empty_tool_response, missing_success_receipt, malformed_success, and remote_failure, and routes all four through todoWriteResponseError with ExecutionStarted=true, Retryable=false. I checked the refactor for lost coverage: the removed requireTodoResponse call only performed the empty check plus the same success check (it treats success as optional), so no validation was weakened — the four rejection cases are all still rejected, now with distinct reasons and correct post-dispatch semantics for non-idempotent writes.
[PASS] B2 dry-run plans without remote reads — this is the strongest change in the PR, and it is genuinely new. Base-vs-head, same arguments, no credentials-dependent difference:
- dws todo +assign --to "张三" --task "probe" --dry-run — base: exit 3 failure, resolution_not_found ("没有找到与张三唯一匹配…"), i.e. the base dry-run performed a real contact-resolution read before previewing. head: exit 0, {"dryRun":true,"executed":false,"preview_kind":"plan","subject":"probe","assigneeQuery":"张三"} with no remote call.
- dws todo +remind --task "probe" --dry-run — base reached contact/get_current_user_profile before returning; head returns the plan locally. RemoteReads: false is therefore truthful at head where it would have been false at base.
- +todo-done and +assign-multi moved their dry-run return ahead of full-page list reads and name resolution respectively — same class of fix.
- The declared preview kinds line up with what the repo's own gate observes: agentExampleDryRunEvidence classifies a JSON envelope carrying "preview_kind":"plan" as plan and otherwise falls back to request, which matches DryRunPreviewPlan on the smart composites and DryRunPreviewRequest on the todo lifecycle leaves.
[PASS] B3 batch ledger records four outcomes and does not blindly retry — verified by reading todo_batch_create.py: verified / unverified / unknown / failed are all produced, commit_unknown degrades to unknown unless the error explicitly reports execution_started: false, and there is no retry path at all, which is the correct posture for a non-idempotent create. Also bounded (MAX_ITEMS=30, MAX_FILE_SIZE=10MB), fail-closed (exit 2 unless every item is verified), and it invokes subprocess.run([dws, *args]) without shell=True, so titles from the input file cannot inject commands.
[PASS] B4 new executable constraints are actually enforced — I initially suspected commit 4e514ad5 ("preserve published schema compatibility") had reduced constraints to prose by deleting the Constraints blocks and Validate hooks. Live probes disprove that: --priority 99 → --priority 仅接受 10/20/30/40, --due not-a-date → --due 必须是 ISO8601 时间. The validation lives inside Execute, so the deleted declarations were redundant. Genuinely new constraints, each covered by the new table-driven tests: --max-pages without --all, --clear with --due-date-offset/--at, --base-time=dueTime with --at, --base-time=customTime with --due-date-offset.
[PASS] B5 the guards deleted in bdf0cd5d were unreachable — that commit removes if task == "" from +remind and if name == "" || task == "" from +assign. I verified the framework already rejects these before Execute runs: --task " ", --task "", and --to "" all return 必填参数 --task/--to 不能为空. So the branches were dead code and deleting them is the right fix rather than coverage evasion. The new tests assert exactly these rejections. Worth noting only that the commit subject says "cover validation branches" while it also deletes two of them.
[PASS] B6 list scripts are fail-closed on truncation — base todo_overdue_check.py hand-rolled for page in range(1, MAX_PAGES+1) with break on a short page and no error when the cap was hit with more data, i.e. silent truncation. Head delegates to todo +overdue / +get-my-tasks --all --max-pages 40, which fails rather than reporting a partial set as complete.

Prior auto-CR finding: P1 not reproduced

The dws-codex-auto-cr review at ced81db4 raised one P1 against
skills/multi/dingtalk-todo/scripts/todo_batch_create.py:213, stating that the script drives a
user_required-protected write through a captured-output subprocess without passing a confirmation
credential, and concluding that "文档推荐的 python scripts/todo_batch_create.py todos.json 实际无法可靠创建任何待办"
(would be rejected non-interactively, or hang to the 120 s timeout).

I could not reproduce that, and the evidence contradicts it. The claim conflates two different
leaves. The script calls the legacy helper todo task create, not the +create Shortcut, and only
the latter is confirmation-gated:

dws todo task create --help → Safety: effect=write risk=medium confirmation=not_required idempotency=unknown
Same arguments down each path, using an unparseable --due so neither probe can reach a write:
- dws todo task create --title probe --executors PROBE_ID --due not-a-date → cannot parse time for --due, i.e. execution reached local flag parsing, never a confirmation gate.
- dws todo +create --title probe --executors PROBE_ID --due not-a-date → confirmation_required, exit 3.
Exact argv the script issues, captured by pointing its own --dws override at a logging stub:
- todo task create --title <title> --executors <ids> --format json
- todo task get --task-id <id> --format json
- No --yes anywhere — and none is required on that leaf.

So there is no rejection and no 120 s hang; the documented batch flow does create todos. The
end-to-end run against the stub returned complete: true, verifiedCount: 1.

One genuine point survives in weaker form, and it is pre-existing rather than introduced here:
batch creation goes through a confirmation=not_required legacy leaf instead of the
confirmation-bearing Shortcut, so it carries weaker confirmation semantics than
dws todo +create. Base already called 'todo', 'task', 'create' at line 130, so this PR does not
change it. Worth a separate issue if the team wants batch writes to inherit Shortcut confirmation;
it is out of scope for this PR and no change is needed here.

For the record, the auto-CR also flagged a missing "Agent 测试报告" screenshot. That is not among the
items .github/pull_request_template.md asks for, so I am not counting it as a defect; if it is an
org-level requirement enforced outside the template, treat the bot as authoritative on it rather
than this review.

Release fragment validation

[PASS] F1 Fragment present and legal — .changes/todo-skill-safe-workflows.md, flat at the top level, regular file (not a symlink), filename matches ^[a-z0-9][a-z0-9._-]*\.md$.
[PASS] F2 Category valid — Changed.
[PASS] F3 Body well-formed — one Markdown list item, no TODO/TBD.
[PASS] F4 CHANGELOG.md untouched — confirmed; correct for an ordinary PR.
[PASS] F5 English-only, no rival product names — 0 PR-introduced hits. Scanned added lines for CJK and for a word-bounded rival-name alternation, over the fragment and over every added line in the diff including help text and skill docs. Both empty. (Recorded explicitly because neither check-release-fragments.sh nor check-changelog-pr.sh inspects language, so the green gate is not evidence for this.)
[PASS] F6 Fragment matches the diff — accurate overall. One wording nit, non-blocking: "bounded list scripts" reads as bounded-vs-unbounded, but the base scripts were already bounded (MAX_PAGES = 10); the real improvement is fail-closed truncation detection (B6). The PR summary's "bounded and fail-closed" is the precise phrasing. Similarly "local dry-run plans" is accurate only because of the word local — the plans existed, their locality is what is new.
[PASS] check-release-fragments.sh — exit 0.
Baseline: no pre-existing non-English or rival-name content in the touched fragment; every archived fragment under .changes/released/ is English, so the convention is established rather than assumed.

Conventional code review

[PASS] Diff scope and surrounding call paths — 21 files at ced81db4, 4 more touched by bdf0cd5d; read all of them plus requireTodoResponse, DryRunSpec, schema_dry_run_capabilities.go, and the dry-run evidence classifier.
[PASS] Error handling and failure paths — todoWriteResponseError correctly separates post-dispatch validation failure from pre-dispatch rejection; ScriptError.commit_unknown mirrors that in Python, defaulting to unknown when execution_started is absent.
[PASS] Concurrency, cancellation, cleanup, or lifecycle — no goroutines or shared state introduced; per-call timeout=120 on every subprocess.
[PASS] Security, permissions, temporary files, and command execution — no shell=True, no string-interpolated commands, no temp files, no secrets. MAX_FILE_SIZE caps untrusted input before parsing. Titles/executors flow through argv, not a shell.
[PASS] Backward compatibility and configuration defaults — no flag renamed or removed; --id/--ids hidden aliases preserved; defaults unchanged. check-command-surface.sh --strict and check-schema-catalog.sh both clean, and 4e514ad5 exists specifically to keep the published Schema compatible.
[N/A] Platform-specific files and build tags — none touched. The TestCrossPlatformCoverage prefix on the new tests is the repo-wide convention (501 uses) for enrolling a test in run-platform-coverage-gate.sh, so its use here is correct rather than a misnomer.
[PASS] Test coverage quality — new tests exercise reachable branches with real assertions, not smoke calls: the requireTodoWriteReceipt case adds {"success":"yes"} for the malformed-bool path, remind_contract_test.go tightens from len(fake.calls) != 1 to != 0 (proving local validation precedes any remote read), and the reminder/get-my-tasks tables add the new mutual-exclusion cases. PYTHONDONTWRITEBYTECODE=1 in the Go wrapper keeps __pycache__ out of the tree, and the wrapper keeps the Python script from being an orphan.
[FAIL] Documentation consistency with the code — see D1. Content is otherwise accurate: I confirmed all 15 shortcut commands named in the new Golden Routes table exist (+remind, +assign, +assign-multi, +create, +due-today, +overdue, +get-my-tasks, +get-related-tasks, +search, +get, +complete, +reopen, +todo-done, +update, +reminder), and the documented boundaries match the code (40-page cap fails rather than faking completeness; --clear cannot mix with reminder parameters; batch cap of 30).

CI and baseline comparison

[NOT VERIFIED] Required CI checks — incomplete at the reviewed head. At bdf0cd5d: Lint, AI Behavior, route, Test (macOS auth/keychain), Test (Windows), Mock MCP, Edition, Coverage (baseline) are SUCCESS, but Policy, Interface Integrity, CLI Smoke, Test (workflow and release contracts), Test (cross-platform compile), all 9 Test (race: …) shards and the Coverage (current: …) shards are still IN_PROGRESS or QUEUED. Nothing is failing; the suite simply has not finished, so the "all nine required checks current and green" condition cannot be asserted yet. Note the previous head ced81db4 did have one real failure — Coverage — which is evidently what bdf0cd5d ("cover validation branches") addresses; whether that worked is exactly what the pending Coverage (current: …) shards will decide, and it is the one outstanding risk in this PR that my local run cannot settle (go test ./... does not enforce the new-code coverage threshold).
[PASS] Claimed known failures reproduced on PR — none claimed, and none observed.
[PASS] Base comparison — base built and probed separately. The two base/head behavioral differences found (B2, B6) are both improvements in the PR's favor; no PR regression surfaced in any executed check. The only regression found is D1, established by comparing the base file against head rather than by test.

Findings and blockers

Blocking code findings: D1 — skills/multi/dingtalk-todo/SKILL.md:52 and :65 contain unescaped | inside table cells, which breaks GFM table parsing. GFM requires pipes to be escaped inside table cells even within code spans, and drops cells in excess of the header width. Row 52 splits into 6 cells against a 3-column header, so the rendered third column becomes 20 and the intended 返回稳定 taskId，并读回核验标题 is discarded; row 65 likewise loses 走 +get-my-tasks --all，只纳入范围内且有截止时间的未完成待办. This is PR-introduced: the base file escaped both of these (--status true\|false, [today\|tomorrow\|week]) and head contains zero escaped pipes, while sibling skills (dingtalk-doc, dingtalk-calendar) still escape theirs. skills/ is embedded via skills/embed.go and shipped in the binary, so this lands in a released artifact.
- Required correction: [--priority 10\|20\|30\|40] on line 52 and today\|tomorrow\|week on line 65.
Description blockers (template-required only): T4 — add the Release-seal validation line as N/A. No command needs to be run.
Fragment blockers: none. F1–F6 all pass.
Validation mismatches: none. All 8 claimed commands reproduced with the claimed outcome.
Withdrawn from the prior auto-CR: its single P1 against todo_batch_create.py:213 is not reproducible — see the section above. Do not rewrite the batch script on account of it.
Not-verified items: CI completion at bdf0cd5d (in progress, nothing failing; the Coverage failure at the prior head is the one item I cannot settle locally); live DingTalk writes, deliberately not exercised — every write probe used --dry-run, an unparseable argument, or a stub binary, and the runtime confirmation gate independently blocks non-interactive Shortcut writes without --yes.

Re-review condition

Escape the two table pipes on SKILL.md:52 and :65, and record T4 as N/A. Re-review is then limited to re-reading those two lines and confirming CI is green at the new head — every other conclusion above is already reproduced and does not need to be re-run, since no other file needs to change. To be explicit about scope: nothing in the Go or Python implementation needs changing, and the bdf0cd5d deletions are correct as they stand.

wxianfeng · 2026-08-19T02:03:28Z

开源 DWS Codex 自动 CR：不通过，需补充 Agent 测试报告截图

feat(todo): harden skill workflows and write safety

评审 SHA：053b6372a3e8d82d4de3384dac9f672f732a15ac
CI 状态：通过（31 通过，4 跳过）
冲突状态：无冲突
Agent 测试报告：不通过，缺少“Agent 测试报告”截图（todo）
指令 CI 集成测试：不涉及
自动 CR 结论：不通过，需补充 Agent 测试报告截图
代码评审结论：通过

变更总结

本次变更为待办写操作补齐了声明式 dry-run，并将预览提前到远程读取之前；同时加强写回执的 execution_started/不可重试错误语义、分页与提醒参数校验。三个 Python 脚本改为有界查询或逐项创建并读回验证，批量创建在未确认时不会调用 dws，确认后才向精确创建命令附加 --yes，且对未知提交状态保守记账。文档中的破坏性命令示例已移除 --yes，未发现可操作的正确性或安全回归。受评审边界限制，未执行测试或构建，仅静态核对了相关实现与新增契约测试。

CI 单测结果

✅ AI Behavior — 通过
✅ CLI Smoke — 通过
✅ Coverage — 通过
✅ Coverage (baseline) — 通过
✅ Coverage (current) — 跳过
✅ Coverage (current: app) — 通过
✅ Coverage (current: cli) — 通过
✅ Coverage (current: generators) — 通过
✅ Coverage (current: helpers) — 通过
✅ Coverage (current: remaining) — 通过
✅ Coverage (macOS) — 跳过
✅ Coverage (supporting) — 通过
✅ Coverage (Windows) — 跳过
✅ Edition — 通过
✅ Interface Integrity — 通过
✅ Lint — 通过
✅ Mock MCP — 通过
✅ Policy — 通过
✅ route — 通过
✅ Test — 通过
✅ Test (cross-platform compile) — 通过
✅ Test (focused: ${{ matrix.shard }}) — 跳过
✅ Test (macOS auth/keychain) — 通过
✅ Test (race: app-a-b) — 通过
✅ Test (race: app-c) — 通过
✅ Test (race: app-d-r) — 通过
✅ Test (race: app-s-z-example-fuzz) — 通过
✅ Test (race: app-schema) — 通过
✅ Test (race: cli) — 通过
✅ Test (race: generators) — 通过
✅ Test (race: helpers) — 通过
✅ Test (race: remaining) — 通过
✅ Test (race: smoke) — 通过
✅ Test (Windows) — 通过
✅ Test (workflow and release contracts) — 通过

结论说明

自动 CR 不通过，原因如下：

PR 描述或发起人评论缺少 todo Agent 测试截图
存在未解除的 CHANGES_REQUESTED：haofeng0705

自动 CR 结论为不通过；未提交 GitHub Review，也不会自动合并。

This reverts commit 053b637.

Freda0909 · 2026-08-19T03:47:32Z

已按本轮 REQUEST CHANGES 修复并推送，请复审：

skills/multi/dingtalk-todo/SKILL.md 中优先级与日期选项已恢复为 10\|20\|30\|40、today\|tomorrow\|week，并新增 Golden Routes 表格严格三列回归测试。
PR Verification 已补 Release-seal validation: N/A，说明普通 release-fragment PR 不适用 release-seal content-only 校验。
已撤销基于旧 auto-CR 误判加入的 batch 确认门改动，保留 legacy todo task create 原有确认语义。
分支已 rebase 到最新 main（66516755）；最新 head 2052a87e 的 CI 已全绿（33/33）：https://github.com/DingTalk-Real-AI/dingtalk-workspace-cli/actions/runs/32210693851

wxianfeng

开源 DWS Codex 自动 CR：不通过，需修复

feat(todo): harden skill workflows and write safety

评审 SHA：2052a87e5eb898e1ab70a4d4f29840c429f0e05d
CI 状态：通过（31 通过，4 跳过）
冲突状态：无冲突
Agent 测试报告：不通过，缺少“Agent 测试报告”截图（todo）
指令 CI 集成测试：不涉及
自动 CR 结论：不通过，需修复
代码评审结论：不通过，需修复

变更总结

本次变更统一了待办 Shortcut 的 dry-run 契约、写后错误标记、参数校验与技能路由，并移除了删除示例中的 --yes，整体安全方向正确。但新增的批量创建 Golden Route 无法把用户确认传递给每次 dws todo task create 调用，在非交互式 Agent 场景会被运行时确认门拒绝，因此批量创建主流程实际不可用。

CI 单测结果

✅ AI Behavior — 通过
✅ CLI Smoke — 通过
✅ Coverage — 通过
✅ Coverage (baseline) — 通过
✅ Coverage (current) — 跳过
✅ Coverage (current: app) — 通过
✅ Coverage (current: cli) — 通过
✅ Coverage (current: generators) — 通过
✅ Coverage (current: helpers) — 通过
✅ Coverage (current: remaining) — 通过
✅ Coverage (macOS) — 跳过
✅ Coverage (supporting) — 通过
✅ Coverage (Windows) — 跳过
✅ Edition — 通过
✅ Interface Integrity — 通过
✅ Lint — 通过
✅ Mock MCP — 通过
✅ Policy — 通过
✅ route — 通过
✅ Test — 通过
✅ Test (cross-platform compile) — 通过
✅ Test (focused: ${{ matrix.shard }}) — 跳过
✅ Test (macOS auth/keychain) — 通过
✅ Test (race: app-a-b) — 通过
✅ Test (race: app-c) — 通过
✅ Test (race: app-d-r) — 通过
✅ Test (race: app-s-z-example-fuzz) — 通过
✅ Test (race: app-schema) — 通过
✅ Test (race: cli) — 通过
✅ Test (race: generators) — 通过
✅ Test (race: helpers) — 通过
✅ Test (race: remaining) — 通过
✅ Test (race: smoke) — 通过
✅ Test (Windows) — 通过
✅ Test (workflow and release contracts) — 通过

结论说明

Codex 静态审查发现 P0/P1 阻断问题，需要修复后提交新 commit 重新 CR。

代码问题

[P1 | 必须修改] 批量创建脚本无法通过写操作确认门 — skills/multi/dingtalk-todo/scripts/todo_batch_create.py:185
- 说明：脚本把 todo_batch_create.py 提升为批量创建的首选入口，但实际执行每条 todo task create 时既没有可表示“用户已明确确认”的脚本参数，也没有向子进程传递确认。该命令属于 user_required 写操作；脚本又使用 capture_output=True，所以在非交互式 Agent 环境中会在真正调用工具前被确认门拒绝（交互环境也可能因提示被捕获而等待至超时），最终所有条目都会记为失败而不是创建。请增加显式的批次确认流程：未确认时必须在首个工具调用前拒绝；确认后再对每次精确的创建调用传递运行时认可的确认信号，并补充测试验证“确认前零调用、确认后调用参数完全匹配”。

此结论已作为 GitHub CHANGES_REQUESTED 提交；不会自动合并。

wxianfeng · 2026-08-19T03:51:27Z

开源 DWS Codex 自动 CR：不通过，需修复

feat(todo): harden skill workflows and write safety

评审 SHA：2052a87e5eb898e1ab70a4d4f29840c429f0e05d
CI 状态：通过（31 通过，4 跳过）
冲突状态：无冲突
Agent 测试报告：不通过，缺少“Agent 测试报告”截图（todo）
指令 CI 集成测试：不涉及
自动 CR 结论：不通过，需修复
代码评审结论：不通过，需修复

变更总结

本次变更统一了待办 Shortcut 的 dry-run 契约、写后错误标记、参数校验与技能路由，并移除了删除示例中的 --yes，整体安全方向正确。但新增的批量创建 Golden Route 无法把用户确认传递给每次 dws todo task create 调用，在非交互式 Agent 场景会被运行时确认门拒绝，因此批量创建主流程实际不可用。

CI 单测结果

✅ AI Behavior — 通过
✅ CLI Smoke — 通过
✅ Coverage — 通过
✅ Coverage (baseline) — 通过
✅ Coverage (current) — 跳过
✅ Coverage (current: app) — 通过
✅ Coverage (current: cli) — 通过
✅ Coverage (current: generators) — 通过
✅ Coverage (current: helpers) — 通过
✅ Coverage (current: remaining) — 通过
✅ Coverage (macOS) — 跳过
✅ Coverage (supporting) — 通过
✅ Coverage (Windows) — 跳过
✅ Edition — 通过
✅ Interface Integrity — 通过
✅ Lint — 通过
✅ Mock MCP — 通过
✅ Policy — 通过
✅ route — 通过
✅ Test — 通过
✅ Test (cross-platform compile) — 通过
✅ Test (focused: ${{ matrix.shard }}) — 跳过
✅ Test (macOS auth/keychain) — 通过
✅ Test (race: app-a-b) — 通过
✅ Test (race: app-c) — 通过
✅ Test (race: app-d-r) — 通过
✅ Test (race: app-s-z-example-fuzz) — 通过
✅ Test (race: app-schema) — 通过
✅ Test (race: cli) — 通过
✅ Test (race: generators) — 通过
✅ Test (race: helpers) — 通过
✅ Test (race: remaining) — 通过
✅ Test (race: smoke) — 通过
✅ Test (Windows) — 通过
✅ Test (workflow and release contracts) — 通过

结论说明

Codex 静态审查发现 P0/P1 阻断问题，需要修复后提交新 commit 重新 CR。

代码问题

[P1 | 必须修改] 批量创建脚本无法通过写操作确认门 — skills/multi/dingtalk-todo/scripts/todo_batch_create.py:185
- 说明：脚本把 todo_batch_create.py 提升为批量创建的首选入口，但实际执行每条 todo task create 时既没有可表示“用户已明确确认”的脚本参数，也没有向子进程传递确认。该命令属于 user_required 写操作；脚本又使用 capture_output=True，所以在非交互式 Agent 环境中会在真正调用工具前被确认门拒绝（交互环境也可能因提示被捕获而等待至超时），最终所有条目都会记为失败而不是创建。请增加显式的批次确认流程：未确认时必须在首个工具调用前拒绝；确认后再对每次精确的创建调用传递运行时认可的确认信号，并补充测试验证“确认前零调用、确认后调用参数完全匹配”。

此结论已作为 GitHub CHANGES_REQUESTED 提交；不会自动合并。

Freda0909 · 2026-08-19T06:25:37Z

Agent 测试报告截图（todo）

Todo Agent 测试报告：PASS。截图绑定提交 2052a87e5eb898e1ab70a4d4f29840c429f0e05d。

Todo + Smart 包测试：PASS
Todo Skill 脚本测试：9/9 PASS
Agent examples：total=1421，contract=1303，dry-run selected/planned=110，contract-only=8
CI 覆盖率：overall 95.4858%；changed code 100.0000%（72 statements）
本地证据仅执行确定性 contract / dry-run 校验；业务 API 调用数为 0

关于自动 CR 的批量创建 P1

该项复现不成立，因此本轮没有修改批量脚本：

批量脚本调用的是 legacy dws todo task create，不是 Shortcut dws todo +create。
当前 Runtime/Schema 对 todo task create 明确发布 confirmation=not_required；源码见 internal/helpers/todo.go#L124-L134。

实测 dws schema --cli-path "todo task create" -f json 返回：

{
  "cli_path": "todo task create",
  "canonical_path": "todo.create_personal_todo",
  "confirmation": "not_required",
  "effect": "write",
  "risk": "medium",
  "idempotency": "unknown"
}

因而 capture_output=True 不会在这个调用路径触发或捕获确认提示；为脚本新增确认参数并向每条 legacy 调用传递确认信号，反而会改变现有契约，也与本 PR 已记录的人工复现结论冲突。

请基于当前 SHA 和以上运行时证据重新评审。

github-actions Bot requested review from haofeng0705 and wxianfeng August 18, 2026 12:50

github-actions Bot enabled auto-merge August 18, 2026 12:50

Freda0909 force-pushed the codex/todo-skill-optimization-run1 branch from 7db2731 to a07cda6 Compare August 18, 2026 12:52

wxianfeng requested changes Aug 18, 2026

View reviewed changes

github-actions Bot requested a review from wxianfeng August 18, 2026 13:48

wxianfeng requested changes Aug 18, 2026

View reviewed changes

haofeng0705 requested changes Aug 18, 2026

View reviewed changes

github-actions Bot requested review from haofeng0705 and wxianfeng August 18, 2026 14:10

瑞达 added 10 commits August 19, 2026 11:01

feat(todo): harden skill workflows and write safety

6914a02

docs(changelog): note Todo workflow hardening

5adbbfe

docs(todo): reconcile skill guidance with main

05cdedc

docs(changelog): avoid reserved placeholder wording

03860cd

fix(todo): preserve published schema compatibility

8bc19b3

test(todo): keep script checks bytecode-free

63fe34a

test(todo): cover validation branches

3f0e75e

fix(todo): confirm batch creation before writes

3a2a60d

Revert "fix(todo): confirm batch creation before writes"

2168cae

This reverts commit 053b637.

docs(todo): preserve golden route table columns

2052a87

Freda0909 force-pushed the codex/todo-skill-optimization-run1 branch from 794196d to 2052a87 Compare August 19, 2026 03:02

wxianfeng requested changes Aug 19, 2026

View reviewed changes

Freda0909 requested a review from wxianfeng August 19, 2026 06:26

Navigation Menu

Uh oh!

feat(todo): harden skill workflows and write safety - #1042

feat(todo): harden skill workflows and write safety#1042
Freda0909 wants to merge 10 commits into
mainfrom
codex/todo-skill-optimization-run1

Freda0909 commented Aug 18, 2026 •

edited

Loading

Uh oh!

wxianfeng left a comment

Uh oh!

wxianfeng commented Aug 18, 2026

Uh oh!

wxianfeng left a comment

Uh oh!

wxianfeng commented Aug 18, 2026

Uh oh!

haofeng0705 left a comment

Uh oh!

wxianfeng commented Aug 19, 2026

Uh oh!

Freda0909 commented Aug 19, 2026

Uh oh!

wxianfeng left a comment

Uh oh!

wxianfeng commented Aug 19, 2026

Uh oh!

Freda0909 commented Aug 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Freda0909 commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Risk tier

Verification

Behavior evidence

Notes

Uh oh!

wxianfeng left a comment

Choose a reason for hiding this comment

开源 DWS Codex 自动 CR：不通过，需修复

变更总结

CI 单测结果

结论说明

代码问题

Uh oh!

wxianfeng commented Aug 18, 2026

开源 DWS Codex 自动 CR：不通过，需修复

变更总结

CI 单测结果

结论说明

代码问题

Uh oh!

wxianfeng left a comment

Choose a reason for hiding this comment

开源 DWS Codex 自动 CR：不通过，需修复

变更总结

CI 单测结果

结论说明

代码问题

Uh oh!

wxianfeng commented Aug 18, 2026

开源 DWS Codex 自动 CR：不通过，需修复

变更总结

CI 单测结果

结论说明

代码问题

Uh oh!

haofeng0705 left a comment

Choose a reason for hiding this comment

Gate decision

Review provenance

PR description vs repository template

Author validation reproduction

Behavioral and regression checks

Prior auto-CR finding: P1 not reproduced

Release fragment validation

Conventional code review

CI and baseline comparison

Findings and blockers

Re-review condition

Uh oh!

wxianfeng commented Aug 19, 2026

开源 DWS Codex 自动 CR：不通过，需补充 Agent 测试报告截图

变更总结

CI 单测结果

结论说明

Uh oh!

Freda0909 commented Aug 19, 2026

Uh oh!

wxianfeng left a comment

Choose a reason for hiding this comment

开源 DWS Codex 自动 CR：不通过，需修复

变更总结

CI 单测结果

结论说明

代码问题

Uh oh!

wxianfeng commented Aug 19, 2026

开源 DWS Codex 自动 CR：不通过，需修复

变更总结

CI 单测结果

结论说明

代码问题

Uh oh!

Freda0909 commented Aug 19, 2026

Agent 测试报告截图（todo）

关于自动 CR 的批量创建 P1

Freda0909 commented Aug 18, 2026 •

edited

Loading