Omit removed services from Context.addOrOmit result types

[fubhy]

Summary

TypeScript accepts Context.get for a service removed by addOrOmit(None), but the accepted call throws Service not found at runtime.

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.

addOrOmit(None) types a removed service as present

Module: Context
Audit ID: core-a-f-context-add-or-omit-none-type
Severity / confidence: medium / high

What happens

TypeScript accepts Context.get for a service removed by addOrOmit(None), but the accepted call throws Service not found at runtime.

Why it happens

Both overloads return Context<Services | I> regardless of the Option branch, while the implementation deletes key.key for None.

Expected behavior

addOrOmit stores the service for Some and removes its key for None; the resulting context type must not prove that a removed service is available to Context.get.

Relevant implementation

These links and excerpts are pinned to audit base c9b56ab507f224426ee8388dc450da447ec4715f.

• packages/effect/src/Context.ts:741-762

View problematic code at packages/effect/src/Context.ts:741-762

export const addOrOmit: {
  <I, S>(
    key: Key<I, S>,
    service: Option.Option<Types.NoInfer<S>>
  ): <Services>(self: Context<Services>) => Context<Services | I>
  <Services, I, S>(
    self: Context<Services>,
    key: Key<I, S>,
    service: Option.Option<Types.NoInfer<S>>
  ): Context<Services | I>
} = dual(3, <Services, I, S>(
  self: Context<Services>,
  key: Key<I, S>,
  service: Option.Option<Types.NoInfer<S>>
): Context<Services | I> =>
  withMapUnsafe(self, (map) => {
    if (service._tag === "None") {
      map.delete(key.key)
    } else {
      map.set(key.key, service.value)
    }
  }))

View exact lines on GitHub

Reproduction

pnpm test-types ContextAddOrOmitNone.tst.ts; pnpm test --run packages/effect/test/ContextAddOrOmitNone.test.ts

Observed failure: The type repro accepted Context.get, while the runtime repro threw Service not found: AuditService.

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-types ContextAddOrOmitNone.tst.ts; pnpm test --run packages/effect/test/ContextAddOrOmitNone.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-context-add-or-omit-none-type
• Initial patch: focused reproduction tests; implementation fix pending

[changeset-bot]

⚠️ No Changeset found

Latest commit: 33db699

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 two reproduction tests currently assert the buggy behavior, not the desired fixed behavior. Flip them before adding the implementation fix so the PR actually validates the type change.

Reviewed changes

Reviewed the two new regression test files on this branch that will exercise Context.addOrOmit(Option.none()).

• packages/effect/test/ContextAddOrOmitNone.test.ts — runtime assertion for accessing a service after it was omitted.
• packages/effect/typetest/ContextAddOrOmitNone.tst.ts — type-level assertion for accessing a service after it was omitted.

ℹ️ Reminder

The PR body notes the Context.addOrOmit implementation fix is still pending. When that implementation lands, it will change the exported overload return type, so a changeset for effect should be included. Tests-only changes do not need one.

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

[pullfrog]

The runtime behavior is already correct — addOrOmit(Option.none()) deletes the key, so Context.get throws "Service not found". This doesNotThrow assertion therefore encodes the current runtime-safe bug rather than validating the fix. If a runtime regression test is needed, use assert.throws(...); otherwise move the assertion to the type test and drop this file.

[pullfrog]

This assertion currently passes and declares that Context.get(context, Service) should type-check — exactly the buggy behavior the PR aims to fix. Once addOrOmit omits I from the result type for None, the call should become a type error. Use expect(Context.get).type.not.toBeCallableWith(context, Service) (or an equivalent @ts-expect-error guard) instead.