Conversation
- guard preflight 移除 phase 检查，消除 design/build/verify 阶段死锁 - comet-state check 参数顺序统一为 <name> <phase>，与其他子命令一致 - init 统一设 phase=open，guard 负责 workflow-aware 过渡 (full→design, hotfix/tweak→build) - comet-open 重构步骤顺序：init 在 entry check 之前执行 - comet-verify 验证失败时设置 verify_result=fail - build entry check 对 hotfix/tweak 跳过 design_doc，修复 design_doc 路径拼接错误 - comet-build 添加上下文管理和断点恢复指引 - 主 SKILL.md 增强连续执行要求和意图判定 - 脚本定位 find 失败添加降级处理 - YAML validator 和 set 枚举加入 phase=open - 测试全面适配新行为
State machine hardening:
- verify-pass now requires verification_report (file exists) and
branch_status=handled before allowing phase advance
- Guard checks these as hard prerequisites during verify phase
- New .comet.yaml fields: verification_report, branch_status
Skill cleanup (zh + en):
- Hotfix root cause check moved before comet-verify (3a/3b split)
- Removed non-action steps from comet-design (dual spec table, doc hierarchy)
- Removed duplicate script location blocks in comet-open, comet-archive
- Removed duplicate 50% threshold in comet-build
- Generic error handling ("Maven" → "Build/test")
- Hotfix header simplified for standalone invocation
Tests:
- New tests for verification evidence blocking, branch_status validation
- Bats CRLF fix for Windows, cross-platform test runner
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates Comet to version 0.2.4, introducing a formal state transition system and enforcing verification evidence requirements, such as report existence and branch status, before phase advancement. It also enhances cross-platform support for shell tests via a new Node.js runner. Review feedback points out a regression in the build verification logic that could block projects without standard build systems, suggests using existing helper functions for YAML parsing to reduce duplication, and recommends adding default setup/teardown implementations to the test runner to prevent execution failures.
| cargo build >/dev/null | ||
| return $? | ||
| fi | ||
| return 1 |
There was a problem hiding this comment.
The build_passes function now returns 1 if no recognized build configuration file (package.json, pom.xml, Cargo.toml) is found. This is a regression that will block the phase guard for projects using other languages (e.g., Python, Go, C++) or those without an automated build step. It should return 0 (success) by default if no build system is detected, assuming there is no automated build step to verify.
| return 1 | |
| return 0 |
| open) green " [APPLY] .comet.yaml updated: phase=design" ;; | ||
| open) | ||
| local new_phase | ||
| new_phase=$(grep "^phase:" "$CHANGE_DIR/.comet.yaml" | sed 's/^phase: *//' | tr -d '"' | tr -d "'") |
There was a problem hiding this comment.
This manual parsing of the phase field duplicates the logic already implemented in the yaml_field_value helper function. Using the helper would improve consistency and maintainability.
| new_phase=$(grep "^phase:" "$CHANGE_DIR/.comet.yaml" | sed 's/^phase: *//' | tr -d '"' | tr -d "'") | |
| new_phase=$(yaml_field_value "phase") |
| local plan_file base_ref | ||
| plan_file=$(cmd_get "$change_name" "plan" 2>/dev/null || true) | ||
| if [ -n "$plan_file" ] && [ "$plan_file" != "null" ] && [ -f "$plan_file" ]; then | ||
| base_ref=$(grep '^base-ref:' "$plan_file" 2>/dev/null | head -1 | sed 's/^base-ref: *//') |
There was a problem hiding this comment.
The manual extraction of base-ref can be replaced with the yaml_field helper function defined earlier in the script. This ensures consistent handling of quotes and whitespace.
| base_ref=$(grep '^base-ref:' "$plan_file" 2>/dev/null | head -1 | sed 's/^base-ref: *//') | |
| base_ref=$(yaml_field "base-ref" "$plan_file") |
| ' status=$?', | ||
| ' return 0', | ||
| '}', | ||
| ...prelude, |
There was a problem hiding this comment.
The generated test runner calls setup and teardown functions unconditionally. If a .bats file does not define these functions, the test execution will fail with a "command not found" error. It is safer to provide empty default implementations in the script prelude.
'setup() { :; }',
'teardown() { :; }',
...prelude,1 participant
No description provided.