Skip to content

Block PR promote/merge while a task is in_progress; sync after task complete (closes #988)#989

Merged
FidoCanCode merged 2 commits into
mainfrom
fix-promote-counts-in-progress
Apr 25, 2026
Merged

Block PR promote/merge while a task is in_progress; sync after task complete (closes #988)#989
FidoCanCode merged 2 commits into
mainfrom
fix-promote-counts-in-progress

Conversation

@FidoCanCode
Copy link
Copy Markdown
Owner

@FidoCanCode FidoCanCode commented Apr 25, 2026

Summary

Two-part fix for #988: fido marked PR #978 ready for review while the trailing task was still actively running, and the PR body's unchecked checkbox never got synced before reviewers saw it.

Bug 1 — in_progress slipped past the promote/merge gate

worker.py:_maybe_promote_to_ready was filtering tasks to status == PENDING only when deciding whether to block promote. A task the worker was actively running through claude (status IN_PROGRESS) didn't count, so the gate let through PRs whose final task was mid-execution. Both PENDING and IN_PROGRESS now block — in-flight work is unfinished work.

Same fix applied to the merge gate at line 2649.

Removed the dead has_real_branch_work loophole at 2733-2738 in the same pass: its distinguishing log line never fires in the 5h of history I have, and the new in_progress check covers any case where it might have been load-bearing.

Bug 2 — sync_tasks never fires after ./fido task complete

The PostToolUse sync hook in hooks.py only matches built-in TaskCreate/TaskUpdate/TodoWrite/TodoRead tools — none of which fido uses to mutate tasks. The real completion path is Bash ./fido task complete <id> (a Bash tool call), which never matches. Result: completing the trailing task leaves the PR body's checkbox unchecked until the worker loop happens to call sync_tasks again.

Tasks.complete_with_resolve now fires sync_tasks_background itself, so every completion (CLI or worker) re-syncs the PR body unconditionally. The dead PostToolUse hook is left in place — harmless, and removing it churns hook wiring beyond the scope of this fix.

Diff stats

  • 4 files changed, +84 / −107
  • 5 _has_substantive_branch_commits tests deleted along with the helper
  • 2 loophole tests replaced with 3 tests for the new in_progress semantics
  • 2 tests added for sync_tasks_background firing on every completion

Test plan

  • ./fido pytest tests/test_worker.py tests/test_tasks.py -q — 913/913 pass
  • ./fido ci — format, lint, pyright, generated-typecheck, tests all green
  • Live verification: pick a small repro PR, confirm fido waits for in_progress to clear before pr_ready, and the PR body's checkboxes match tasks.json at promote time

Closes #988.

🤖 Generated with Claude Code

…omplete (closes #988)

Two-part fix for the bug where fido marked PR #978 ready_for_review
while the trailing task was still actively executing — the PR body
showed an unchecked checkbox at the moment reviewers saw it.

Bug 1 — in_progress treated as not-pending in promote/merge gate.
worker.py:_maybe_promote_to_ready filtered tasks to status == PENDING
when deciding whether to block promote, so a task the worker was
currently running through claude (status == IN_PROGRESS) didn't count.
Now both PENDING and IN_PROGRESS block promote and merge — in-flight
work is unfinished work. Removed the dead "branch has real commits
overrides pending" loophole at the same time (its log line never fired
in the visible 5h history; the new in_progress check covers any case
where it might have been load-bearing).

Bug 2 — sync_tasks never fires after ./fido task complete.
The PostToolUse sync hook in hooks.py only matches built-in TaskCreate/
TaskUpdate/TodoWrite/TodoRead — none of which are how fido tasks are
mutated. The actual completion path is `Bash ./fido task complete <id>`
(a Bash tool call), which never triggers the hook. Result: completing
the trailing task leaves the PR body's checkbox unchecked until the
worker loop happens to call sync_tasks again. Now Tasks.complete_with_resolve
itself fires sync_tasks_background, so every completion (CLI or worker)
re-syncs the PR body unconditionally. The dead PostToolUse hook is
left in place — harmless, and removing it churns hooks.py wiring
beyond the scope of this fix.

Tests:
- test_draft_pending_tasks_with_real_commits_can_promote /
  test_draft_pending_tasks_with_real_commits_requests_review
  replaced with test_draft_pending_tasks_block_promote_even_with_real_commits,
  test_draft_in_progress_task_blocks_promote, and
  test_merge_blocked_while_task_in_progress.
- 5 _has_substantive_branch_commits tests deleted along with the helper.
- test_triggers_background_sync and test_triggers_background_sync_even_for_unknown_id
  added to TestTasksCompleteWithResolve.

CI green, 913/913 tests pass.
@FidoCanCode FidoCanCode marked this pull request as ready for review April 25, 2026 22:44
@FidoCanCode FidoCanCode requested a review from rhencke April 25, 2026 22:44
@FidoCanCode FidoCanCode merged commit 805a53b into main Apr 25, 2026
1 check passed
@FidoCanCode FidoCanCode deleted the fix-promote-counts-in-progress branch April 25, 2026 22:50
FidoCanCode added a commit that referenced this pull request Apr 26, 2026
…closes #999)

confusio worker stuck looping every 60s on PR #288 with no task progress
for ~10 minutes. Trace showed `execute_task` returning 0 every cycle
while the active task remained `in_progress`, then the promote gate
(added in #989) correctly seeing the in_progress task and refusing to
promote past it. Deadlock: worker can neither pick up the in_progress
task nor advance to merge.

Root cause: `_pick_next_task` filtered tasks to `status == PENDING` only,
so any task left in_progress by an iteration that ended abnormally
(subprocess crash, fido self-restart, kill) became invisible to the
picker. Before #989 this was masked because the promote gate let the
in_progress task slip through; #989 closed that hole and exposed the
picker's blind spot.

Fix: include IN_PROGRESS in the eligible candidates and prioritise it
over PENDING — finishing what we already started keeps tasks.json
honest before we reach for new work. ASK/DEFER prefixes still suppress
IN_PROGRESS (those signals mean a task is awaiting a human, not a
signal to resume).

Tests: 4 new TestPickNextTask cases covering IN_PROGRESS over PENDING,
IN_PROGRESS over CI-PENDING, lone IN_PROGRESS returned, and
ASK-prefixed IN_PROGRESS still suppressed. Existing 15 cases still
pass.
FidoCanCode added a commit that referenced this pull request Apr 26, 2026
…closes #999) (#1000)

## Summary

Closes [#999](#999) — confusio
worker deadlock where an `IN_PROGRESS` task was invisible to
`_pick_next_task` and the #989 promote gate (correctly) refused to
promote past it.

## Diagnosis

Trace from `~/log/fido.log`, ~23:44Z–23:55Z (10 min stuck loop on PR
#288):

```
rescope_before_pick: fewer than 2 pending tasks — skipping
checking: ci → green
checking: tasks
PR #288: 2 tasks still pending or in-progress — not promoting yet
   ← idle wait 60s → identical loop
```

`execute_task` returned 0 every cycle because `_pick_next_task` filtered
to `status == PENDING` only. The active task was `IN_PROGRESS` (left
there by an earlier iteration that ended without claude running `./fido
task complete`). Combined with #989's promote gate, the worker had no
way out.

Before #989 this was masked — the promote gate let the in_progress task
slip through and the PR got marked ready while work was still in flight
(the original #988 symptom). #989 closed that hole; this PR fixes the
picker so the cycle completes.

## Fix

`_pick_next_task` now treats `IN_PROGRESS` as the highest-priority
candidate (resume what we already started), then falls back to CI tasks,
then list-order PENDING. ASK/DEFER prefixes still suppress IN_PROGRESS —
those signals mean a task is waiting for a human, not a signal to
resume.

## Test plan

- [x] 4 new `TestPickNextTask` cases: IN_PROGRESS over PENDING,
IN_PROGRESS over CI, lone IN_PROGRESS returned, ASK-prefixed IN_PROGRESS
still suppressed
- [x] 15 existing `TestPickNextTask` cases still pass
- [x] `./fido ci` green
- [ ] Live verification: bring fido up, confirm confusio's stuck `task
1/2 — Extend validation scripts` resumes within one worker cycle (≤60s)
instead of looping indefinitely

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Fido Can Code <190991155+FidoCanCode@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: fido marks PR ready for review with task still pending; final sync_tasks never runs

2 participants