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

[fullsend-ai-coder]

Extract duplicated workflow-polling logic from admin.go (awaitRepoMaintenanceWithInterval) and enrollment.go (awaitWorkflowRun) into a shared AwaitWorkflowCompletion function in a new workflowpoll.go file under internal/layers/.

The shared utility accepts a PollConfig struct with InitialInterval, MaxInterval, and Timeout fields, enabling exponential backoff (2s initial, 15s cap, 3-minute timeout by default). Both call sites now delegate to AwaitWorkflowCompletion, eliminating the duplicated polling loop. The admin.go path adopts exponential backoff in production while preserving the test helper signature for existing tests.

An optional ProgressFunc callback replaces the direct printer coupling, allowing callers to relay status messages to their own UI layer.

New tests in workflowpoll_test.go cover: successful completion, timeout, context cancellation, progress callbacks, filtering of old runs, and the backoff interval doubling logic.

Note: make lint could not run in the sandbox due to shellcheck-py failing to download (403 Forbidden network restriction). go vet passed. The post-script runs an authoritative pre-commit check on the runner.

---

Closes #175

Post-script verification

• Branch is not main/master (agent/175-extract-workflow-poll)
• Secret scan passed (gitleaks — f96750babbed5ada406a9ae04e8068449701d9c7..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]

Review — #217 refactor(#175): extract shared workflow-polling utility

Verdict: approve

Summary

Clean refactoring that extracts duplicated workflow-polling logic from internal/cli/admin.go and internal/layers/enrollment.go into a shared AwaitWorkflowCompletion utility in internal/layers/workflowpoll.go. The change is well-scoped, addresses all four validation criteria from issue #175, and includes thorough tests.

What was verified

Issue #175 validation criteria:

1. ✅ admin.go no longer contains its own polling loop — it delegates to AwaitWorkflowCompletion via awaitRepoMaintenanceWithConfig.
2. ✅ enrollment.go:awaitWorkflowRun delegates to the same shared utility.
3. ✅ Existing tests in admin_test.go continue to compile — the legacy awaitRepoMaintenanceWithInterval wrapper is preserved with its original signature. Timeout semantics are preserved (tests use sub-millisecond intervals where the *3 multiplier is negligible).
4. ✅ admin.go production path now uses exponential backoff (2 s → 15 s cap, 3-minute timeout via DefaultPollConfig()).

Correctness:

• The AwaitWorkflowCompletion function correctly replicates the original polling logic: filters runs by dispatch time, handles context cancellation, and returns the completed run.
• The select statement correctly prioritizes context cancellation and deadline expiry over the interval timer. The standard Go timer non-determinism when multiple channels fire simultaneously is acceptable here (at worst, one extra or one fewer poll iteration near deadline).
• The annotation-harvesting logic in admin.go (post-polling) is correctly preserved outside the refactored section.
• The ProgressFunc callback cleanly decouples progress reporting from UI layer dependency.

Security: No concerns — pure internal refactoring with no new external inputs.

Architecture: Correctly places the shared utility in internal/layers/, consistent with the existing package structure. Uses the forge.Client interface as required by AGENTS.md (no direct GitHub API calls).

Low-severity notes

1. nextInterval parameter shadows max builtin (workflowpoll.go:100) — The parameter name max shadows Go's built-in max function (Go 1.21+). Non-functional, but maxInterval would be clearer.

2. Legacy wrapper timeout multiplier (admin.go:2556) — The *3 multiplier in Timeout: pollInterval * time.Duration(maxAttempts*3) overestimates the needed timeout since MaxInterval == InitialInterval means intervals don't actually grow. This is test-only code with sub-millisecond intervals, so it has no practical impact, but the mapping from old semantics (attempt count) to new semantics (wall-clock timeout) could be pollInterval * time.Duration(maxAttempts) for a more precise equivalence.

Previous run

Review

Verdict: Approve

Clean refactoring that extracts duplicated workflow-polling logic into a shared AwaitWorkflowCompletion utility, matching the scope authorized by issue #175. The implementation correctly uses the forge.Client interface (no forge abstraction violations), introduces proper exponential backoff with configurable parameters, and includes solid test coverage for the new utility. Both call sites (admin.go and enrollment.go) now delegate to the shared function. The ProgressFunc callback is a good decoupling choice.

Findings

All findings are low severity and non-blocking.

1. Undocumented *3 multiplier in legacy test adapter (internal/cli/admin.go)

The awaitRepoMaintenanceWithInterval adapter computes Timeout: pollInterval * time.Duration(maxAttempts*3). With the old call signature (5s, 36), the effective timeout becomes 540s (9 minutes) instead of the original ~180s (3 minutes). Since MaxInterval == InitialInterval (no actual backoff), this means ~108 iterations instead of 36. This only affects test paths — production uses DefaultPollConfig() (3-minute timeout) — but the *3 multiplier lacks a comment explaining its rationale. Consider either documenting the intent or using Timeout: pollInterval * time.Duration(maxAttempts) to preserve the original iteration count.

2. Exponential backoff applies unconditionally during in-progress waiting (internal/layers/workflowpoll.go)

nextInterval() is called after every loop iteration, including when the target run has been found in-progress. This means polling cadence ramps from 2s → 4s → 8s → 15s even while watching a known run. The first two polls are faster than the old 5s fixed interval, but after ~14s cumulative time the 15s cap is slower. This is the intended design per the issue, but consider resetting the interval when a matching run is found if faster completion detection is desired.

3. No validation that Timeout > InitialInterval (internal/layers/workflowpoll.go)

If a caller constructs a PollConfig with Timeout <= InitialInterval, the deadline timer fires before the first poll, returning a timeout error without ever calling the API. No current caller creates this scenario (DefaultPollConfig has 2s vs 3min), but the constraint is undocumented. A comment on PollConfig or a validation check would prevent future misuse.

4. Parameter max shadows Go builtin (internal/layers/workflowpoll.go)

In func nextInterval(current, max time.Duration), the parameter max shadows the Go 1.21+ builtin max(). This has no behavioral impact in this function but could cause confusion. Consider renaming to maxInterval for clarity.

---

Labels: PR refactors internal Go code to extract shared utility, matching enhancement label from linked issue.

[guyoron1]

/fs-review