diff --git a/.agents/AGENTS.md b/.agents/AGENTS.md index 9431a682..c70d428b 100644 --- a/.agents/AGENTS.md +++ b/.agents/AGENTS.md @@ -104,6 +104,14 @@ Use `/save-todo` after planning. Auto-detects complexity: **Dependencies**: `blocked-by:t001`, `blocks:t002`, `t001.1` (subtask) +**Task completion rules:** +- NEVER mark a task `[x]` unless YOU wrote code/content for it in THIS session +- If a worker branch or merged PR completed the task, leave it `[ ]` — the repo owner or reviewer marks it done +- Before marking `[x]`, verify: deliverable file exists AND content is substantive (not a stub/placeholder) +- Marking `[x]` triggers `issue-sync` to auto-close the linked GitHub Issue — false completions cascade + +_See `workflows/plans.md` "Task Completion Rules" for detailed guidance._ + **After ANY TODO/planning edit** (interactive sessions only, NOT workers): Commit and push immediately. Planning-only files (TODO.md, todo/) go directly to main -- no branch, no PR. Mixed changes (planning + non-exception files) use a worktree. NEVER `git checkout -b` in the main repo. Workers must NOT edit TODO.md -- see `workflows/plans.md` "Worker TODO.md Restriction". **Full docs**: `workflows/plans.md`, `tools/task-management/beads.md` diff --git a/.agents/scripts/issue-sync-helper.sh b/.agents/scripts/issue-sync-helper.sh index 49176c90..c0f9a36d 100755 --- a/.agents/scripts/issue-sync-helper.sh +++ b/.agents/scripts/issue-sync-helper.sh @@ -12,7 +12,7 @@ # push [tNNN] Create/update GitHub issues from TODO.md tasks # enrich [tNNN] Update existing issue bodies with full context # pull Sync GitHub issue refs back to TODO.md -# close [tNNN] Close GitHub issue when TODO.md task is [x] +# close [tNNN] Close GitHub issue when TODO.md task is [x] (requires verified: field) # status Show sync drift between TODO.md and GitHub # parse [tNNN] Parse and display task context (dry-run) # help Show this help message @@ -20,6 +20,7 @@ # Options: # --repo SLUG Override repo slug (default: auto-detect from git remote) # --dry-run Show what would be done without making changes +# --force Skip verified: field check in close command # --verbose Show detailed output # # Part of aidevops framework: https://aidevops.sh @@ -35,6 +36,7 @@ source "${SCRIPT_DIR}/shared-constants.sh" VERBOSE="${VERBOSE:-false}" DRY_RUN="${DRY_RUN:-false}" +FORCE="${FORCE:-false}" REPO_SLUG="" # ============================================================================= @@ -961,8 +963,10 @@ add_gh_ref_to_todo() { } # Close: close GitHub issue when TODO.md task is completed +# Safety: requires verified: field or --force flag to prevent false closure cascades cmd_close() { local target_task="${1:-}" + local force="${FORCE:-false}" local project_root project_root=$(find_project_root) || return 1 local repo_slug="${REPO_SLUG:-$(detect_repo_slug "$project_root")}" @@ -990,6 +994,7 @@ cmd_close() { fi local closed=0 + local skipped=0 for task_id in "${tasks[@]}"; do local task_line task_line=$(grep -E "^- \[.\] ${task_id} " "$todo_file" | head -1 || echo "") @@ -1000,6 +1005,16 @@ cmd_close() { continue fi + # Safety guard: require verified: field unless --force + # This prevents false completion cascades (see PR #616 incident) + if [[ "$force" != "true" ]]; then + if ! echo "$task_line" | grep -qE 'verified:[0-9]{4}-[0-9]{2}-[0-9]{2}'; then + log_verbose "Skipping #$issue_number ($task_id): no verified: field (use --force to override)" + skipped=$((skipped + 1)) + continue + fi + fi + # Check if issue is already closed local issue_state issue_state=$(gh issue view "$issue_number" --repo "$repo_slug" --json state --jq '.state' 2>/dev/null || echo "") @@ -1014,7 +1029,13 @@ cmd_close() { continue fi - if gh issue close "$issue_number" --repo "$repo_slug" --comment "Completed. Task $task_id marked done in TODO.md." 2>/dev/null; then + local close_comment + if [[ "$force" == "true" ]]; then + close_comment="Completed. Task $task_id marked done in TODO.md (force-closed, verified: field not checked)." + else + close_comment="Completed. Task $task_id marked done in TODO.md (verified)." + fi + if gh issue close "$issue_number" --repo "$repo_slug" --comment "$close_comment" 2>/dev/null; then print_success "Closed #$issue_number ($task_id)" closed=$((closed + 1)) else @@ -1022,7 +1043,7 @@ cmd_close() { fi done - print_info "Close complete: $closed issues closed" + print_info "Close complete: $closed issues closed, $skipped skipped (no verified: field)" return 0 } @@ -1174,7 +1195,7 @@ Commands: push [tNNN] Create GitHub issues from TODO.md tasks (all open or specific) enrich [tNNN] Update existing issue bodies with full context from PLANS.md pull Sync GitHub issue refs back to TODO.md - close [tNNN] Close GitHub issues for completed TODO.md tasks + close [tNNN] Close GitHub issues for completed TODO.md tasks (requires verified: field) status Show sync drift between TODO.md and GitHub parse [tNNN] Parse and display task context (debug/dry-run) help Show this help message @@ -1182,6 +1203,7 @@ Commands: Options: --repo SLUG Override repo slug (default: auto-detect from git remote) --dry-run Show what would be done without making changes + --force Skip verified: field check in close command --verbose Show detailed output Examples: @@ -1190,7 +1212,8 @@ Examples: issue-sync-helper.sh enrich t020 # Enrich issue with plan context issue-sync-helper.sh enrich # Enrich all open tasks with refs issue-sync-helper.sh pull # Sync GH refs to TODO.md - issue-sync-helper.sh close # Close issues for done tasks + issue-sync-helper.sh close # Close verified done tasks + issue-sync-helper.sh close --force # Close all done tasks (skip verified: check) issue-sync-helper.sh status # Show sync drift issue-sync-helper.sh parse t020 # Debug: show parsed context issue-sync-helper.sh push --dry-run # Preview without changes @@ -1213,6 +1236,10 @@ main() { DRY_RUN="true" shift ;; + --force) + FORCE="true" + shift + ;; --verbose) VERBOSE="true" shift diff --git a/.agents/scripts/supervisor-helper.sh b/.agents/scripts/supervisor-helper.sh index e2eb7b5f..772ba4bb 100755 --- a/.agents/scripts/supervisor-helper.sh +++ b/.agents/scripts/supervisor-helper.sh @@ -4853,6 +4853,59 @@ cmd_pulse() { log_info " Cleaned: $orphan_killed stale worker processes" fi + # Phase 6: Verify deliverables for merged tasks (t163.4) + # Before marking tasks complete in TODO.md, verify that PRs are actually merged. + # Uses state_log to track verification (avoids needing a separate notes column). + local verified_count=0 + local merged_unverified + # Single query: fetch id, pr_url, repo for deployed tasks not yet verified + merged_unverified=$(db "$SUPERVISOR_DB" " + SELECT t.id, t.pr_url, t.repo FROM tasks t + WHERE t.status = 'deployed' + AND t.id NOT IN ( + SELECT task_id FROM state_log WHERE reason LIKE 'verified:%' + ); + " 2>/dev/null || echo "") + if [[ -n "$merged_unverified" ]]; then + while IFS='|' read -r verify_tid verify_pr_url verify_repo; do + [[ -n "$verify_tid" ]] || continue + + # Check PR is actually merged + local pr_merged="false" + if [[ -n "$verify_pr_url" ]]; then + local pr_number + pr_number=$(echo "$verify_pr_url" | grep -oE '[0-9]+$' || echo "") + # Extract repo slug from PR URL (e.g., https://github.com/owner/repo/pull/123) + local pr_repo_slug + pr_repo_slug=$(echo "$verify_pr_url" | sed -E 's|.*/([^/]+/[^/]+)/pull/[0-9]+$|\1|' || echo "") + if [[ -n "$pr_number" && -n "$pr_repo_slug" ]]; then + local pr_state + pr_state=$(gh pr view "$pr_number" --repo "$pr_repo_slug" --json state --jq '.state' 2>/dev/null || echo "") + if [[ "$pr_state" == "MERGED" ]]; then + pr_merged="true" + fi + fi + fi + + if [[ "$pr_merged" == "true" ]]; then + # PR is merged — record verification in state_log + local escaped_verify_tid + escaped_verify_tid=$(sql_escape "$verify_tid") + db "$SUPERVISOR_DB" " + INSERT INTO state_log (task_id, from_state, to_state, reason) + VALUES ('$escaped_verify_tid', 'deployed', 'deployed', 'verified:$(date +%Y-%m-%d)'); + " 2>/dev/null || true + log_info " Verified: $verify_tid (PR merged)" + verified_count=$((verified_count + 1)) + else + log_warn " Unverified: $verify_tid (PR not merged or not found)" + fi + done <<< "$merged_unverified" + fi + if [[ "$verified_count" -gt 0 ]]; then + log_info " Verified: $verified_count task deliverables" + fi + # Phase 7: Reconcile TODO.md for any stale tasks (t160) # Runs when completed tasks exist and nothing is actively running/queued if [[ "$total_running" -eq 0 && "$total_queued" -eq 0 && "$total_complete" -gt 0 ]]; then diff --git a/.agents/workflows/plans.md b/.agents/workflows/plans.md index dee5510d..8ac29e46 100644 --- a/.agents/workflows/plans.md +++ b/.agents/workflows/plans.md @@ -686,6 +686,21 @@ An "always switch branches for TODO.md" rule fails the 80% universal applicabili **Bottom line**: Use judgment. Related work stays together; unrelated TODO-only backlog goes directly to main; mixed changes use a worktree. +## MANDATORY: Task Completion Rules + +**NEVER mark a task `[x]` unless ALL of these are true:** + +1. **YOU wrote code/content** for it in THIS session (not just verified a file exists) +2. **Deliverable is substantive** — file exists, content is complete (not a stub/placeholder), passes quality checks +3. **You can cite what you did** — specific files created/modified, lines changed +4. **Add `verified:YYYY-MM-DD`** to the task line — `issue-sync close` requires this field to auto-close the linked GitHub Issue + +**If someone else did the work** (worker branch, merged PR, another session): leave it `[ ]`. The repo owner or reviewer marks it done after verification. + +**Why this matters**: Marking `[x]` triggers `issue-sync` GitHub Action which auto-closes the linked GitHub Issue. False completions cascade — 38 issues were incorrectly closed in a single incident (PR #616). + +**The `actual:` field** should only appear on completed `[x]` tasks. Remove it when reverting a task to `[ ]`. + ## MANDATORY: Worker TODO.md Restriction **Workers (headless dispatch runners) must NEVER edit TODO.md directly.** This is the primary cause of merge conflicts when multiple workers + supervisor all push to TODO.md on main simultaneously. diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index b4a1f41b..79f45f39 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -130,6 +130,59 @@ jobs: echo "" echo "All shell scripts passed ShellCheck" + todo-validation: + name: TODO.md Task Completion Validation + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Check for unverified task completions + run: | + # Compare TODO.md between base and head to find new [x] transitions + BASE="${{ github.event.pull_request.base.sha }}" + HEAD="${{ github.event.pull_request.head.sha }}" + + # Get tasks that changed from [ ] to [x] in this PR + # comm -12 finds the intersection: IDs that were [ ] in BASE AND [x] in HEAD + BASE_OPEN=$(git show "$BASE":TODO.md 2>/dev/null | grep -E '^\- \[ \] t[0-9]+' | grep -oE 't[0-9]+' | sort || true) + HEAD_DONE=$(git show "$HEAD":TODO.md 2>/dev/null | grep -E '^\- \[x\] t[0-9]+' | grep -oE 't[0-9]+' | sort || true) + NEW_COMPLETIONS=$(comm -12 <(echo "$BASE_OPEN") <(echo "$HEAD_DONE") || true) + + if [[ -z "$NEW_COMPLETIONS" ]]; then + echo "No new task completions in this PR" + exit 0 + fi + + echo "New task completions found:" + echo "$NEW_COMPLETIONS" + echo "" + + WARNINGS=0 + while IFS= read -r task_id; do + [[ -z "$task_id" ]] && continue + TASK_LINE=$(git show "$HEAD":TODO.md | grep -E "^\- \[x\] ${task_id} " | head -1 || true) + + # Check for verified: field + if ! echo "$TASK_LINE" | grep -qE 'verified:[0-9]{4}-[0-9]{2}-[0-9]{2}'; then + echo "WARNING: $task_id marked [x] without verified: field" + WARNINGS=$((WARNINGS + 1)) + else + echo "OK: $task_id has verified: field" + fi + done <<< "$NEW_COMPLETIONS" + + if [[ "$WARNINGS" -gt 0 ]]; then + echo "" + echo "WARNING: $WARNINGS task(s) marked complete without verified: field." + echo "Add 'verified:YYYY-MM-DD' to confirm completion, or issue-sync will skip auto-closing the linked GitHub Issue." + echo "This is a warning only — the PR can still merge." + fi + sonarcloud: name: SonarCloud Analysis runs-on: ubuntu-latest