authorgravatar for ethorelli@gmail.comErik Thorelli <ethorelli@gmail.com> 2026-10-05 19:28:02-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-05 19:28:02-07:00
log6262e5f50081845bb247e5af2fc81b068c8ae893
treecef8a843edd80a84d4373a603ef637b78aa68f1e
parentea54be7f853aca37981f60958d66039196f13a0c
signature Signed by PGP key B5690EEEBB952194

docs: put the Effect and web UI review rules in the docs (#16286)


3 files changed, 106 insertions(+), 12 deletions(-)

AGENTS.md+1-1
...@@ -151,7 +151,7 @@ Architecture and its constraints: `docs/internals/overview.md`. Glossary: `docs/...@@ -151,7 +151,7 @@ Architecture and its constraints: `docs/internals/overview.md`. Glossary: `docs/
151151
152- Complexity belongs at the adapter boundary. Orchestration stays pure, UI stays dumb.152- Complexity belongs at the adapter boundary. Orchestration stays pure, UI stays dumb.
153- 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).153- 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).
154- `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.154- `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. See [Web UI](docs/internals/web-ui.md).
155- Inferred types over annotations. `any` is the enemy.155- Inferred types over annotations. `any` is the enemy.
156- 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.156- 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.
157- Our users drive agents all day and notice a dropped frame, a lying spinner, and a stale label. No continuously repainting animations; they peg the GPU on high-refresh displays.157- Our users drive agents all day and notice a dropped frame, a lying spinner, and a stale label. No continuously repainting animations; they peg the GPU on high-refresh displays.
docs/internals/effect-services.md+66-11
...@@ -13,8 +13,8 @@ A server capability is a method on a service in its domain folder (`project/`, `...@@ -13,8 +13,8 @@ A server capability is a method on a service in its domain folder (`project/`, `
13does.13does.
1414
15A transport handler does three things: decode the request, call one service method, and map the15A transport handler does three things: decode the request, call one service method, and map the
16service's typed errors to the transport's error. Nothing else. Filesystem, Git, or process work,16service's typed errors to the transport's error. Nothing else. Filesystem, Git, process, or
17folder naming, multi-step dispatch, retries, and rollback belong in the service.17persistence work, folder naming, multi-step dispatch, retries, and rollback belong in the service.
1818
19The reason is reach. Users trigger a capability from the WebSocket, agents reach it through MCP19The reason is reach. Users trigger a capability from the WebSocket, agents reach it through MCP
20tools, and scheduled tasks and the CLI run it too. Logic written into one handler is missing from20tools, and scheduled tasks and the CLI run it too. Logic written into one handler is missing from
...@@ -64,21 +64,76 @@ const make = Effect.gen(function* () {...@@ -64,21 +64,76 @@ const make = Effect.gen(function* () {
64export const layer = Layer.effect(Foo, make);64export const layer = Layer.effect(Foo, make);
65```65```
6666
67- **Imports.** Consumers use the module as a namespace: `import * as Foo from "./Foo.ts"`, then67- **Imports.** Import Effect modules as namespaces from their subpaths:
68 `yield* Foo.Foo` and `Foo.layer`. Never `import { layer as fooLayer }`.68 `import * as Effect from "effect/Effect"`, never `import { Effect } from "effect"`. Consumers use
69- **Dependencies** come from the environment (`yield* FileSystem.FileSystem`), never as parameters to69 a service module the same way: `import * as Foo from "./Foo.ts"`, then `yield* Foo.Foo` and
70 `make`.70 `Foo.layer`. Never `import { layer as fooLayer }`. Named imports are fine for packages like
71- **`make`** stays private unless another module imports it. Knip fails CI on an unused export.71 `@t3tools/contracts` and for modules used only for a pure helper, error, schema, config value, or
72- **Errors** are `Schema.TaggedError` classes with structured attributes and a `cause` when they wrap72 type. A barrel exposes a whole service module as `export * as TokenStore from "./tokenStore.ts"`,
73 a failure. The message is fixed or built from attributes, never from `cause`. Construct the error73 not as renamed `make` and `layer` exports.
74 where the failure happens; map it to a transport error only in the transport. Catch known tags74- **Interface.** No standalone `FooShape`; name the type `Foo["Service"]`.
75 with `Effect.catchTags`.75- **Dependencies** come from the environment (`yield* FileSystem.FileSystem`), never as parameters
76 to `make`, so the types of `make` and `layer` show what they need. Never hide one in a module
77 global, a closure over a singleton, or a `Layer.succeed` that calls runtime-backed or imperative
78 APIs. Tests may pass service instances directly. Configuration, immutable values, and deliberate
79 callbacks are fine as parameters; they aren't services.
80- **`make`** exists when the module owns construction and stays private unless another module
81 imports it. Knip fails CI on an unused export. Don't write `make = Effect.succeed(...)` to force
82 `Layer.effect`; use the constructor that fits, like `Layer.succeed` or `Layer.sync`.
83- **Names.** A module named for its implementation uses plain `make` and `layer`
84 ([`NodePtyAdapter.ts`](../../apps/server/src/terminal/NodePtyAdapter.ts)). A port module that also
85 holds implementations names them, like `makeCloudflaredRelayClient` and `layerCloudflared`.
86- **Moves.** Moving a service deletes the old files and updates every consumer, including
87 orchestration, MCP, tests, and integration harnesses. No re-export shims.
76- **Tests** exercise behavior through the service, with test layers only for external dependencies.88- **Tests** exercise behavior through the service, with test layers only for external dependencies.
77 Don't mock the logic under test.89 Don't mock the logic under test.
7890
91## Runtime boundaries
92
93`ManagedRuntime.make`, `runPromise`, and `runPromiseExit` belong at application and framework
94boundaries: React, native callbacks, the CLI, HTTP adapters. Never in a domain service, repository,
95persistence code, or service constructor. A named adapter may bridge a service into a Promise API,
96but no Effect service depends on it.
97
98Compose a shared resource once in an application-owned layer and provide its context to integration
99runtimes. Don't create a managed or Atom runtime per feature to hand it out. When acquisition can
100fail and callers need a fallback, keep the failure typed: an error on the operation or an explicit
101optional-service layer. Don't route around the layer with an imperative runtime.
102
103## Errors
104
105- **Attributes.** Failures are `Schema.TaggedError` classes with structured attributes: the
106 operation or stage, the resource path or entity id, a normalized category or status. The message
107 is fixed or built from those attributes, never from `cause`, `cause.message`, or a stringified
108 defect. No `detail` field that copies `cause.message`.
109- **Cause.** An error that wraps a failure keeps the immediate underlying error as `cause`; make it
110 required when every construction wraps one. Validation and domain errors with nothing underneath
111 have none.
112- **Safe values.** Attributes and log annotations stay bounded: no raw payloads, command arguments
113 or output, signed URLs, credentials, query strings, or arbitrary defect text. The exact value
114 lives only in `cause`. Expose a category, length, count, or a URL's protocol and host instead.
115- **Translation.** Construct the error where the failure happens, and map it to a transport error
116 only in the transport. A translation boundary passes through domain errors already in the target
117 channel and wraps only unknown or lower-level failures. Map each failure where its context is
118 known; don't wrap a whole pipeline in one generic error.
119- **Discriminators.** Don't encode one distinction twice, as a specific tag and a single-value
120 `operation`, `reason`, `kind`, or `phase` literal. Split classes when the distinction drives
121 control flow or the user-facing message; a field that only helps diagnostics stays a field. A
122 message that reaches HTTP, RPC, persisted state, or the UI is behavior, and a refactor keeps it.
123- **Mappers.** Don't write a helper that only does `(...args) => new SomeError({ ...args })`. Keep
124 a mapper only when it normalizes, passes domain errors through, or adds context. A mapper that
125 belongs to the target error is a static factory on that class.
126- **Predicates** are exported directly as `export const isFoo = Schema.is(Foo)`, not a function
127 wrapping a private `Schema.is`.
128- **Catching.** Catch known tags with `Effect.catchTags({ ... })`, even for one tag, not `catchTag`
129 or `catchIf` with a schema predicate. `Effect.catch` is for handling the whole channel; `catchIf`
130 is for structural checks like a platform error code.
131
79## Before you push132## Before you push
80133
81- Does any handler you touched do more than decode, call, and map errors?134- 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?135- 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?136- 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.137- Did you run knip? A new export with no importer fails it.
138- Does every directive you added that disables a lint, type-checker, or LSP diagnostic say why, in
139 a `-- reason` suffix or a comment above it?
docs/internals/web-ui.md created+39
...@@ -0,0 +1,39 @@
1# Web UI
2
3## Primitives
4
5Controls come from [`components/ui`](../../apps/web/src/components/ui).
6[AGENTS.md](../../AGENTS.md#taste) says how to use and extend them.
7
8Don't style a raw `<button>`, `<input>`, `<select>`, or `<textarea>` to look like a control a
9primitive provides. Raw elements are right when the element isn't that control: semantic rows, tabs,
10resize handles, swatches, editor surfaces, anything whose behavior or geometry differs by design.
11
12## Styles
13
14Style with Tailwind classes in the component that renders the element.
15
16- Global CSS, in [`index.css`](../../apps/web/src/index.css) or any other stylesheet, is only for
17 what classes on the owning component can't express: generated Markdown or imperative DOM, custom
18 elements and shadow roots, animations, runtime theme variables, and browser or Electron
19 integration.
20- No `style={{ ... }}` or inline `<style>` for static values a class covers. Values computed at
21 runtime are fine.
22- Theme-only rules use `@variant dark` and `@variant light`, not `.dark` selectors.
23
24## Composition
25
26The composer banners are the reference.
27
28- [`ComposerBanner.tsx`](../../apps/web/src/components/chat/ComposerBanner.tsx) exports one object
29 of small slots. Slots own their classes, take `className`, and spread the rest of their props,
30 merging classes with `cn`. Variants and density are props on `Root`. A slot that needs a control
31 renders `Button`.
32- [`ComposerBannerStack.tsx`](../../apps/web/src/components/chat/ComposerBannerStack.tsx) composes
33 the slots and owns only stack behavior: priority, expand and collapse, dismiss transitions.
34- Consumers are a few lines of slots, and the slots carry the styling:
35 [`ComposerActivityStatus.tsx`](../../apps/web/src/components/chat/ComposerActivityStatus.tsx).
36
37Build new UI the same way. Compose or extend a component instead of copying its markup and classes.
38When a call site repeats a treatment another one already uses, make it a variant or a slot on the
39shared component. A new composer banner or notice uses the `ComposerBanner` slots.