t3-code-android-nightly/.macroscope/check-run-agents/effect-service-conventions.md
Julius Marminge 094fb230e9
docs: server features are services, and handlers stay thin (#14613)
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-10-01 13:06:35 -07:00

76 lines
8.4 KiB
Markdown

---
title: Effect Service Conventions
model: gpt-6-sol
effort: max
input: incremental
tools:
- browse_code
- modify_pr
include:
- "apps/**/*.ts"
- "packages/**/*.ts"
- "infra/**/*.ts"
exclude:
- "**/*.test.ts"
requires:
- Check
maxBudgetPerRun: 5
maxBudgetPerPR: 25
conclusion: neutral
showToolCalls: 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.