Preserve sibling input evidence when Config.all fails

[fubhy]

Summary

When a nested Config.all has at least one supplied value but another child fails, an enclosing withDefault can silently replace the entire parent with its fallback. A partially supplied group should fail; only a completely absent group should use the parent fallback.

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.

Config.all loses sibling input evidence

Module: Config
Audit ID: core-a-f-config-all-loses-input-evidence
Severity / confidence: medium / high

What happens

When a nested Config.all has at least one supplied value but another child fails, an enclosing withDefault can silently replace the entire parent with its fallback. A partially supplied group should fail; only a completely absent group should use the parent fallback.

Why it happens

Config.all evaluates its children with Effect.all, which short-circuits on an EvaluationFailure before resolveArray or resolveRecord can combine sibling hasInput flags. A failing child with hasInput false can therefore erase evidence that another child successfully read provider input.

Expected behavior

Config.withDefault may replace a Config.all parent only when none of its children read provider input. If any child had input while a sibling is missing or fails, parsing must fail instead of defaulting the entire parent.

Relevant implementation

These links and excerpts are pinned to audit base c9b56ab507f224426ee8388dc450da447ec4715f.

• packages/effect/src/Config.ts:415-427
• packages/effect/src/Config.ts:343-354
• packages/effect/src/Config.ts:431-469

View problematic code at packages/effect/src/Config.ts:415-427

    return make((provider, pathPrefix) =>
      Effect.flatMapEager(
        Effect.all(configs.map((config) => evaluateAt(config, provider, pathPrefix))),
        resolveArray
      )
    ) as any
  } else {
    return make((provider, pathPrefix) =>
      Effect.flatMapEager(
        Effect.all(Rec.map(configs, (config) => evaluateAt(config, provider, pathPrefix))),
        resolveRecord
      )
    ) as any

View exact lines on GitHub

View problematic code at packages/effect/src/Config.ts:343-354

  return make<A | A2>((provider, pathPrefix) =>
    Effect.matchEffect(evaluateAt(self, provider, pathPrefix), {
      onFailure: (failure) =>
        preserveInputEvidence(
          evaluateAt(that(failure.error), provider, pathPrefix),
          failure.hasInput
        ),
      onSuccess: (resolution): Effect.Effect<Resolution<A | A2>, EvaluationFailure> =>
        resolution._tag === "Absent"
          ? evaluateAt(that(resolution.error), provider, pathPrefix)
          : Effect.succeed(resolution)
    })

View exact lines on GitHub

View problematic code at packages/effect/src/Config.ts:431-469

const resolveArray = (
  resolutions: ReadonlyArray<Resolution<any>>
): Effect.Effect<Resolution<Array<any>>, EvaluationFailure> => {
  const values: Array<any> = []
  let firstAbsent: Absent | undefined
  let hasInput = false
  for (const resolution of resolutions) {
    if (resolution._tag === "Absent") {
      firstAbsent ??= resolution
    } else {
      values.push(resolution.value)
      hasInput = hasInput || resolution.hasInput
    }
  }
  if (firstAbsent !== undefined) {
    return hasInput ? Effect.fail(evaluationFailure(firstAbsent.error, true)) : Effect.succeed(firstAbsent)
  }
  return Effect.succeed(resolved(values, hasInput))
}

const resolveRecord = (
  resolutions: Record<string, Resolution<any>>
): Effect.Effect<Resolution<Record<string, any>>, EvaluationFailure> => {
  const values: Record<string, any> = {}
  let firstAbsent: Absent | undefined
  let hasInput = false
  for (const key in resolutions) {
    const resolution = resolutions[key]
    if (resolution._tag === "Absent") {
      firstAbsent ??= resolution
    } else {
      InternalRecord.assignProperty(values, key, resolution.value)
      hasInput = hasInput || resolution.hasInput
    }
  }
  if (firstAbsent !== undefined) {
    return hasInput ? Effect.fail(evaluationFailure(firstAbsent.error, true)) : Effect.succeed(firstAbsent)
  }
  return Effect.succeed(resolved(values, hasInput))

View exact lines on GitHub

Reproduction

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

Observed failure: The outer default incorrectly succeeded instead of preserving the partial-input failure.

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/ConfigAllInputEvidence.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-config-all-loses-input-evidence
• Initial patch: focused reproduction tests; implementation fix pending

[changeset-bot]

⚠️ No Changeset found

Latest commit: 3e85f43

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

This PR currently contains only the failing regression test; the implementation fix described in the PR body is not present, so CI fails. Please add the Config.all fix before merging.

Reviewed changes

Reviewed the single new regression test in packages/effect/test/ConfigAllInputEvidence.test.ts against head 3e85f43 on branch audit/repro-core-config-all-input-evidence.

• Adds one focused test that reproduces the input-evidence bug: a nested Config.all with one sibling that reads provider input and another that fails currently causes an enclosing Config.withDefault to replace the whole group.

The test is sound. I ran pnpm test --run packages/effect/test/ConfigAllInputEvidence.test.ts and it fails as expected, returning the default value instead of the required-field error. Once packages/effect/src/Config.ts is updated so Config.all preserves sibling hasInput evidence, this test should pass.

⚠️ Implementation fix is still pending

The PR title and description promise a fix, but only the test has been pushed. The fix needs to land in packages/effect/src/Config.ts around the Config.all implementation (lines 414–427). Specifically, Config.all must collect all child results rather than short-circuiting on the first EvaluationFailure, so that resolveArray and resolveRecord can combine sibling hasInput flags and fail with hasInput=true when any sibling read input.

No implementation concerns were found in the test itself.

  ^{｜ ⚠️ 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) ｜ 𝕏}