Skip to content

refactor(effect): unify service namespaces and align naming#18093

Merged
kitlangton merged 6 commits intodevfrom
refactor/unify-effect-namespaces
Mar 18, 2026
Merged

refactor(effect): unify service namespaces and align naming#18093
kitlangton merged 6 commits intodevfrom
refactor/unify-effect-namespaces

Conversation

@kitlangton
Copy link
Contributor

@kitlangton kitlangton commented Mar 18, 2026

Summary

Unifies all Effect service namespaces to follow the patterns in EFFECT_MIGRATION_PLAN.md.

Fully-migrated modules (single namespace, no suffix)

Collapse the two-namespace pattern (Foo + FooService class) into a single domain namespace with Interface, Service, layer, defaultLayer, and facade functions:

  • File, FileTime, FileWatcher, Format, Vcs, Skill, Snapshot, Discovery

Mixed-mode modules collapsed into index

These had separate effect.ts + index.ts files. Since they go through instances.ts (lazy via LayerMap), there's no circular import — collapsed into one file:

  • Questionquestion/service.ts deleted, everything in question/index.ts
  • PermissionNextpermission/service.ts deleted, everything in permission/index.ts
  • ProviderAuthprovider/auth-service.ts deleted, everything in provider/auth.ts

Mixed-mode modules kept separate (eager runtime import)

These are imported eagerly by runtime.ts for ManagedRuntime.make, so they can't be collapsed without circular deps:

  • AccountEffectaccount/service.tsaccount/effect.ts
  • AuthEffectauth/service.tsauth/effect.ts

Other changes

  • AuthServiceErrorAuthError
  • DiscoveryServiceDiscovery
  • ProviderAuth.methods() return type narrowed to Record<ProviderID, Method[]> with ProviderID.make() at the plugin boundary
  • Removed remeda dependency from provider/auth.ts (replaced with Effect Array.filterMap)
  • Bus subscriptions in Vcs and Format converted to Effect.acquireRelease
  • Removed no-op init() methods and unnecessary defaultLayer = layer aliases
  • All Effect.fn trace names use bare domain ("File.read", "Permission.ask")
  • All interfaces named Interface consistently
  • Fixed sortHiddenLast double hidden() call
  • Fixed Snapshot revert to use first patch hash for overlapping files (was using last due to Map overwrite)
  • Fixed Windows CI: dispose instances before temp dir cleanup in project-init-git test

Test plan

  • tsc --noEmit passes
  • bun turbo typecheck passes across all packages
  • 135 tests pass across 7 changed test files
  • New test: snapshot revert with overlapping patches
  • Full local test suite: 1336 pass (5 pre-existing flaky failures in FileWatcher/Session unrelated to this PR)

@kitlangton kitlangton force-pushed the refactor/unify-effect-namespaces branch 5 times, most recently from 7b1e927 to 6e5e24f Compare March 18, 2026 14:36
@kitlangton kitlangton marked this pull request as ready for review March 18, 2026 14:37
@kitlangton kitlangton force-pushed the refactor/unify-effect-namespaces branch 4 times, most recently from bb00a9f to ce6a32e Compare March 18, 2026 15:14
@kitlangton
Copy link
Contributor Author

CI status: Linux unit tests pass. Windows unit test failure is pre-existing on dev (last 4 dev test runs all fail on Windows). The Windows error is an unhandled ENOENT from ripgrep.ts:238 between test files — a background file indexing race that exists independent of this PR.

@kitlangton kitlangton force-pushed the refactor/unify-effect-namespaces branch from 422f628 to f84b148 Compare March 18, 2026 16:55
Fully-migrated modules now use a single domain namespace with Interface,
Service, layer, and defaultLayer directly on it — collapsing the old
two-namespace pattern (e.g. File + FileService → File.Service).

Mixed-mode modules renamed to *Effect suffix per migration plan:
  - account/service.ts → account/effect.ts (AccountEffect)
  - auth/service.ts → auth/effect.ts (AuthEffect)
  - question/service.ts → question/effect.ts (QuestionEffect)
  - permission/service.ts → permission/effect.ts (PermissionEffect)
  - provider/auth-service.ts → provider/auth-effect.ts (ProviderAuthEffect)

Other cleanups:
  - DiscoveryService → Discovery for naming consistency
  - AuthServiceError → AuthError
  - Remove no-op init() methods and unnecessary defaultLayer = layer
  - Un-export internal-only helpers (stamp, session in FileTime)
  - Fix double hidden() call in sortHiddenLast
  - Effect.fn trace names use bare domain (e.g. "File.read")
Verifies that when the same file appears in multiple patches passed to
revert(), the first patch's snapshot hash is used — not the last.
Instance.reload triggers InstanceBootstrap which fires File.init()
without await (intentionally async warmup). On Windows, the background
Ripgrep.files() call would fail with ENOENT after the temp directory
was deleted by `await using tmp`. Disposing instances in the finally
block ensures background work is torn down before the directory goes away.
kick() is fire-and-forget background work. If the instance directory is
removed before indexing completes (e.g. test cleanup on Windows), the
unhandled ENOENT from Ripgrep.files() would crash the process. Catch
and leave the cache as-is instead.
@kitlangton kitlangton force-pushed the refactor/unify-effect-namespaces branch 2 times, most recently from dad6d16 to 0369451 Compare March 18, 2026 17:03
Replace the fire-and-forget Promise-based kick() with an Effect fiber
scoped to the layer. When the instance is disposed, Effect interrupts
the fiber automatically — no more unhandled ENOENT from Ripgrep when
the directory is deleted during test cleanup.

File.init() now joins the fiber (waits for scan completion) instead of
awaiting a raw Promise.
@kitlangton kitlangton force-pushed the refactor/unify-effect-namespaces branch from 0369451 to 36d02d5 Compare March 18, 2026 17:15
@kitlangton kitlangton enabled auto-merge (squash) March 18, 2026 17:19
@kitlangton kitlangton disabled auto-merge March 18, 2026 17:33
@kitlangton kitlangton merged commit a800583 into dev Mar 18, 2026
6 of 8 checks passed
@kitlangton kitlangton deleted the refactor/unify-effect-namespaces branch March 18, 2026 17:34
AbhishekChorotiya pushed a commit to AbhishekChorotiya/opencode that referenced this pull request Mar 19, 2026
AvatarGanymede pushed a commit to AvatarGanymede/opencode-dev that referenced this pull request Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant