refactor(#257): extract shared workflow-polling utility

[fullsend-ai-coder]

Extract duplicated workflow-polling loops from admin.go and enrollment.go into a shared AwaitWorkflowCompletion function in internal/layers/workflowpoll.go. The new utility uses exponential backoff (doubling from InitialInterval up to MaxInterval) with a configurable timeout, replacing the fixed-interval polling that both callers previously implemented inline.

admin.go: awaitRepoMaintenance now delegates through awaitRepoMaintenanceWithConfig using DefaultPollConfig. awaitRepoMaintenanceWithInterval is kept as a compatibility shim for existing tests.

enrollment.go: awaitWorkflowRun now delegates to the shared AwaitWorkflowCompletion with DefaultPollConfig.

Note: pre-commit could not run in sandbox (shellcheck-py network restriction). Post-script runs authoritative pre-commit on the runner.

---

Closes #257

Post-script verification

• Branch is not main/master (agent/257-extract-workflow-poll)
• Secret scan passed (gitleaks — c776c248ac9a91f10998ca7107e974a6c9d76514..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[github-actions]

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

[fullsend-ai-review]

🤖 Finished Review · ❌ Failure · Started 12:59 PM UTC · Completed 12:59 PM UTC
Commit: c776c24 · View workflow run →

[guyoron1]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:57 PM UTC · Completed 4:10 PM UTC
Commit: d8e3df7 · View workflow run →

[fullsend-ai-review]

Review

Verdict: comment — medium-severity findings worth discussing, none blocking.

Clean refactoring that correctly consolidates the duplicated workflow-polling loops from admin.go and enrollment.go into AwaitWorkflowCompletion. The new utility is well-placed in internal/layers/, uses the forge.Client interface properly, and adds exponential backoff as requested by #257. No security concerns. Scope matches the authorized issue exactly.

Findings

1. Shim timeout math triples the effective timeout

Severity: medium · File: internal/cli/admin.go · Category: behavioral-change

The compatibility shim computes Timeout: pollInterval * time.Duration(maxAttempts*3). The original loop's effective wall-clock timeout was pollInterval * maxAttempts (it sleeps pollInterval per attempt for exactly maxAttempts iterations). The *3 multiplier triples this.

For the production path this doesn't matter — awaitRepoMaintenance now calls awaitRepoMaintenanceWithConfig with DefaultPollConfig() directly, bypassing the shim. The shim is only used by the 4 existing tests in admin_test.go, where the 1ms intervals make the difference negligible. But the shim's stated purpose is backward compatibility, and its math doesn't preserve the original timeout semantics.

Suggestion: Use Timeout: pollInterval * time.Duration(maxAttempts) (drop the *3), or better yet, update the 4 tests to call awaitRepoMaintenanceWithConfig with an explicit PollConfig and delete the shim entirely.

2. Timer/select race can overshoot the deadline

Severity: medium · File: internal/layers/workflowpoll.go · Category: correctness

The select statement races deadline.C, ctx.Done(), and time.After(interval). When the deadline and interval timers fire simultaneously, Go picks a case at random. This means the loop can execute one extra poll after the deadline has expired — a minor deviation from the stated timeout contract.

Suggestion: Replace the manual time.NewTimer with ctx, cancel := context.WithTimeout(ctx, cfg.Timeout) and defer cancel(). This composes cleanly with the existing ctx.Done() case, eliminates the race, and removes 3 lines of timer management.

3. Test gaps for key behavioral branches

Severity: medium · File: internal/layers/workflowpoll_test.go · Category: test-adequacy

The new test file covers success, timeout, cancellation, progress callback, and the nextInterval helper — good baseline. But several important branches lack coverage:

• In-progress run: No test where a run exists with Status != "completed". This exercises the break (keep-waiting) path and the progress callback on the in-progress branch — distinct from the error-path callback tested by TestAwaitWorkflowCompletion_ProgressCallback, which actually returns on the first poll because the run is already completed.
• No matching run by dispatch time: No test where runs are returned but all have CreatedAt before dispatchTime.
• Error then success: No test where ListWorkflowRuns fails initially then succeeds. A stateful fake would verify the retry-after-error path.

4. max parameter shadows Go builtin

Severity: low · File: internal/layers/workflowpoll.go (line ~100) · Category: style

In nextInterval(current, max time.Duration), the parameter max shadows Go's builtin max function (Go 1.21+, this repo uses Go 1.26). While it compiles cleanly and causes no issues here, it's a linting smell.

Suggestion: Rename to maxInterval to match the PollConfig.MaxInterval field name.

5. Backoff applies to successful-but-in-progress polls

Severity: low · File: internal/layers/workflowpoll.go · Category: behavioral-change

The nextInterval call at the bottom of the loop doubles the interval even when ListWorkflowRuns succeeds and finds the run — just not completed yet. The old code used a fixed 5s interval regardless. With DefaultPollConfig (5s→15s cap), the polling cadence for a long-running workflow goes: 5s, 10s, 15s, 15s... vs the old constant 5s. This reduces API pressure (good) but slows user feedback on workflow progress. This may be intentional — just noting the behavioral delta.

Summary

Severity	Count
Critical	0
High	0
Medium	3
Low	2

All findings are non-blocking. The refactoring achieves its goal, the production path is correct, and the medium findings are edge cases in the compatibility shim and test infrastructure. Nice deduplication work.

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

[low] style

Parameter name 'max' in nextInterval(current, max time.Duration) shadows the builtin max function introduced in Go 1.21 (this repo uses Go 1.26).

Suggested fix: Rename to maxInterval to match the PollConfig.MaxInterval field name.

[guyoron1]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ❌ Failure · Started 5:38 PM UTC · Completed 5:51 PM UTC
Commit: d8e3df7 · View workflow run →