Skip to content

fix: fail fast on structural worktree-setup failures instead of retry-looping - #486

Merged
getappz merged 3 commits into
masterfrom
task/467-fail-fast-on-structural-worktree-setup-f
Aug 14, 2026
Merged

fix: fail fast on structural worktree-setup failures instead of retry-looping#486
getappz merged 3 commits into
masterfrom
task/467-fail-fast-on-structural-worktree-setup-f

Conversation

@getappz

@getappz getappz commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Commit landed cleanly with trailers auto-added by the repo's hook. Work is committed and verified.

Summary

Item #465/#466's incident: execute_work's structural setup-failure paths (no worktree created, or a claimed worktree that can't be chdir'd into — both caused by broken git state, like a stale prunable worktree registration) were retried through the normal max_retries backoff budget even though the underlying cause can't change between attempts. This delayed reaching terminal state=failed, where the existing handle_terminal_job_failure hook would swap the item off dispatched back to discoverable.

Change: added a fatal: bool flag threaded through JobFailure (agentflare-jobs) → WorkOutcome (cli/work.rs) → Queue::fail, which now skips the retry budget entirely and marks the job terminal immediately when fatal is set, regardless of retries remaining.

  • crates/agentflare-jobs/src/executor.rs: JobFailure gains fatal.
  • crates/agentflare-jobs/src/queue.rs: Queue::fail takes a fatal param; retried = !fatal && retries < max_retries. Added test fail_fatal_marks_failed_immediately_even_with_retries_remaining.
  • crates/agentflare-jobs/src/worker.rs: threads fatal through record_fail/queue.fail call sites.
  • src/cli/work.rs: WorkOutcome gains fatal; the "no worktree created" and "failed to chdir into worktree" branches now set fatal: true; the agent-run failure branch stays fatal: false (unaffected, keeps existing retry/cooldown behavior). Extracted the WorkOutcome → JobFailure mapping into job_failure_for so it's unit-testable without a real claim/worktree cycle; added two tests covering both the fatal and non-fatal mapping.

Did not touch src/supervisor.rsdispatch_item's ready-for-workdispatched label-flip timing is unchanged, and orphan_reconcile.rs's terminal-failure-hook tests pass unmodified.

Verified: cargo fmt --check, cargo clippy --workspace --all-features [--tests] -A unsafe_code -A clippy::pedantic -D warnings, cargo test -p agentflare-jobs (31 passed), and the cli::work/dashboard::orphan_reconcile test modules in the main binary (51 passed) — all green.

Summary by CodeRabbit

  • Bug Fixes

    • Fatal job failures now stop immediately without retrying.
    • Retryable failures continue following the existing retry behavior.
    • Worktree setup failures are correctly treated as terminal, while agent execution failures remain retryable.
    • Failure status and retry timing are consistently recorded across job execution paths.
  • Tests

    • Added coverage confirming fatal failures remain terminal and do not increase retry counts.

execute_work's "claim succeeded but no worktree was created" and
"failed to chdir into <worktree>" paths were structural git-state
failures (e.g. a stale prunable worktree registration, confirmed live
for items #465/#466) that retry identically every time, but went
through the normal max_retries backoff budget before the terminal-
failure recovery hook could swap the item off `dispatched`.

Add a `fatal` flag to JobFailure/WorkOutcome that Queue::fail uses to
skip straight to terminal `state=failed` regardless of retries
remaining, and set it on those two setup-failure branches. Agent-run
failures are unaffected and keep the existing retry/cooldown behavior.
dispatch_item's ready-for-work/dispatched label-flip timing is
untouched.

Agentflare-Agent: claude-code
Agentflare-Branch: task/467-fail-fast-on-structural-worktree-setup-f
Agentflare-Item: 467
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 09247ce5-6800-4817-937f-1dd9317f150c

📥 Commits

Reviewing files that changed from the base of the PR and between 815ce25 and b419d6e.

📒 Files selected for processing (5)
  • crates/agentflare-jobs/src/executor.rs
  • crates/agentflare-jobs/src/queue.rs
  • crates/agentflare-jobs/src/worker.rs
  • crates/agentflare-jobs/tests/queue_test.rs
  • src/cli/work.rs

📝 Walkthrough

Walkthrough

The change adds fatal status to job failures, maps structural worktree failures to fatal outcomes, propagates failure status through workers, and prevents fatal jobs from entering retry handling.

Changes

Fatal failure handling

Layer / File(s) Summary
Failure contract and queue behavior
crates/agentflare-jobs/src/executor.rs, crates/agentflare-jobs/src/queue.rs, crates/agentflare-jobs/tests/queue_test.rs
JobFailure now exposes fatal. Queue::fail accepts the flag and skips retries for fatal failures. Queue tests cover terminal fatal failures and existing nonfatal retry behavior.
Work outcome fatal mapping
src/cli/work.rs
WorkOutcome records fatal status and maps it into JobFailure. Worktree setup failures are fatal. Agent-run failures remain nonfatal.
Worker failure propagation
crates/agentflare-jobs/src/worker.rs
Worker failure paths pass explicit nonfatal status or forward executor-provided fatal status to Queue::fail.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to b419d

The change makes structural worktree-setup failures terminal instead of retrying them, while preserving retries for agent-run failures. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the change, affected components, failure behavior, tests, and verification results, although it does not use every template heading.
Title check ✅ Passed The title clearly and concisely describes the primary change: structural worktree setup failures now fail immediately instead of retrying.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task/467-fail-fast-on-structural-worktree-setup-f

Comment @coderabbitai help to get the list of available commands.

@getappz getappz changed the title Fail fast on structural worktree-setup failures instead of retry-looping fix: fail fast on structural worktree-setup failures instead of retry-looping Aug 13, 2026
@getappz
getappz enabled auto-merge (squash) August 13, 2026 13:53
@getappz
getappz merged commit 66ea4aa into master Aug 14, 2026
17 checks passed
@getappz
getappz deleted the task/467-fail-fast-on-structural-worktree-setup-f branch August 14, 2026 08:42
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.

1 participant