Skip to content

Commit a800583

Browse files
authored
refactor(effect): unify service namespaces and align naming (#18093)
1 parent 171e69c commit a800583

35 files changed

Lines changed: 2034 additions & 2059 deletions

packages/opencode/AGENTS.md

Lines changed: 30 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -9,71 +9,55 @@
99
- **Output**: creates `migration/<timestamp>_<slug>/migration.sql` and `snapshot.json`.
1010
- **Tests**: migration tests should read the per-folder layout (no `_journal.json`).
1111

12-
# opencode Effect guide
12+
# opencode Effect rules
1313

14-
Instructions to follow when writing Effect.
14+
Use these rules when writing or migrating Effect code.
1515

16-
## Schemas
16+
See `specs/effect-migration.md` for the compact pattern reference and examples.
1717

18-
- Use `Schema.Class` for data types with multiple fields.
19-
- Use branded schemas (`Schema.brand`) for single-value types.
20-
21-
## Services
22-
23-
- Services use `ServiceMap.Service<ServiceName, ServiceName.Service>()("@console/<Name>")`.
24-
- In `Layer.effect`, always return service implementations with `ServiceName.of({ ... })`, never a plain object.
25-
26-
## Errors
27-
28-
- Use `Schema.TaggedErrorClass` for typed errors.
29-
- For defect-like causes, use `Schema.Defect` instead of `unknown`.
30-
- In `Effect.gen`, prefer `yield* new MyError(...)` over `yield* Effect.fail(new MyError(...))` for direct early-failure branches.
31-
32-
## Effects
18+
## Core
3319

3420
- Use `Effect.gen(function* () { ... })` for composition.
35-
- Use `Effect.fn("ServiceName.method")` for named/traced effects and `Effect.fnUntraced` for internal helpers.
36-
- `Effect.fn` / `Effect.fnUntraced` accept pipeable operators as extra arguments, so avoid unnecessary `flow` or outer `.pipe()` wrappers.
37-
- **`Effect.callback`** (not `Effect.async`) for callback-based APIs. The classic `Effect.async` was renamed to `Effect.callback` in effect-smol/v4.
38-
39-
## Time
40-
21+
- Use `Effect.fn("Domain.method")` for named/traced effects and `Effect.fnUntraced` for internal helpers.
22+
- `Effect.fn` / `Effect.fnUntraced` accept pipeable operators as extra arguments, so avoid unnecessary outer `.pipe()` wrappers.
23+
- Use `Effect.callback` for callback-based APIs.
4124
- Prefer `DateTime.nowAsDate` over `new Date(yield* Clock.currentTimeMillis)` when you need a `Date`.
4225

43-
## Errors
26+
## Schemas and errors
27+
28+
- Use `Schema.Class` for multi-field data.
29+
- Use branded schemas (`Schema.brand`) for single-value types.
30+
- Use `Schema.TaggedErrorClass` for typed errors.
31+
- Use `Schema.Defect` instead of `unknown` for defect-like causes.
32+
- In `Effect.gen` / `Effect.fn`, prefer `yield* new MyError(...)` over `yield* Effect.fail(new MyError(...))` for direct early-failure branches.
4433

45-
- In `Effect.gen/fn`, prefer `yield* new MyError(...)` over `yield* Effect.fail(new MyError(...))` for direct early-failure branches.
34+
## Runtime vs Instances
4635

47-
## Instance-scoped Effect services
36+
- Use the shared runtime for process-wide services with one lifecycle for the whole app.
37+
- Use `src/effect/instances.ts` for per-directory or per-project services that need `InstanceContext`, per-instance state, or per-instance cleanup.
38+
- If two open directories should not share one copy of the service, it belongs in `Instances`.
39+
- Instance-scoped services should read context from `InstanceContext`, not `Instance.*` globals.
4840

49-
Services that need per-directory lifecycle (created/destroyed per instance) go through the `Instances` LayerMap:
41+
## Preferred Effect services
5042

51-
1. Define a `ServiceMap.Service` with a `static readonly layer` (see `FileWatcherService`, `QuestionService`, `PermissionService`, `ProviderAuthService`).
52-
2. Add it to `InstanceServices` union and `Layer.mergeAll(...)` in `src/effect/instances.ts`.
53-
3. Use `InstanceContext` inside the layer to read `directory` and `project` instead of `Instance.*` globals.
54-
4. Call from legacy code via `runPromiseInstance(MyService.use((s) => s.method()))`.
43+
- In effectified services, prefer yielding existing Effect services over dropping down to ad hoc platform APIs.
44+
- Prefer `FileSystem.FileSystem` instead of raw `fs/promises` for effectful file I/O.
45+
- Prefer `ChildProcessSpawner.ChildProcessSpawner` with `ChildProcess.make(...)` instead of custom process wrappers.
46+
- Prefer `HttpClient.HttpClient` instead of raw `fetch`.
47+
- Prefer `Path.Path`, `Config`, `Clock`, and `DateTime` when those concerns are already inside Effect code.
48+
- For background loops or scheduled tasks, use `Effect.repeat` or `Effect.schedule` with `Effect.forkScoped` in the layer definition.
5549

56-
### Instance.bind — ALS context for native callbacks
50+
## Instance.bind — ALS for native callbacks
5751

58-
`Instance.bind(fn)` captures the current Instance AsyncLocalStorage context and returns a wrapper that restores it synchronously when called.
52+
`Instance.bind(fn)` captures the current Instance AsyncLocalStorage context and restores it synchronously when called.
5953

60-
**Use it** when passing callbacks to native C/C++ addons (`@parcel/watcher`, `node-pty`, native `fs.watch`, etc.) that need to call `Bus.publish`, `Instance.state()`, or anything that reads `Instance.directory`.
54+
Use it for native addon callbacks (`@parcel/watcher`, `node-pty`, native `fs.watch`, etc.) that need to call `Bus.publish`, `Instance.state()`, or anything that reads `Instance.directory`.
6155

62-
**Don't need it** for `setTimeout`, `Promise.then`, `EventEmitter.on`, or Effect fibers — Node.js ALS propagates through those automatically.
56+
You do not need it for `setTimeout`, `Promise.then`, `EventEmitter.on`, or Effect fibers.
6357

6458
```typescript
65-
// Native addon callback — needs Instance.bind
6659
const cb = Instance.bind((err, evts) => {
6760
Bus.publish(MyEvent, { ... })
6861
})
6962
nativeAddon.subscribe(dir, cb)
7063
```
71-
72-
## Flag → Effect.Config migration
73-
74-
Flags in `src/flag/flag.ts` are being migrated from static `truthy(...)` reads to `Config.boolean(...).pipe(Config.withDefault(false))` as their consumers get effectified.
75-
76-
- Effectful flags return `Config<boolean>` and are read with `yield*` inside `Effect.gen`.
77-
- The default `ConfigProvider` reads from `process.env`, so env vars keep working.
78-
- Tests can override via `ConfigProvider.layer(ConfigProvider.fromUnknown({ ... }))`.
79-
- Keep all flags in `flag.ts` as the single registry — just change the implementation from `truthy()` to `Config.boolean()` when the consumer moves to Effect.
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
# Effect patterns
2+
3+
Practical reference for new and migrated Effect code in `packages/opencode`.
4+
5+
## Choose scope
6+
7+
Use the shared runtime for process-wide services with one lifecycle for the whole app.
8+
9+
Use `src/effect/instances.ts` for services that are created per directory or need `InstanceContext`, per-project state, or per-instance cleanup.
10+
11+
- Shared runtime: config readers, stateless helpers, global clients
12+
- Instance-scoped: watchers, per-project caches, session state, project-bound background work
13+
14+
Rule of thumb: if two open directories should not share one copy of the service, it belongs in `Instances`.
15+
16+
## Service shape
17+
18+
For a fully migrated module, use the public namespace directly:
19+
20+
```ts
21+
export namespace Foo {
22+
export interface Interface {
23+
readonly get: (id: FooID) => Effect.Effect<FooInfo, FooError>
24+
}
25+
26+
export class Service extends ServiceMap.Service<Service, Interface>()("@opencode/Foo") {}
27+
28+
export const layer = Layer.effect(
29+
Service,
30+
Effect.gen(function* () {
31+
return Service.of({
32+
get: Effect.fn("Foo.get")(function* (id) {
33+
return yield* ...
34+
}),
35+
})
36+
}),
37+
)
38+
39+
export const defaultLayer = layer.pipe(Layer.provide(FooRepo.defaultLayer))
40+
}
41+
```
42+
43+
Rules:
44+
45+
- Keep `Interface`, `Service`, `layer`, and `defaultLayer` on the owning namespace
46+
- Export `defaultLayer` only when wiring dependencies is useful
47+
- Use the direct namespace form once the module is fully migrated
48+
49+
## Temporary mixed-mode pattern
50+
51+
Prefer a single namespace whenever possible.
52+
53+
Use a `*Effect` namespace only when there is a real mixed-mode split, usually because a legacy boundary facade still exists or because merging everything immediately would create awkward cycles.
54+
55+
```ts
56+
export namespace FooEffect {
57+
export interface Interface {
58+
readonly get: (id: FooID) => Effect.Effect<Foo, FooError>
59+
}
60+
61+
export class Service extends ServiceMap.Service<Service, Interface>()("@opencode/Foo") {}
62+
63+
export const layer = Layer.effect(...)
64+
}
65+
```
66+
67+
Then keep the old boundary thin:
68+
69+
```ts
70+
export namespace Foo {
71+
export function get(id: FooID) {
72+
return runtime.runPromise(FooEffect.Service.use((svc) => svc.get(id)))
73+
}
74+
}
75+
```
76+
77+
Remove the `Effect` suffix when the boundary split is gone.
78+
79+
## Scheduled Tasks
80+
81+
For loops or periodic work, use `Effect.repeat` or `Effect.schedule` with `Effect.forkScoped` in the layer definition.
82+
83+
## Preferred Effect services
84+
85+
In effectified services, prefer yielding existing Effect services over dropping down to ad hoc platform APIs.
86+
87+
Prefer these first:
88+
89+
- `FileSystem.FileSystem` instead of raw `fs/promises` for effectful file I/O
90+
- `ChildProcessSpawner.ChildProcessSpawner` with `ChildProcess.make(...)` instead of custom process wrappers
91+
- `HttpClient.HttpClient` instead of raw `fetch`
92+
- `Path.Path` instead of mixing path helpers into service code when you already need a path service
93+
- `Config` for effect-native configuration reads
94+
- `Clock` / `DateTime` for time reads inside effects
95+
96+
## Child processes
97+
98+
For child process work in services, yield `ChildProcessSpawner.ChildProcessSpawner` in the layer and use `ChildProcess.make(...)`.
99+
100+
Keep shelling-out code inside the service, not in callers.
101+
102+
## Shared leaf models
103+
104+
Shared schema or model files can stay outside the service namespace when lower layers also depend on them.
105+
106+
That is fine for leaf files like `schema.ts`. Keep the service surface in the owning namespace.
107+
108+
## Migration checklist
109+
110+
Done now:
111+
112+
- [x] `AccountEffect` (mixed-mode)
113+
- [x] `AuthEffect` (mixed-mode)
114+
- [x] `TruncateEffect` (mixed-mode)
115+
- [x] `Question`
116+
- [x] `PermissionNext`
117+
- [x] `ProviderAuth`
118+
- [x] `FileWatcher`
119+
- [x] `FileTime`
120+
- [x] `Format`
121+
- [x] `Vcs`
122+
- [x] `Skill`
123+
- [x] `Discovery`
124+
- [x] `File`
125+
- [x] `Snapshot`
126+
127+
Still open and likely worth migrating:
128+
129+
- [ ] `Plugin`
130+
- [ ] `ToolRegistry`
131+
- [ ] `Pty`
132+
- [ ] `Worktree`
133+
- [ ] `Installation`
134+
- [ ] `Bus`
135+
- [ ] `Command`
136+
- [ ] `Config`
137+
- [ ] `Session`
138+
- [ ] `SessionProcessor`
139+
- [ ] `SessionPrompt`
140+
- [ ] `SessionCompaction`
141+
- [ ] `Provider`
142+
- [ ] `Project`
143+
- [ ] `LSP`
144+
- [ ] `MCP`
Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ const mapAccountServiceError =
108108
),
109109
)
110110

111-
export namespace AccountService {
112-
export interface Service {
111+
export namespace AccountEffect {
112+
export interface Interface {
113113
readonly active: () => Effect.Effect<Option.Option<Account>, AccountError>
114114
readonly list: () => Effect.Effect<Account[], AccountError>
115115
readonly orgsByAccount: () => Effect.Effect<readonly AccountOrgs[], AccountError>
@@ -124,11 +124,11 @@ export namespace AccountService {
124124
readonly login: (url: string) => Effect.Effect<Login, AccountError>
125125
readonly poll: (input: Login) => Effect.Effect<PollResult, AccountError>
126126
}
127-
}
128127

129-
export class AccountService extends ServiceMap.Service<AccountService, AccountService.Service>()("@opencode/Account") {
130-
static readonly layer: Layer.Layer<AccountService, never, AccountRepo | HttpClient.HttpClient> = Layer.effect(
131-
AccountService,
128+
export class Service extends ServiceMap.Service<Service, Interface>()("@opencode/Account") {}
129+
130+
export const layer: Layer.Layer<Service, never, AccountRepo | HttpClient.HttpClient> = Layer.effect(
131+
Service,
132132
Effect.gen(function* () {
133133
const repo = yield* AccountRepo
134134
const http = yield* HttpClient.HttpClient
@@ -148,8 +148,6 @@ export class AccountService extends ServiceMap.Service<AccountService, AccountSe
148148
mapAccountServiceError("HTTP request failed"),
149149
)
150150

151-
// Returns a usable access token for a stored account row, refreshing and
152-
// persisting it when the cached token has expired.
153151
const resolveToken = Effect.fnUntraced(function* (row: AccountRow) {
154152
const now = yield* Clock.currentTimeMillis
155153
if (row.token_expiry && row.token_expiry > now) return row.access_token
@@ -218,11 +216,11 @@ export class AccountService extends ServiceMap.Service<AccountService, AccountSe
218216
)
219217
})
220218

221-
const token = Effect.fn("AccountService.token")((accountID: AccountID) =>
219+
const token = Effect.fn("Account.token")((accountID: AccountID) =>
222220
resolveAccess(accountID).pipe(Effect.map(Option.map((r) => r.accessToken))),
223221
)
224222

225-
const orgsByAccount = Effect.fn("AccountService.orgsByAccount")(function* () {
223+
const orgsByAccount = Effect.fn("Account.orgsByAccount")(function* () {
226224
const accounts = yield* repo.list()
227225
const [errors, results] = yield* Effect.partition(
228226
accounts,
@@ -237,7 +235,7 @@ export class AccountService extends ServiceMap.Service<AccountService, AccountSe
237235
return results
238236
})
239237

240-
const orgs = Effect.fn("AccountService.orgs")(function* (accountID: AccountID) {
238+
const orgs = Effect.fn("Account.orgs")(function* (accountID: AccountID) {
241239
const resolved = yield* resolveAccess(accountID)
242240
if (Option.isNone(resolved)) return []
243241

@@ -246,7 +244,7 @@ export class AccountService extends ServiceMap.Service<AccountService, AccountSe
246244
return yield* fetchOrgs(account.url, accessToken)
247245
})
248246

249-
const config = Effect.fn("AccountService.config")(function* (accountID: AccountID, orgID: OrgID) {
247+
const config = Effect.fn("Account.config")(function* (accountID: AccountID, orgID: OrgID) {
250248
const resolved = yield* resolveAccess(accountID)
251249
if (Option.isNone(resolved)) return Option.none()
252250

@@ -270,7 +268,7 @@ export class AccountService extends ServiceMap.Service<AccountService, AccountSe
270268
return Option.some(parsed.config)
271269
})
272270

273-
const login = Effect.fn("AccountService.login")(function* (server: string) {
271+
const login = Effect.fn("Account.login")(function* (server: string) {
274272
const response = yield* executeEffectOk(
275273
HttpClientRequest.post(`${server}/auth/device/code`).pipe(
276274
HttpClientRequest.acceptJson,
@@ -291,7 +289,7 @@ export class AccountService extends ServiceMap.Service<AccountService, AccountSe
291289
})
292290
})
293291

294-
const poll = Effect.fn("AccountService.poll")(function* (input: Login) {
292+
const poll = Effect.fn("Account.poll")(function* (input: Login) {
295293
const response = yield* executeEffectOk(
296294
HttpClientRequest.post(`${input.server}/auth/device/token`).pipe(
297295
HttpClientRequest.acceptJson,
@@ -337,7 +335,7 @@ export class AccountService extends ServiceMap.Service<AccountService, AccountSe
337335
return new PollSuccess({ email: account.email })
338336
})
339337

340-
return AccountService.of({
338+
return Service.of({
341339
active: repo.active,
342340
list: repo.list,
343341
orgsByAccount,
@@ -352,8 +350,5 @@ export class AccountService extends ServiceMap.Service<AccountService, AccountSe
352350
}),
353351
)
354352

355-
static readonly defaultLayer = AccountService.layer.pipe(
356-
Layer.provide(AccountRepo.layer),
357-
Layer.provide(FetchHttpClient.layer),
358-
)
353+
export const defaultLayer = layer.pipe(Layer.provide(AccountRepo.layer), Layer.provide(FetchHttpClient.layer))
359354
}

packages/opencode/src/account/index.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,20 @@ import {
55
type AccountError,
66
type AccessToken,
77
AccountID,
8-
AccountService,
8+
AccountEffect,
99
OrgID,
10-
} from "./service"
10+
} from "./effect"
1111

12-
export { AccessToken, AccountID, OrgID } from "./service"
12+
export { AccessToken, AccountID, OrgID } from "./effect"
1313

1414
import { runtime } from "@/effect/runtime"
1515

16-
function runSync<A>(f: (service: AccountService.Service) => Effect.Effect<A, AccountError>) {
17-
return runtime.runSync(AccountService.use(f))
16+
function runSync<A>(f: (service: AccountEffect.Interface) => Effect.Effect<A, AccountError>) {
17+
return runtime.runSync(AccountEffect.Service.use(f))
1818
}
1919

20-
function runPromise<A>(f: (service: AccountService.Service) => Effect.Effect<A, AccountError>) {
21-
return runtime.runPromise(AccountService.use(f))
20+
function runPromise<A>(f: (service: AccountEffect.Interface) => Effect.Effect<A, AccountError>) {
21+
return runtime.runPromise(AccountEffect.Service.use(f))
2222
}
2323

2424
export namespace Account {

0 commit comments

Comments
 (0)