What happened
In PR #2359, internal/layers/enrollment.go introduced two different backoff strategies in the same file: dispatchRepoMaintenanceWithRetry uses additive backoff (delay += initial, producing 3s, 6s, 9s, 12s, 15s), while awaitWorkflowRun uses exponential backoff via nextInterval (current * 2, producing 2s, 4s, 8s, 15s). Both cap at 15 seconds. The PR was merged without this inconsistency being flagged by either the review agent or the human reviewer.
What could go better
This is a minor code quality issue — both approaches work and converge to the same max interval. However, a review agent could have flagged the inconsistency as a low-severity style finding, prompting the author to adopt a single backoff strategy. Confidence: medium. The inconsistency does not cause bugs, but it creates cognitive overhead for future maintainers who must reason about why two different backoff strategies were chosen in the same file. The additive backoff in dispatchRepoMaintenanceWithRetry may actually be intentional (it was pre-existing code), in which case the review comment would still be useful as a prompt for the author to explain the rationale.
Proposed change
Add guidance to the review agent's code-review skill (or the repo's AGENTS.md) to flag inconsistent retry/backoff patterns within a single file or module as a low-severity finding. Specifically, when a PR introduces a new retry loop in a file that already contains one, the review should note whether the strategies are consistent and ask the author to justify divergence or harmonize. This could be added as a bullet point in the review agent's code-review skill under a 'consistency' or 'patterns' section.
Validation criteria
On the next 5 PRs that introduce or modify retry/backoff logic in a file that already has such logic, the review agent should flag the consistency question. This can be verified by inspecting review agent comments on such PRs. The finding should be low-severity (informational, not blocking).
Generated by retro agent from #2359
What happened
In PR #2359,
internal/layers/enrollment.gointroduced two different backoff strategies in the same file:dispatchRepoMaintenanceWithRetryuses additive backoff (delay += initial, producing 3s, 6s, 9s, 12s, 15s), whileawaitWorkflowRunuses exponential backoff vianextInterval(current * 2, producing 2s, 4s, 8s, 15s). Both cap at 15 seconds. The PR was merged without this inconsistency being flagged by either the review agent or the human reviewer.What could go better
This is a minor code quality issue — both approaches work and converge to the same max interval. However, a review agent could have flagged the inconsistency as a low-severity style finding, prompting the author to adopt a single backoff strategy. Confidence: medium. The inconsistency does not cause bugs, but it creates cognitive overhead for future maintainers who must reason about why two different backoff strategies were chosen in the same file. The additive backoff in
dispatchRepoMaintenanceWithRetrymay actually be intentional (it was pre-existing code), in which case the review comment would still be useful as a prompt for the author to explain the rationale.Proposed change
Add guidance to the review agent's code-review skill (or the repo's AGENTS.md) to flag inconsistent retry/backoff patterns within a single file or module as a low-severity finding. Specifically, when a PR introduces a new retry loop in a file that already contains one, the review should note whether the strategies are consistent and ask the author to justify divergence or harmonize. This could be added as a bullet point in the review agent's code-review skill under a 'consistency' or 'patterns' section.
Validation criteria
On the next 5 PRs that introduce or modify retry/backoff logic in a file that already has such logic, the review agent should flag the consistency question. This can be verified by inspecting review agent comments on such PRs. The finding should be low-severity (informational, not blocking).
Generated by retro agent from #2359