diff --git a/.macroscope/check-run-agents/effect-service-conventions.md b/.macroscope/check-run-agents/effect-service-conventions.md index 66ee81306a5a..52fd09fd1b33 100644 --- a/.macroscope/check-run-agents/effect-service-conventions.md +++ b/.macroscope/check-run-agents/effect-service-conventions.md @@ -22,7 +22,7 @@ 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. 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. +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 @@ -31,11 +31,16 @@ Review changed TypeScript for the conventions below. They apply when a pull requ - 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"]`. -- Export a real `make` when the module owns construction. Do not write `make = Effect.succeed(...)` only to force `Layer.effect`; use `Layer.succeed`, `Layer.scoped`, or whichever constructor matches. +- 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. diff --git a/AGENTS.md b/AGENTS.md index 00425996ccc5..1a3001c7adcb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -69,6 +69,7 @@ The most common defect in this repo is a change that works on the path you teste - **Entry points.** A behavior reachable from the chat view is usually also reachable from Settings, the command palette, and a keybinding. Fixing one is not fixing the feature. - **Clients.** Web, desktop (wraps web, adds Electron shell/IPC), and mobile (React Native, separate navigation). Shared logic lives in `packages/client-runtime` - **Providers.** Codex, Claude, Cursor, Grok, OpenCode, and Antigravity each have an adapter. Provider-shaped features need a decision per adapter, even if the decision is "not supported here". +- **Agents.** A capability a user can trigger is usually one an agent should reach through MCP tools, and scheduled tasks run the same paths. That only works when it is a service method, not handler code. - **Contracts.** Anything crossing the wire is typed in `packages/contracts`. Change the schema and the server, web, mobile, and desktop all follow. - **Reverse states.** If you added a way in, add the way out and the way to see it. Snooze needs unsnooze. Close needs reopen. A one-way door is a bug. - **Connection modes.** Local, remote/relay, and tunnel behave differently. Multi-device and multi-environment cases are real. @@ -148,7 +149,7 @@ Full glossary with file links: `docs/internals/glossary.md` ## Where code lives -- `apps/server` - WebSocket, orchestration, providers, checkpointing. Effect-heavy: read `.repos/effect-smol/LLMS.md` before writing Effect code. +- `apps/server` - WebSocket, orchestration, providers, checkpointing. Effect-heavy: read [Effect services](docs/internals/effect-services.md) before adding server code, and `.repos/effect-smol/LLMS.md` for the Effect library itself. - `apps/web` - React/Vite UI. `apps/desktop` wraps it, `apps/mobile` is React Native, `apps/marketing` is the site. - `packages/contracts` - Effect/Schema contracts plus small derived helpers. No heavy runtime logic. - `packages/shared` - shared runtime utils, subpath exports, no barrel. @@ -158,6 +159,7 @@ Full glossary with file links: `docs/internals/glossary.md` ## Taste - Complexity belongs at the adapter boundary. Orchestration stays pure, UI stays dumb. +- Server features are services; transports stay thin. A `ws.ts` RPC handler, HTTP route, or MCP tool decodes input, calls one service method, and maps errors. See [Effect services](docs/internals/effect-services.md). - `apps/web/src/components/ui` exports own their look. Pick a `variant` or `size`; do not restyle one with `className`. If none fits and the look is a generic concept, add a variant to the component; a look that belongs to one feature stays in that feature's own component, not in `components/ui`. Layout classes (width, flex, margin, position) belong on the parent. `shadcn/no-restyle` fails lint on violations. - Inferred types over annotations. `any` is the enemy. - Comments describe how a thing is used, and move when the code moves. To be used mostly to describe functions, not to annotate every line of behavior. diff --git a/docs/internals/effect-services.md b/docs/internals/effect-services.md new file mode 100644 index 000000000000..7ae4671d2e6a --- /dev/null +++ b/docs/internals/effect-services.md @@ -0,0 +1,84 @@ +# Effect services + +Server features are Effect services. Transports call them: WebSocket RPC handlers in +[`ws.ts`](../../apps/server/src/ws.ts), HTTP routes, MCP tools, scheduled tasks, and the CLI. This +page holds the rules for writing them. The +[Effect Service Conventions](../../.macroscope/check-run-agents/effect-service-conventions.md) +review check enforces the same rules; keep the two in step. + +## Where a feature lives + +A server capability is a method on a service in its domain folder (`project/`, `workspace/`, `git/`, +`provider/`, ...). Extend the service that already owns the domain; add a new one only when none +does. + +A transport handler does three things: decode the request, call one service method, and map the +service's typed errors to the transport's error. Nothing else. Filesystem, Git, or process work, +folder naming, multi-step dispatch, retries, and rollback belong in the service. + +The reason is reach. Users trigger a capability from the WebSocket, agents reach it through MCP +tools, and scheduled tasks and the CLI run it too. Logic written into one handler is missing from +the others, and testing it needs a socket. Plain functions for pure work (a slug, an SVG, a +message) are fine next to the service; the capability itself is the method. + +```ts +// ws.ts: a thin handler +[WS_METHODS.projectsCreateNew]: (input) => + observeRpcEffect( + WS_METHODS.projectsCreateNew, + projectFolders.createNamedProject(input).pipe( + Effect.mapError((cause) => new ProjectCreateNewError({ cause })), + ), + { "rpc.aggregate": "orchestration" }, + ), +``` + +## Shape of a service module + +One module per service, in this order: imports, errors and schemas, the `Context.Service` tag with +its interface inline, `make`, then `layer`. [`WorkspacePaths.ts`](../../apps/server/src/workspace/WorkspacePaths.ts) +and [`T3ProjectFileLoader.ts`](../../apps/server/src/project/T3ProjectFileLoader.ts) are good +references. + +```ts +export class FooWriteError extends Schema.TaggedError()("FooWriteError", { + path: Schema.String, + cause: Schema.Defect(), +}) { + override get message(): string { + return "Failed to write the foo file."; + } +} + +export class Foo extends Context.Service< + Foo, + { readonly write: (input: { readonly path: string }) => Effect.Effect } +>()("t3/area/Foo") {} + +const make = Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + // ... + return Foo.of({ write }); +}); + +export const layer = Layer.effect(Foo, make); +``` + +- **Imports.** Consumers use the module as a namespace: `import * as Foo from "./Foo.ts"`, then + `yield* Foo.Foo` and `Foo.layer`. Never `import { layer as fooLayer }`. +- **Dependencies** come from the environment (`yield* FileSystem.FileSystem`), never as parameters to + `make`. +- **`make`** stays private unless another module imports it. Knip fails CI on an unused export. +- **Errors** are `Schema.TaggedError` classes with structured attributes and a `cause` when they wrap + a failure. The message is fixed or built from attributes, never from `cause`. Construct the error + where the failure happens; map it to a transport error only in the transport. Catch known tags + with `Effect.catchTags`. +- **Tests** exercise behavior through the service, with test layers only for external dependencies. + Don't mock the logic under test. + +## Before you push + +- Does any handler you touched do more than decode, call, and map errors? +- Could an agent (MCP) or a scheduled task use this capability? If not, is that deliberate? +- Did you extend the domain's existing service before adding a new one? +- Did you run knip? A new export with no importer fails it.