| author | |
| committer | |
| log | 094fb230e90ffb0af303a318ca716fecefd66c80 |
| tree | 2d31ed7eebac31a5443751121ec1c73e882e2b92 |
| parent | 6ea01f8d24419019588f2be589681929d8e26b0c |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>3 files changed, 94 insertions(+), 3 deletions(-)
.macroscope/check-run-agents/effect-service-conventions.md+7-2| ... | @@ -22,7 +22,7 @@ showToolCalls: true | ... | @@ -22,7 +22,7 @@ showToolCalls: true |
| 22 | 22 | ||
| 23 | # Effect service review | 23 | # Effect service review |
| 24 | 24 | ||
| 25 | 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. | 25 | 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. |
| 26 | 26 | ||
| 27 | ## Imports and module namespaces | 27 | ## Imports and module namespaces |
| 28 | 28 | ||
| ... | @@ -31,11 +31,16 @@ Review changed TypeScript for the conventions below. They apply when a pull requ | ... | @@ -31,11 +31,16 @@ Review changed TypeScript for the conventions below. They apply when a pull requ |
| 31 | - 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`. | 31 | - 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`. |
| 32 | - When a barrel exposes a whole service module, prefer `export * as TokenStore from "./tokenStore.ts"` over individually renamed `make` and `layer` exports. | 32 | - When a barrel exposes a whole service module, prefer `export * as TokenStore from "./tokenStore.ts"` over individually renamed `make` and `layer` exports. |
| 33 | 33 | ||
| 34 | ## Placement | ||
| 35 | |||
| 36 | - 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. | ||
| 37 | - 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. | ||
| 38 | |||
| 34 | ## Service definition | 39 | ## Service definition |
| 35 | 40 | ||
| 36 | - One canonical module per service in this order: imports, error and schema declarations, the `Context.Service` tag with its interface inline, `make`, then `layer`. | 41 | - One canonical module per service in this order: imports, error and schema declarations, the `Context.Service` tag with its interface inline, `make`, then `layer`. |
| 37 | - Define the interface inline in `Context.Service`. Do not add a standalone `FooShape` interface; refer to the inferred type as `Foo["Service"]`. | 42 | - Define the interface inline in `Context.Service`. Do not add a standalone `FooShape` interface; refer to the inferred type as `Foo["Service"]`. |
| 38 | - 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. | 43 | - 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. |
| 39 | - 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)`. | 44 | - 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)`. |
| 40 | - 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. | 45 | - 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. |
| 41 | 46 |
AGENTS.md+3-1| ... | @@ -69,6 +69,7 @@ The most common defect in this repo is a change that works on the path you teste | ... | @@ -69,6 +69,7 @@ The most common defect in this repo is a change that works on the path you teste |
| 69 | - **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. | 69 | - **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. |
| 70 | - **Clients.** Web, desktop (wraps web, adds Electron shell/IPC), and mobile (React Native, separate navigation). Shared logic lives in `packages/client-runtime` | 70 | - **Clients.** Web, desktop (wraps web, adds Electron shell/IPC), and mobile (React Native, separate navigation). Shared logic lives in `packages/client-runtime` |
| 71 | - **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". | 71 | - **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". |
| 72 | - **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. | ||
| 72 | - **Contracts.** Anything crossing the wire is typed in `packages/contracts`. Change the schema and the server, web, mobile, and desktop all follow. | 73 | - **Contracts.** Anything crossing the wire is typed in `packages/contracts`. Change the schema and the server, web, mobile, and desktop all follow. |
| 73 | - **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. | 74 | - **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. |
| 74 | - **Connection modes.** Local, remote/relay, and tunnel behave differently. Multi-device and multi-environment cases are real. | 75 | - **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` | ... | @@ -148,7 +149,7 @@ Full glossary with file links: `docs/internals/glossary.md` |
| 148 | 149 | ||
| 149 | ## Where code lives | 150 | ## Where code lives |
| 150 | 151 | ||
| 151 | - `apps/server` - WebSocket, orchestration, providers, checkpointing. Effect-heavy: read `.repos/effect-smol/LLMS.md` before writing Effect code. | 152 | - `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. |
| 152 | - `apps/web` - React/Vite UI. `apps/desktop` wraps it, `apps/mobile` is React Native, `apps/marketing` is the site. | 153 | - `apps/web` - React/Vite UI. `apps/desktop` wraps it, `apps/mobile` is React Native, `apps/marketing` is the site. |
| 153 | - `packages/contracts` - Effect/Schema contracts plus small derived helpers. No heavy runtime logic. | 154 | - `packages/contracts` - Effect/Schema contracts plus small derived helpers. No heavy runtime logic. |
| 154 | - `packages/shared` - shared runtime utils, subpath exports, no barrel. | 155 | - `packages/shared` - shared runtime utils, subpath exports, no barrel. |
| ... | @@ -158,6 +159,7 @@ Full glossary with file links: `docs/internals/glossary.md` | ... | @@ -158,6 +159,7 @@ Full glossary with file links: `docs/internals/glossary.md` |
| 158 | ## Taste | 159 | ## Taste |
| 159 | 160 | ||
| 160 | - Complexity belongs at the adapter boundary. Orchestration stays pure, UI stays dumb. | 161 | - Complexity belongs at the adapter boundary. Orchestration stays pure, UI stays dumb. |
| 162 | - 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). | ||
| 161 | - `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. | 163 | - `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. |
| 162 | - Inferred types over annotations. `any` is the enemy. | 164 | - Inferred types over annotations. `any` is the enemy. |
| 163 | - 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. | 165 | - 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. |
docs/internals/effect-services.md created+84| ... | @@ -0,0 +1,84 @@ | ||
| 1 | # Effect services | ||
| 2 | |||
| 3 | Server features are Effect services. Transports call them: WebSocket RPC handlers in | ||
| 4 | [`ws.ts`](../../apps/server/src/ws.ts), HTTP routes, MCP tools, scheduled tasks, and the CLI. This | ||
| 5 | page holds the rules for writing them. The | ||
| 6 | [Effect Service Conventions](../../.macroscope/check-run-agents/effect-service-conventions.md) | ||
| 7 | review check enforces the same rules; keep the two in step. | ||
| 8 | |||
| 9 | ## Where a feature lives | ||
| 10 | |||
| 11 | A server capability is a method on a service in its domain folder (`project/`, `workspace/`, `git/`, | ||
| 12 | `provider/`, ...). Extend the service that already owns the domain; add a new one only when none | ||
| 13 | does. | ||
| 14 | |||
| 15 | A transport handler does three things: decode the request, call one service method, and map the | ||
| 16 | service's typed errors to the transport's error. Nothing else. Filesystem, Git, or process work, | ||
| 17 | folder naming, multi-step dispatch, retries, and rollback belong in the service. | ||
| 18 | |||
| 19 | The reason is reach. Users trigger a capability from the WebSocket, agents reach it through MCP | ||
| 20 | tools, and scheduled tasks and the CLI run it too. Logic written into one handler is missing from | ||
| 21 | the others, and testing it needs a socket. Plain functions for pure work (a slug, an SVG, a | ||
| 22 | message) are fine next to the service; the capability itself is the method. | ||
| 23 | |||
| 24 | ```ts | ||
| 25 | // ws.ts: a thin handler | ||
| 26 | [WS_METHODS.projectsCreateNew]: (input) => | ||
| 27 | observeRpcEffect( | ||
| 28 | WS_METHODS.projectsCreateNew, | ||
| 29 | projectFolders.createNamedProject(input).pipe( | ||
| 30 | Effect.mapError((cause) => new ProjectCreateNewError({ cause })), | ||
| 31 | ), | ||
| 32 | { "rpc.aggregate": "orchestration" }, | ||
| 33 | ), | ||
| 34 | ``` | ||
| 35 | |||
| 36 | ## Shape of a service module | ||
| 37 | |||
| 38 | One module per service, in this order: imports, errors and schemas, the `Context.Service` tag with | ||
| 39 | its interface inline, `make`, then `layer`. [`WorkspacePaths.ts`](../../apps/server/src/workspace/WorkspacePaths.ts) | ||
| 40 | and [`T3ProjectFileLoader.ts`](../../apps/server/src/project/T3ProjectFileLoader.ts) are good | ||
| 41 | references. | ||
| 42 | |||
| 43 | ```ts | ||
| 44 | export class FooWriteError extends Schema.TaggedError<FooWriteError>()("FooWriteError", { | ||
| 45 | path: Schema.String, | ||
| 46 | cause: Schema.Defect(), | ||
| 47 | }) { | ||
| 48 | override get message(): string { | ||
| 49 | return "Failed to write the foo file."; | ||
| 50 | } | ||
| 51 | } | ||
| 52 | |||
| 53 | export class Foo extends Context.Service< | ||
| 54 | Foo, | ||
| 55 | { readonly write: (input: { readonly path: string }) => Effect.Effect<void, FooWriteError> } | ||
| 56 | >()("t3/area/Foo") {} | ||
| 57 | |||
| 58 | const make = Effect.gen(function* () { | ||
| 59 | const fileSystem = yield* FileSystem.FileSystem; | ||
| 60 | // ... | ||
| 61 | return Foo.of({ write }); | ||
| 62 | }); | ||
| 63 | |||
| 64 | export const layer = Layer.effect(Foo, make); | ||
| 65 | ``` | ||
| 66 | |||
| 67 | - **Imports.** Consumers use the module as a namespace: `import * as Foo from "./Foo.ts"`, then | ||
| 68 | `yield* Foo.Foo` and `Foo.layer`. Never `import { layer as fooLayer }`. | ||
| 69 | - **Dependencies** come from the environment (`yield* FileSystem.FileSystem`), never as parameters to | ||
| 70 | `make`. | ||
| 71 | - **`make`** stays private unless another module imports it. Knip fails CI on an unused export. | ||
| 72 | - **Errors** are `Schema.TaggedError` classes with structured attributes and a `cause` when they wrap | ||
| 73 | a failure. The message is fixed or built from attributes, never from `cause`. Construct the error | ||
| 74 | where the failure happens; map it to a transport error only in the transport. Catch known tags | ||
| 75 | with `Effect.catchTags`. | ||
| 76 | - **Tests** exercise behavior through the service, with test layers only for external dependencies. | ||
| 77 | Don't mock the logic under test. | ||
| 78 | |||
| 79 | ## Before you push | ||
| 80 | |||
| 81 | - Does any handler you touched do more than decode, call, and map errors? | ||
| 82 | - Could an agent (MCP) or a scheduled task use this capability? If not, is that deliberate? | ||
| 83 | - Did you extend the domain's existing service before adding a new one? | ||
| 84 | - Did you run knip? A new export with no importer fails it. | ||