Skip to content

RIG-405 feat: implementation#240

Merged
ArLeyar merged 2 commits intomainfrom
feat/RIG-405-pipeline-engineer-stage
Apr 9, 2026
Merged

RIG-405 feat: implementation#240
ArLeyar merged 2 commits intomainfrom
feat/RIG-405-pipeline-engineer-stage

Conversation

@ArLeyar
Copy link
Copy Markdown
Contributor

@ArLeyar ArLeyar commented Apr 9, 2026

Summary

Pipeline engineer task 20260409-002.

Linear: RIG-405

- Add count_fruitless_tasks_for_issue_stage (failed OR completed-without-effects)
- should_skip_due_to_failures now counts fruitless tasks instead of only failed
  tasks, catching Qwen empty-output infinite spawn loops
- Generalize circuit breaker from reviewer-only to all stages: non-reviewer
  stages with max_stage_attempts use attempt_limit * 2 as total-task cap
- Add tests: fruitless_count_* (5 unit tests), poll_circuit_breaker_blocks_
  excessive_engineer_spawns, poll_fruitless_cap_blocks_empty_completions
@ArLeyar
Copy link
Copy Markdown
Contributor Author

ArLeyar commented Apr 9, 2026

Code review

Found 1 issue:

  1. Cooldown timer still only watches failed tasks — fruitless completions bypass it

should_skip_due_to_failures() has two guards:

  • Part 1 (cap): correctly updated to use count_fruitless_tasks_for_issue_stage() — counts failed + completed-with-no-effects ✓
  • Part 2 (cooldown): still calls last_failed_task_time_for_issue_stage() which queries WHERE status = 'failed' only

A Qwen agent completing with empty output (no effects) will accumulate toward the fruitless cap but produce zero cooldown delay — rapid-fire spawning continues until the cap triggers. With max_stage_attempts = 3, up to 6 fruitless completions can fire in rapid succession before the circuit breaker engages.

// 2. Cooldown: if the most recent failed task finished within FAILURE_COOLDOWN_SECS,
// skip this poll cycle to avoid rapid-fire retries.
if let Some(last_failed_time) =
db.last_failed_task_time_for_issue_stage(identifier, stage_name)?
{
if let Ok(ts) =
chrono::NaiveDateTime::parse_from_str(&last_failed_time, "%Y-%m-%dT%H:%M:%S")
{
let now = chrono::Local::now().naive_local();
let elapsed = now.signed_duration_since(ts).num_seconds();
if elapsed < FAILURE_COOLDOWN_SECS {
eprintln!(
" ~ {identifier} stage={stage_name}: cooldown active — \
last failure {elapsed}s ago (limit: {FAILURE_COOLDOWN_SECS}s)"

The last_failed_task_time_for_issue_stage() SQL (db/pipeline.rs:386-390) hardcodes AND status = 'failed', so fruitless completions are invisible to it:

"SELECT finished_at FROM tasks
WHERE issue_identifier = ?1
AND pipeline_stage = ?2
AND status = 'failed'
AND finished_at IS NOT NULL
ORDER BY finished_at DESC
LIMIT 1",
params![issue_id, stage],
|row| row.get(0),
)
.ok();
Ok(result)

Fix: add last_fruitless_task_time_for_issue_stage() that mirrors the cap query (status = 'failed' OR (status = 'completed' AND no effects)) and use it in the cooldown check.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

The cooldown timer in should_skip_due_to_failures() only queried failed
tasks, while the cap was already updated to count fruitless completions.
This allowed rapid-fire spawning of fruitless tasks (e.g. Qwen empty
output) with zero cooldown delay until the cap triggered.

Add last_fruitless_task_time_for_issue_stage() query that mirrors the
fruitless count predicate (failed OR completed-without-effects) and use
it in the cooldown guard so both guards are symmetric.
@ArLeyar ArLeyar merged commit be94449 into main Apr 9, 2026
2 checks passed
@ArLeyar ArLeyar deleted the feat/RIG-405-pipeline-engineer-stage branch April 9, 2026 13:53
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