[ISS-3] Review dispatcher dedups on FAILED jobs: a failed PR review is never retried for the same head #268

New issue

Open

opened 2026-09-28 03:52:26 +02:00 by janpeter · 0 comments

janpeter commented

2026-09-28 03:52:26 +02:00

Owner

Beheerd door Scrum4Me — wijzigingen hier worden overschreven. Bron: https://thuis.jp-visser.nl/issues/cmukl9qxa002klj7rsxds8e8o

Status: new · Severity: s3_major · Gemeld door: scrum4me-server:claude · Occurrences: 1 (laatst: 2026-09-28T01:48:50.446Z) · Aangemaakt: 2026-09-28T01:48:50.446Z

Registratie

lib/review-dispatch/enqueue.ts (enqueueReviewJob) checks for an existing job with findFirst({ where: { orchestration_key } }) regardless of status. The key is auto:pr:<owner>/<repo>#<n>@<head_sha>. A PR_REVIEW (or TASK_REVIEW) that ends FAILED blocks every later attempt for the same head: the cron (/api/cron/dispatch-reviews, every 5 min) reports it as skipped.dedup until someone pushes a new commit.

Observed (2026-09-28): agent-harness had 14 FAILED PR_REVIEW jobs in a row. 13 of them failed with post_pr_review_failed, a Forgejo 404 because the bot s4m-codex-reviewer was not a collaborator on the private repo. After the bot was added, PR #13 still did not get a new review. It only came through after the orchestration_key of the failed job was renamed by hand (…#failed-404-rerun); the new job cmukl52qz00c7vz7r5c9i6fhg was DONE/APPROVED.

Fix: dedup only on jobs that did not fail, for example where: { orchestration_key, status: { notIn: ['FAILED', 'CANCELLED'] } }, with a cap on the number of FAILED attempts per key (e.g. ≤ 3 within 24 h, counted via created_at). That way a permanent error (a 404 when access is missing) does not produce a new job every 5 minutes. Optionally a backoff based on the last finished_at. Test: FAILED job + the same candidate → enqueued; 3 FAILED → dedup/cap; QUEUED/CLAIMED/DONE → dedup (unchanged).

Related: the missing bot access was an onboarding gap (4 of 13 private repos). Consider a check in the dispatcher or health collector: private repo with a product but without the collaborator s4m-codex-reviewer → warning.

Onderzoek

Nog geen onderzoek.

Oplossing

Nog geen oplossing.

> Beheerd door Scrum4Me — wijzigingen hier worden overschreven. Bron: https://thuis.jp-visser.nl/issues/cmukl9qxa002klj7rsxds8e8o Status: new · Severity: s3_major · Gemeld door: scrum4me-server:claude · Occurrences: 1 (laatst: 2026-09-28T01:48:50.446Z) · Aangemaakt: 2026-09-28T01:48:50.446Z ## Registratie `lib/review-dispatch/enqueue.ts` (`enqueueReviewJob`) checks for an existing job with `findFirst({ where: { orchestration_key } })` **regardless of status**. The key is `auto:pr:<owner>/<repo>#<n>@<head_sha>`. A PR_REVIEW (or TASK_REVIEW) that ends FAILED blocks every later attempt for the same head: the cron (`/api/cron/dispatch-reviews`, every 5 min) reports it as `skipped.dedup` until someone pushes a new commit. **Observed (2026-09-28):** agent-harness had 14 FAILED PR_REVIEW jobs in a row. 13 of them failed with `post_pr_review_failed`, a Forgejo 404 because the bot `s4m-codex-reviewer` was not a collaborator on the private repo. After the bot was added, PR #13 still did not get a new review. It only came through after the orchestration_key of the failed job was renamed by hand (`…#failed-404-rerun`); the new job cmukl52qz00c7vz7r5c9i6fhg was DONE/APPROVED. **Fix:** dedup only on jobs that did not fail, for example `where: { orchestration_key, status: { notIn: ['FAILED', 'CANCELLED'] } }`, with a cap on the number of FAILED attempts per key (e.g. ≤ 3 within 24 h, counted via `created_at`). That way a permanent error (a 404 when access is missing) does not produce a new job every 5 minutes. Optionally a backoff based on the last `finished_at`. Test: FAILED job + the same candidate → enqueued; 3 FAILED → dedup/cap; QUEUED/CLAIMED/DONE → dedup (unchanged). **Related:** the missing bot access was an onboarding gap (4 of 13 private repos). Consider a check in the dispatcher or health collector: private repo with a product but without the collaborator `s4m-codex-reviewer` → warning. ## Onderzoek _Nog geen onderzoek._ ## Oplossing _Nog geen oplossing._

janpeter added the

severity/s3

label

2026-09-28 03:52:26 +02:00

No labels

severity/s3

severity/s4

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

janpeter/Scrum4Me#268

No description provided.

Rows
Columns