t2060: fix(task-complete-helper): move completed entries to ## Done instead of in-place marking#18760
t2060: fix(task-complete-helper): move completed entries to ## Done instead of in-place marking#18760alex-solovyev wants to merge 2 commits intomainfrom
Conversation
… of in-place marking Fixes t2060 / Resolves #18746. Part 1 — Behavior change: replace the sed in-place substitution in complete_task() (task-complete-helper.sh:353) with a two-pass awk extract-and-move approach. The function now: - Captures the parent task line + all indented children as a block - Transforms the parent line ([ ] -> [x], appends proof_log + completed:DATE) - Removes the block from its original location - Inserts the block at the top of the ## Done section - Verifies the move via post-edit grep + awk checks - Errors out clearly when ## Done header is missing - Preserves existing subtask guards, backup/restore, and proof-log validation logic unchanged Part 2 — Retroactive cleanup: moved 561 completed [x] entries that had accumulated in ## Ready (3 entries) and ## Backlog (558 entries) into ## Done via a one-time Python batch operation. - Total [x] count before = after = 804 (no entries lost or duplicated) - ## Ready and ## Backlog now contain zero [x] entries Part 3 — Test harness: new tests/test-task-complete-move.sh with 10 assertions covering 7 edge cases: - Single-line task (no subtasks) - Task with indented subtasks (children move with parent) - Task blocked by open subtasks (cannot complete) - Task already in ## Done (idempotent) - Task in ## In Progress (moves to Done) - ## Done header missing (errors out clearly) - Multiple consecutive entries (block boundary doesn't bleed) - Proof-log + completed:date correctly appended
alex-solovyev
added
the
origin:worker
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Merge SummaryTask: t2060 — fix(task-complete-helper): move completed entries to ## Done What was done:
Verification:
Root cause fixed: |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 41 minutes and 7 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ 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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report SonarCloud: 0 bugs, 0 vulnerabilities, 226 code smells Mon Apr 13 23:41:01 UTC 2026: Code review monitoring started 📈 Current Quality Metrics
Generated on: Mon Apr 13 23:41:04 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
Up to standards ✅🟢 Issues
|
…00-line gate Main branch ratcheted FUNCTION_COMPLEXITY_THRESHOLD to 30 (PR #18755); the initial complete_task() rewrite was 174 lines, pushing violations to 31. Extract two helpers: _check_task_subtasks_open() — 45 lines (moved subtask guard logic) _move_task_block_to_done() — 60 lines (awk extract + insert passes) complete_task() — 67 lines (thin orchestrator, was 174) No behaviour change; all 10 test assertions still pass.
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report SonarCloud: 0 bugs, 0 vulnerabilities, 228 code smells Mon Apr 13 23:49:15 UTC 2026: Code review monitoring started 📈 Current Quality Metrics
Generated on: Mon Apr 13 23:49:17 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
|
Closing — this PR has merge conflicts with the base branch. The work for this task ( Closed by deterministic merge pass (pulse-wrapper.sh, GH#17574). |
1 similar comment
|
Closing — this PR has merge conflicts with the base branch. The work for this task ( Closed by deterministic merge pass (pulse-wrapper.sh, GH#17574). |
|
Reopened — this PR was closed in error by the deterministic merge pass ( The closer cited PR #18749 as having "already landed on main", but #18749 is a planning-only PR — it added
The branch Root cause of the false closure: A framework fix for this false positive is being filed and shipped as an urgent patch release. Once the fix is in, this PR can be rebased and re-merged safely. For #18746 |
|
Closing — this PR has merge conflicts with the base branch. The work for this task ( Closed by deterministic merge pass (pulse-wrapper.sh, GH#17574). |
…icting worker PRs The deterministic merge pass closed PR #18760 with a false 'work has already landed on main' claim, citing PR #18749 as the merging PR. PR #18749 was a planning-only PR (added briefs, modified TODO.md) — it touched zero implementation files. The closing PR's task ID appeared in #18749's commit subject ('plan(t2059, t2060): file follow-ups...') and the task-ID grep matched, but no implementation work had landed. Fix: require non-planning file overlap between the closing PR and the matching commit before claiming 'work landed on main'. On overlap miss OR file-lookup failure, fail CLOSED — leave the PR open and post a one-time rebase nudge instead of discarding work. Three new helpers: - _is_planning_path_for_overlap: classifies TODO.md, README.md, CHANGELOG.md, VERSION, todo/**, and simplification-state.json as planning paths that cannot satisfy the duplicate-work signal. - _verify_pr_overlaps_commit: fetches the closing PR's files and the matching commit's files, returns 0 only on non-planning intersection; returns 1 on lookup failure (fail-CLOSED). - _post_rebase_nudge_on_worker_conflicting: idempotent rebase nudge modelled on _post_rebase_nudge_on_interactive_conflicting, posted exactly once per affected worker PR. Refactored _close_conflicting_pr to fetch (sha, subject) JSON from gh api commits and use jq for word-boundary matching, so the matching commit's files can be looked up by SHA. Test coverage: extends test-close-conflicting-pr-wording.sh from 3 to 7 assertions covering 4 new GH#18815 regressions: - Planning-only match leaves PR open (the #18760 incident exactly) - Real implementation overlap still closes PR - commit-files lookup failure → fail-CLOSED - PR-files lookup failure → fail-CLOSED
…icting worker PRs (#18820) The deterministic merge pass closed PR #18760 with a false 'work has already landed on main' claim, citing PR #18749 as the merging PR. PR #18749 was a planning-only PR (added briefs, modified TODO.md) — it touched zero implementation files. The closing PR's task ID appeared in #18749's commit subject ('plan(t2059, t2060): file follow-ups...') and the task-ID grep matched, but no implementation work had landed. Fix: require non-planning file overlap between the closing PR and the matching commit before claiming 'work landed on main'. On overlap miss OR file-lookup failure, fail CLOSED — leave the PR open and post a one-time rebase nudge instead of discarding work. Three new helpers: - _is_planning_path_for_overlap: classifies TODO.md, README.md, CHANGELOG.md, VERSION, todo/**, and simplification-state.json as planning paths that cannot satisfy the duplicate-work signal. - _verify_pr_overlaps_commit: fetches the closing PR's files and the matching commit's files, returns 0 only on non-planning intersection; returns 1 on lookup failure (fail-CLOSED). - _post_rebase_nudge_on_worker_conflicting: idempotent rebase nudge modelled on _post_rebase_nudge_on_interactive_conflicting, posted exactly once per affected worker PR. Refactored _close_conflicting_pr to fetch (sha, subject) JSON from gh api commits and use jq for word-boundary matching, so the matching commit's files can be looked up by SHA. Test coverage: extends test-close-conflicting-pr-wording.sh from 3 to 7 assertions covering 4 new GH#18815 regressions: - Planning-only match leaves PR open (the #18760 incident exactly) - Real implementation overlap still closes PR - commit-files lookup failure → fail-CLOSED - PR-files lookup failure → fail-CLOSED
|
Final resolution — work is preserved, fix is shipped, system self-validated. This PR is now legitimately closed because the t2060 implementation actually landed via PR #18806 (a fresh re-implementation by a parallel worker session, merged at 2026-04-14T02:51:03Z). The current closing comment cites #18806, not the original false-positive #18749, because the deterministic merge pass re-ran AFTER the GH#18815 fix landed and correctly verified file overlap this time. End-to-end timeline:
The current "Closed by deterministic merge pass" comment is the FIXED behaviour: cites the real implementation PR #18806, not the planning PR. No value lost — t2060 is on main, the framework bug that wrongly closed this PR is fixed in v3.8.19, and the regression test ( For #18746 aidevops.sh v3.8.20 plugin for OpenCode v1.4.3 with claude-opus-4-6 spent 30m and 69,534 tokens on this with the user in an interactive session. |
3 participants
Summary
complete_task()intask-complete-helper.shnow extracts the task block (parent + indented children) from its current section and inserts it at the top of## Doneinstead of doing in-placesedsubstitution.[x]entries from## Ready(3) and## Backlog(558) to## Done. Total[x]count unchanged at 804.tests/test-task-complete-move.shwith 10 assertions covering 7 edge cases.Changes
.agents/scripts/task-complete-helper.shsed -iin-place substitution (line 353) with a two-pass awk approach:- [ ] TASKIDline + indented children, writes transformed block to temp file, outputs file without the block.## Done(after its blank-line separator).## Doneheader is missing.## Donesection via awk.TODO.md[x]entries from## Readyand## Backlogmoved to## Done.## Readysection comment to reflect the fix..agents/scripts/tests/test-task-complete-move.sh(new)10 test assertions covering:
## In Progress→ moves to Done## Doneheader missing → errors out, file unchangedcompleted:DATEcorrectly appended to moved entryVerification
Resolves #18746