| title |
model |
effort |
input |
tools |
include |
exclude |
requires |
maxBudgetPerRun |
maxBudgetPerPR |
conclusion |
showToolCalls |
| Effect Service Conventions |
gpt-6-sol |
max |
incremental |
|
| apps/**/*.ts |
| packages/**/*.ts |
| infra/**/*.ts |
|
|
|
5 |
25 |
neutral |
true |
Effect service review
Review changed TypeScript for the conventions below. They apply when a pull request creates, moves, refactors, or consumes an Effect service, or adds server behavior. Authors read the same rules in docs/internals/effect-services.md; keep the two in step. Review only the lines the PR changed; older code in the same file that predates these conventions is not a finding. Do not demand repository-wide cleanup.
Imports and module namespaces
- Import Effect modules from their subpaths as namespaces:
import * as Effect from "effect/Effect", import * as Layer from "effect/Layer". Flag named imports from the bare "effect" package.
- At a service boundary, import the local service module as a namespace and use its public shape:
WorkspacePaths.WorkspacePaths, WorkspacePaths.make, WorkspacePaths.layer. Flag aliases such as import { layer as workspacePathsLayer } that erase the namespace.
- Named imports stay correct for whole packages such as
@t3tools/contracts and for modules used only for a pure helper, error, schema, config value, or type. Do not request import type * as Contracts.
- When a barrel exposes a whole service module, prefer
export * as TokenStore from "./tokenStore.ts" over individually renamed make and layer exports.
Placement
- Flag new or broadened feature logic in a transport handler: a WebSocket RPC handler in
apps/server/src/ws.ts, an HTTP route handler, or an MCP tool handler. A handler decodes input, calls one service method, and maps the service's errors to the transport's error. Filesystem, Git, process, or persistence work, folder naming, multi-step command dispatch, retries, or rollback inside a handler belongs in a service method; ask for it to move to the domain's existing service, or a new one when none owns the domain. Existing inline handlers are legacy; flag only changed lines.
- Flag a new exported free function that does a server capability's effectful work (filesystem, Git, processes, persisted state) and is called from a handler, when it should be a service method. Plain functions for pure work (formatting, naming, parsing) are fine.
Service definition
- One canonical module per service in this order: imports, error and schema declarations, the
Context.Service tag with its interface inline, make, then layer.
- Define the interface inline in
Context.Service. Do not add a standalone FooShape interface; refer to the inferred type as Foo["Service"].
- Define a real
make when the module owns construction, and export it only when another module imports it; knip fails CI on an unused export, so do not ask for an export nothing uses. Do not write make = Effect.succeed(...) only to force Layer.effect; use Layer.succeed, Layer.scoped, or whichever constructor matches.
- Use plain
make and layer in a module named for its implementation (BunPtyAdapter.ts). Keep implementation-specific names when one abstract port module holds several implementations (makeCloudflaredRelayClient, layerCloudflared in RelayClient.ts). infra/relay/src/db.ts may keep its inline Layer.succeed(RelayDb, db).
- When a service moves, delete the old files and update every consumer, including orchestration, MCP, tests, and integration harnesses. Do not leave compatibility re-export shims.
Dependency acquisition and runtime boundaries
- Production service construction acquires its Effect dependencies from the environment with
yield* Foo.Foo, and make/layer types expose those requirements. Flag a factory that takes Foo["Service"] (or an object of Effect-returning methods) as a parameter when that value is a service dependency. Passing service instances explicitly in tests is fine; passing pure configuration, immutable domain values, or deliberate callback strategies is not service injection.
- Do not hide dependencies in module globals, closures over singleton services, or a
Layer.succeed whose implementation calls runtime-backed or imperative APIs.
ManagedRuntime.make, runPromise, and runPromiseExit belong at application or framework boundaries: React, native callbacks, CLI, HTTP adapters. Flag them in domain services, repositories, persistence, and service constructors. A named imperative adapter may bridge an Effect service into a Promise API but must not become a dependency of another Effect service.
- Do not create per-feature managed runtimes or Atom runtimes to hand the same owned resource to several consumers. Compose the resource once in an application-owned layer and provide its context to integration runtimes.
- When acquisition can fail and callers need fallback behavior, keep the failure typed in Effect (an error in the service operation or an explicit optional-service layer) rather than bypassing the layer through an imperative runtime.
Errors
- Define service failures with
Schema.TaggedError and structured attributes: operation or stage, resource path or entity identifier, normalized category or status. Derive message from those attributes only. Never derive it from cause, cause.message, or a stringified defect, and do not add a detail field that copies cause.message.
- When wrapping a real failure, keep the immediate underlying error as
cause so the chain and stack survive. Make cause required if every construction wraps a failure. Pure validation or domain errors created without an underlying failure need no cause.
- Keep attributes and log annotations safe and bounded: no raw wire payloads, command arguments or output, signed URLs, credentials, query strings, or arbitrary defect text. Preserve the exact value only as
cause; expose normalized categories, lengths, counts, and safe URL protocol or hostname where useful.
- At a translation boundary, pass through an already structured domain error when it is part of the target error channel; wrap only unknown or lower-level failures. Map failures where the context is known instead of wrapping a whole multi-step pipeline in one generic error.
- Do not encode the same distinction twice with both a specific error tag and a single-value
operation, reason, kind, or phase literal. Split into separate error classes when a discriminator drives caller control flow or the user-facing message; a discriminator used only for diagnostics may stay a field. Caller-visible messages exposed through HTTP, RPC, persisted state, or UI are behavior and must survive a structural refactor.
- Do not add a helper whose only behavior is
(...args) => new SomeError({ ...args }). Construct the error at the failure boundary. Keep a mapper only when it performs real normalization, passes through domain errors, or adds reusable context; when such a mapper belongs to the target error type, prefer a static factory on that class.
- Export predicates directly as
export const isFoo = Schema.is(Foo). Flag a private Schema.is constant wrapped by a function with the same signature.
- Catch statically known tagged failures with
Effect.catchTags({ ... }), including for a single tag; do not use catchTag or catchIf with a schema predicate for that. Effect.catch is fine when the whole error channel is handled; catchIf is fine for structural predicates such as a platform error code.
Change discipline
- Every new or broadened directive that disables a lint, type-checker, LSP, or static-analysis diagnostic needs an adjacent comment explaining why. The directive itself is not an explanation; a missing one is a concrete violation.
- If backend behavior changes, require focused tests that use test layers for external services only, never mocks of core business logic. Do not require new tests for mechanical refactors or import-only changes.
- Do not require
Layer.effect, universal namespace imports, generic make/layer names for abstract-port implementations, or separate error classes for diagnostic-only fields.
Reporting
Report only violations introduced by changed lines. Post each as a precise inline comment on the smallest relevant range and state the expected fix. A clear convention violation may fail the check; optional style preferences and untouched legacy code may not.
When there are no findings, make the entire final response exactly All clear on one line with nothing else.