What happened
PR #2359 added awaitWorkflowRun to internal/layers/enrollment.go with exponential backoff and a bounded 3-minute timeout. However, a near-identical function awaitRepoMaintenanceWithInterval already exists in internal/cli/admin.go (line 2789). Both functions poll ListWorkflowRuns, filter runs by dispatch timestamp, wait for status completed, handle context cancellation, and report progress. The admin.go version uses a fixed 5-second poll interval with 36 max attempts (also ~3 minutes), while the enrollment.go version uses exponential backoff (2s doubling to 15s cap). The duplication was not addressed in the PR.
What could go better
The code agent likely did not explore admin.go before implementing the new polling loop, or it did not recognize the shared pattern. The review agent (or human reviewer) also did not flag the duplication. Confidence level: high — the code pattern overlap is clear and mechanical. The enrollment version's exponential backoff is the better design, and the admin.go version would benefit from adopting it. The risk of not consolidating is that future fixes to polling logic (e.g., better error handling, jitter) must be applied in two places.
Proposed change
Extract a shared awaitWorkflowCompletion utility (or similar) into a common location — either a new file in internal/layers/ or a shared utility package. The function should accept: context, forge client, org, workflow name, dispatch time, and a config struct for timeout/backoff parameters. Both admin.go:awaitRepoMaintenanceWithInterval and enrollment.go:awaitWorkflowRun should delegate to it. The admin.go version should adopt exponential backoff at the same time. The annotation-harvesting logic currently in admin.go's version can remain as a post-call step.
Validation criteria
After refactoring: (1) admin.go no longer contains its own workflow-polling loop — it calls the shared utility. (2) enrollment.go:awaitWorkflowRun delegates to the same shared utility. (3) All existing tests in admin_test.go and enrollment_test.go pass without changes to assertions. (4) admin.go now uses exponential backoff instead of fixed 5s intervals.
Generated by retro agent from #2359
What happened
PR #2359 added
awaitWorkflowRuntointernal/layers/enrollment.gowith exponential backoff and a bounded 3-minute timeout. However, a near-identical functionawaitRepoMaintenanceWithIntervalalready exists ininternal/cli/admin.go(line 2789). Both functions pollListWorkflowRuns, filter runs by dispatch timestamp, wait for statuscompleted, handle context cancellation, and report progress. The admin.go version uses a fixed 5-second poll interval with 36 max attempts (also ~3 minutes), while the enrollment.go version uses exponential backoff (2s doubling to 15s cap). The duplication was not addressed in the PR.What could go better
The code agent likely did not explore
admin.gobefore implementing the new polling loop, or it did not recognize the shared pattern. The review agent (or human reviewer) also did not flag the duplication. Confidence level: high — the code pattern overlap is clear and mechanical. The enrollment version's exponential backoff is the better design, and the admin.go version would benefit from adopting it. The risk of not consolidating is that future fixes to polling logic (e.g., better error handling, jitter) must be applied in two places.Proposed change
Extract a shared
awaitWorkflowCompletionutility (or similar) into a common location — either a new file ininternal/layers/or a shared utility package. The function should accept: context, forge client, org, workflow name, dispatch time, and a config struct for timeout/backoff parameters. Bothadmin.go:awaitRepoMaintenanceWithIntervalandenrollment.go:awaitWorkflowRunshould delegate to it. The admin.go version should adopt exponential backoff at the same time. The annotation-harvesting logic currently in admin.go's version can remain as a post-call step.Validation criteria
After refactoring: (1)
admin.gono longer contains its own workflow-polling loop — it calls the shared utility. (2)enrollment.go:awaitWorkflowRundelegates to the same shared utility. (3) All existing tests inadmin_test.goandenrollment_test.gopass without changes to assertions. (4)admin.gonow uses exponential backoff instead of fixed 5s intervals.Generated by retro agent from #2359