refactor(test): extract pollUntil/pollForLength to test/lib/polling (F3) by tesdal · Pull Request #6 · tesdal/opencode

tesdal · 2026-04-23T22:35:58Z

Summary

Audit finding F3 from 2026-04-22 diamond audit (codex-5.3): run-events.test.ts had two bespoke helpers (waitForQuestionCount, waitForPermissionCount) plus an inline 100-iteration retry loop for replies.length === 1. Task 3 extracts a generic pollUntil + pollForLength into test/lib/polling.ts so the convention is consistent and future test files reuse the same helper.

Changes

New: packages/opencode/test/lib/polling.ts — exports pollUntil<A>(probe, opts?) and pollForLength<A>(probe, count, opts?). Defaults: 100 iterations × 10ms interval. Both delegate to one loop; pollForLength is a thin specialization.
packages/opencode/test/cli/run-events.test.ts: local waitForQuestionCount / waitForPermissionCount definitions removed; all call sites replaced with pollForLength(() => question.list(), N) (and permission). Inline replies.length === 1 retry loop consolidated via pollUntil.

Verification

bun typecheck — clean
bun test test/cli/run-events.test.ts — 12/12 pass (plan said ~14; file has 12 it.live tests; no tests dropped)
bun test test/session/subagent-hang-regression.test.ts — 2/2 pass (sanity)

Diamond review status

Spec compliance (codex-5.3): ✅ 10/10 APPROVE.
Code quality (Opus): APPROVE WITH NITS. Four minor nits — one actionable (redundant Omit<PollOptions, \"label\"> & { label?: string } → PollOptions) was fixed and amended into the commit. Three judgement-call nits (A | undefined vs Option<A> probe contract, Array.from({ length }) allocation, Effect.sync vs Effect.succeed) left as-is — match existing test-file conventions.

Context

Part of 14-task audit remediation plan. Base phase-ab-base is 65677213c (F2 merge on local/integration-v2). Draft PR opened for Copilot review only — merge target is local integration branch.

Copilot

Pull request overview

Refactors test polling/retry logic by extracting shared polling helpers into a reusable test utility, and updates run-events CLI tests to use the new helpers for consistent retry behavior.

Changes:

Added pollUntil and pollForLength helpers in test/lib/polling.ts with standardized polling defaults.
Replaced bespoke waitForQuestionCount / waitForPermissionCount loops in run-events.test.ts with pollForLength.
Consolidated an inline “wait until replies length is 1” loop via pollUntil.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
packages/opencode/test/lib/polling.ts	Introduces shared polling helpers used to standardize test synchronization loops.
packages/opencode/test/cli/run-events.test.ts	Migrates local polling helpers and an inline retry loop to the new shared polling utilities.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-23T22:39:17Z

+  const interval = `${opts.intervalMillis ?? 10} millis` as const
+  const label = opts.label ?? "pollUntil predicate"
+  return Effect.gen(function* () {
+    for (const _ of Array.from({ length: iterations })) {


Array.from({ length: iterations }) allocates an array on every call just to drive the loop. Since this helper is intended to be reused across the suite, prefer a simple counted loop (for (let i = 0; i < iterations; i++)) to avoid the extra allocation and make it obvious that iterations must be a non-negative integer.

Suggested change

for (const _ of Array.from({ length: iterations })) {

for (let i = 0; i < iterations; i++) {

Copilot · 2026-04-23T22:39:17Z

+  const iterations = opts.iterations ?? 100
+  const interval = `${opts.intervalMillis ?? 10} millis` as const
+  const label = opts.label ?? "pollUntil predicate"
+  return Effect.gen(function* () {


iterations is used as an array length (Array.from({ length: iterations })). If a caller passes a negative or non-integer value, this will throw a RangeError rather than failing through the Effect error channel. Consider validating/sanitizing opts.iterations (e.g. clamp to >= 0 and Math.floor) and failing with a clear error when invalid.

Suggested change

return Effect.gen(function* () {

return Effect.gen(function* () {

if (!Number.isInteger(iterations) || iterations < 0) {

return yield* Effect.fail(

new Error(

`invalid iterations for ${label}: expected a non-negative integer, got ${String(iterations)}`,

),

)

}

Copilot · 2026-04-23T22:39:17Z

+export function pollUntil<A>(
+  probe: () => Effect.Effect<A | undefined>,
+  opts: PollOptions = {},
+): Effect.Effect<A, Error> {


pollUntil/pollForLength are typed as Effect.Effect<…> with omitted error/environment parameters, which effectively restricts probes to E = never and R = never. That makes the helper less reusable than intended and forces callers to wrap/cast if a probe can fail or requires an environment. Consider generalizing the signatures to accept probe: () => Effect.Effect<A | undefined, E, R> and return Effect.Effect<A, E | Error, R> (and similarly for pollForLength).

Copilot · 2026-04-23T22:39:17Z

+export function pollUntil<A>(
+  probe: () => Effect.Effect<A | undefined>,
+  opts: PollOptions = {},
+): Effect.Effect<A, Error> {
+  const iterations = opts.iterations ?? 100
+  const interval = `${opts.intervalMillis ?? 10} millis` as const
+  const label = opts.label ?? "pollUntil predicate"
+  return Effect.gen(function* () {
+    for (const _ of Array.from({ length: iterations })) {
+      const value = yield* probe()
+      if (value !== undefined) return value


pollUntil uses undefined as the sentinel for “keep polling”, but the generic A is unconstrained. If a caller ever legitimately expects A to include undefined, the function can never succeed even when the probe has reached a valid state. Consider constraining A to exclude undefined (e.g. A extends {} | null) or changing the contract to return an explicit Option<A> (or { done: boolean; value?: A }) from the probe.

tesdal · 2026-04-23T23:20:05Z

All 4 design comments addressed in force-pushed commit 2cefabe:

L21 counted loop — replaced Array.from({ length }) with for (let i = 0; i < iterations; i++).
L20 iterations validation — Number.isInteger(iterations) && iterations >= 0 check, fails through the Effect error channel with a clear message.
L16 generic E/R — signatures now <A, E, R>(probe: () => Effect.Effect<Option.Option<A>, E, R>) => Effect.Effect<A, E | Error, R>. Probes can fail and require environment.
L23 sentinel — contract now Option<A> instead of A | undefined. Call sites updated.

Verification: bun typecheck clean, 12/12 tests pass. Requesting re-review.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-23T23:22:40Z

+  return pollUntil(
+    () =>
+      Effect.gen(function* () {
+        const list = yield* probe()
+        return list.length === count ? Option.some(list) : Option.none()
+      }),
+    { ...opts, label: opts.label ?? `list length=${count}` },


pollForLength accepts count: number but doesn't validate it. If count is negative or non-integer, the condition can never be satisfied and the helper will always time out, which makes failures harder to diagnose. Consider failing fast with a clear error when count is not a non-negative integer (and include it in the timeout/validation message).

Suggested change

return pollUntil(

() =>

Effect.gen(function* () {

const list = yield* probe()

return list.length === count ? Option.some(list) : Option.none()

}),

{ ...opts, label: opts.label ?? `list length=${count}` },

const label = opts.label ?? `list length=${count}`

if (!Number.isInteger(count) || count < 0) {

return Effect.fail(

new Error(`invalid count for ${label}: expected a non-negative integer, got ${String(count)}`),

)

}

return pollUntil(

() =>

Effect.gen(function* () {

const list = yield* probe()

return list.length === count ? Option.some(list) : Option.none()

}),

{ ...opts, label },

Removes bespoke waitForQuestionCount / waitForPermissionCount helpers from run-events.test.ts. Future tests that need to poll for a service list length can use the shared pollForLength helper instead of writing yet another local 100-iteration loop. Addresses audit finding F3 (codex-5.3 diamond review, 2026-04-22).

tesdal · 2026-04-24T07:02:21Z

Second iteration addresses the new comment: pollForLength now validates count identically to pollUntil's iterations check. Clean typecheck, 12/12 pass.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tesdal · 2026-04-24T07:11:06Z

Three Copilot review rounds. 4 initial design comments + 1 follow-up all addressed. Final review: no new comments. Diamond status: codex-5.3 spec 10/10 APPROVE, Opus quality APPROVE WITH NITS (all fixed), Copilot clean. Merging --no-ff into local/integration-v2. PR closed as review-only.

Diamond: codex-5.3 spec 10/10 APPROVE, Opus quality APPROVE WITH NITS (fixed), Copilot clean after 3 review rounds (5 comments across rounds, all addressed). Closes audit finding F3. Copilot review PR #6 (closed as review-only).

tesdal requested a review from Copilot April 23, 2026 22:35

Copilot started reviewing on behalf of tesdal April 23, 2026 22:36 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

tesdal force-pushed the audit-f3-polling-helpers branch from dff2221 to 2cefabe Compare April 23, 2026 23:19

tesdal requested a review from Copilot April 23, 2026 23:20

Copilot started reviewing on behalf of tesdal April 23, 2026 23:20 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

tesdal force-pushed the audit-f3-polling-helpers branch from 2cefabe to 8d546e5 Compare April 24, 2026 07:02

tesdal requested a review from Copilot April 24, 2026 07:03

Copilot started reviewing on behalf of tesdal April 24, 2026 07:04 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

tesdal closed this Apr 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(test): extract pollUntil/pollForLength to test/lib/polling (F3)#6

refactor(test): extract pollUntil/pollForLength to test/lib/polling (F3)#6
tesdal wants to merge 1 commit intophase-ab-basefrom
audit-f3-polling-helpers

tesdal commented Apr 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

tesdal commented Apr 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

tesdal commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

tesdal commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	for (const _ of Array.from({ length: iterations })) {
	for (let i = 0; i < iterations; i++) {

-  return Effect.gen(function* () {
+  return Effect.gen(function* () {
+    if (!Number.isInteger(iterations) || iterations < 0) {
+      return yield* Effect.fail(
+        new Error(
+          `invalid iterations for ${label}: expected a non-negative integer, got ${String(iterations)}`,
+        ),
+      )
+    }

Conversation

tesdal commented Apr 23, 2026

Summary

Changes

Verification

Diamond review status

Context

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

tesdal commented Apr 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

tesdal commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

tesdal commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants