Reject zero execution attempts in ExecutionPlan steps

[fubhy]

Summary

A step accepts attempts: 0 and then executes under default behavior instead of rejecting the impossible count.

Important

This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.

Zero attempts bypasses positive-count validation

Module: ExecutionPlan
Audit ID: core-a-f-execution-plan-zero-attempts
Severity / confidence: medium / high

What happens

A step accepts attempts: 0 and then executes under default behavior instead of rejecting the impossible count.

Why it happens

Validation is guarded by if (options.attempts && ...), so zero skips the less-than-one check. Downstream truthiness handling also treats zero as absent.

Expected behavior

attempts is a count of executions for a step, and the constructor explicitly rejects values below one with "must be greater than 0".

Relevant implementation

These links and excerpts are pinned to audit base c9b56ab507f224426ee8388dc450da447ec4715f.

• packages/effect/src/ExecutionPlan.ts:181-187
• packages/effect/src/internal/executionPlan.ts:94-117

View problematic code at packages/effect/src/ExecutionPlan.ts:181-187

  makeProto(steps.map((options, i) => {
    if (options.attempts && options.attempts < 1) {
      throw new Error(`ExecutionPlan.make: step[${i}].attempts must be greater than 0`)
    }
    return {
      schedule: options.schedule,
      attempts: options.attempts,

View exact lines on GitHub

View problematic code at packages/effect/src/internal/executionPlan.ts:94-117

/** @internal */
export const scheduleFromStep = <Provides, In, PlanE, PlanR>(
  step: Api.ExecutionPlan<{
    provides: Provides
    input: In
    error: PlanE
    requirements: PlanR
  }>["steps"][number],
  first: boolean
) => {
  if (!first) {
    return internalSchedule.buildFromOptions({
      schedule: step.schedule ? step.schedule : step.attempts ? undefined : scheduleOnce,
      times: step.attempts,
      while: step.while
    })
  } else if (step.attempts === 1 || !(step.schedule || step.attempts)) {
    return undefined
  }
  return internalSchedule.buildFromOptions({
    schedule: step.schedule,
    while: step.while,
    times: step.attempts ? step.attempts - 1 : undefined
  })

View exact lines on GitHub

Reproduction

pnpm test --run packages/effect/test/ExecutionPlanAttemptValidation.test.ts

Observed failure: A direct Node 24 source probe printed accepted for attempts: 0.

Implementation handoff

The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.

1. Start with the pinned implementation excerpts and the Why it happens analysis above.
2. Change the implementation so it satisfies the stated Expected behavior; do not weaken or remove the reproduction assertions.
3. Run the focused reproduction command(s) and confirm the observed failures become passing tests:

pnpm test --run packages/effect/test/ExecutionPlanAttemptValidation.test.ts

4. Run the affected package's existing tests, then the repository lint and type checks before requesting review.

Audit provenance

• Audit base: c9b56ab507f224426ee8388dc450da447ec4715f
• Reproduction base: c9b56ab507f224426ee8388dc450da447ec4715f
• Findings: core-a-f-execution-plan-zero-attempts
• Initial patch: focused reproduction tests; implementation fix pending

[changeset-bot]

⚠️ No Changeset found

Latest commit: 30737c6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[pullfrog]

Important

The reproduction test is correct, but the implementation fix described in the PR body is not yet present. The PR title promises rejection of zero attempts, which will not happen until the guard in ExecutionPlan.ts is updated.

Reviewed changes

This PR adds a focused regression test for ExecutionPlan.make rejecting attempts: 0.

• Added packages/effect/test/ExecutionPlanAttemptValidation.test.ts with a single synchronous test asserting that passing attempts: 0 to ExecutionPlan.make throws.
• Ran pnpm test --run packages/effect/test/ExecutionPlanAttemptValidation.test.ts; the test fails exactly as expected, confirming it is a valid reproduction.

⚠️ Fix required before merge

ExecutionPlan.make currently guards the attempts check with if (options.attempts && options.attempts < 1) at packages/effect/src/ExecutionPlan.ts:182. Because 0 is falsy, the condition short-circuits and zero is accepted. The test in this PR correctly reproduces that bug.

To satisfy the PR's stated expected behavior, change the guard to explicitly distinguish absence from zero—for example, if (options.attempts !== undefined && options.attempts < 1). Also verify packages/effect/src/internal/executionPlan.ts:104-117, which uses truthy checks (step.attempts ? ...) to derive schedules; zero should not silently produce a default retry schedule there.

  ^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}