mirror of
https://github.com/vector-im/element-call.git
synced 2026-09-13 21:59:30 +00:00
Add AGENTS.md documenting the repo's conventions (#4256)
* Add AGENTS.md documenting the repo's conventions Codifies what is currently tribal knowledge or only discoverable by reading CI config: the view model / view contract, the rule that nothing reads the page, Clean Code ordering, the three test layers, and reuse-before-you-build. The root AGENTS.md is the always-loaded minimum and routes to docs/agents/, so an agent loads only the detail its task needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Allow either hand at the commit, and measure continuous redraws Committing from the agent rules out running it where there is no git identity, a sandbox included. Either is now fine: commit on the user's word where the setup allows it, or write the message and hand it over. Redrawing continuously is the other addition. A level meter reported a fresh value every animation frame, so its whole menu reconciled sixty times a second, silence included, and nobody noticed until review. A PR that adds anything of the kind now says what it costs, or better, counts the redraws — commits rather than render calls, since React runs a component it then discards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
fkwp
@@ -10,6 +10,9 @@ public/config.json
|
||||
backend/synapse_tmp/*
|
||||
backend/synapse_tmp_othersite/*
|
||||
/coverage
|
||||
|
||||
# Transient agent scratch files
|
||||
/agent-workspace/
|
||||
config.json
|
||||
|
||||
# Yarn
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
# AGENTS.md — Element Call
|
||||
|
||||
MatrixRTC (MSC4143) + LiveKit video calling, shipped standalone, as a widget in
|
||||
Element Web and Element X, as embedded packages, and as a React component in a
|
||||
host's page. It is the MatrixRTC reference implementation.
|
||||
|
||||
## Non-negotiables
|
||||
|
||||
- Features and UI changes need a pre-approved issue. No issue, no review.
|
||||
- Exactly one `PR-*` label per PR. CI enforces it; it drives the changelog.
|
||||
- New behaviour ships with unit tests, a story if it renders, and an e2e spec if it
|
||||
is user-facing. All three.
|
||||
- Reuse an existing component. If you genuinely cannot, say so in the PR body and
|
||||
name what you rejected — never add a new one silently.
|
||||
- Nothing reads the page. No `window.location`, `document.body`, `window.inner*`,
|
||||
`@media`, global `i18next` or the `widget` global — take it from a provider.
|
||||
- The change works standalone, as a widget and as a component. Say what you checked.
|
||||
- Every gate below is green before you push.
|
||||
- Hand off after the first implementation, before the quality pass. Then commit on
|
||||
the user's word where the setup allows it, or hand them the message.
|
||||
|
||||
## Read before you
|
||||
|
||||
| … | … |
|
||||
| -------------------------------- | ---------------------------------------------------------- |
|
||||
| start a task of any size | [docs/agents/workflow.md](docs/agents/workflow.md) |
|
||||
| touch call logic or a view model | [docs/agents/architecture.md](docs/agents/architecture.md) |
|
||||
| write any code | [docs/agents/code-style.md](docs/agents/code-style.md) |
|
||||
| write a test, story or e2e spec | [docs/agents/testing.md](docs/agents/testing.md) |
|
||||
| open a PR | [CONTRIBUTING.md](CONTRIBUTING.md) |
|
||||
| implement from a feature spec | [FEATURES_SPEC/AGENTS.md](FEATURES_SPEC/AGENTS.md) |
|
||||
|
||||
## Gates
|
||||
|
||||
```sh
|
||||
pnpm lint # tsc, oxlint, knip, component externals
|
||||
pnpm format # oxfmt
|
||||
pnpm test # vitest: unit and storybook projects
|
||||
pnpm i18n:check
|
||||
```
|
||||
|
||||
`knip` fails on dead code; a file deliberately inert ahead of its consumer goes in
|
||||
`knip.ts` `ignoreFiles` with a reason.
|
||||
|
||||
## A good PR
|
||||
|
||||
- One slice, under ~400 changed lines, green on its own, linked to its issue.
|
||||
- View model + marble tests, thin view, a story per state, an e2e spec.
|
||||
- Any new shared component called out explicitly, with why nothing existing fit.
|
||||
- Template filled for real: what, why, before/after screenshots, repro steps.
|
||||
- Anything that redraws continuously — an animation frame, a stream — says what it
|
||||
costs, or better, counts the redraws in a test.
|
||||
- Branch `<handle>/<topic>`. Plain imperative commit subjects, no prefixes.
|
||||
- Once review starts, fix forward. Never force-push a regeneration over a review.
|
||||
@@ -9,3 +9,12 @@ This folder contains documentation for setup, usage, and development of Element
|
||||
- [MatrixRTC modes](./matrix_rtc_modes.md)
|
||||
- [Self-Hosting](./self_hosting.md)
|
||||
- [Developing with linked packages](./linking.md)
|
||||
|
||||
### Writing code for Element Call (agent-readable conventions)
|
||||
|
||||
Entry point: [AGENTS.md](../AGENTS.md).
|
||||
|
||||
- [Agent workflow](./agents/workflow.md)
|
||||
- [Architecture](./agents/architecture.md)
|
||||
- [Code style](./agents/code-style.md)
|
||||
- [Testing](./agents/testing.md)
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
# Architecture
|
||||
|
||||
## Logic lives in view models; components render
|
||||
|
||||
- Call logic lives in `src/state/`. A component that derives call state is
|
||||
misplaced logic.
|
||||
- Composing behaviors: a factory taking `(scope, ...deps$)`, returning named
|
||||
behaviors plus callbacks. Copy `src/state/LayoutSwitchViewModel.ts`. Prefer this
|
||||
for anything new.
|
||||
- Owning a resource: a class taking the scope in its constructor — `MediaDevices`,
|
||||
`MuteStates`, `TileStore`, `Publisher`, `Connection`. Both styles are current;
|
||||
don't convert one to the other in passing.
|
||||
- `foo$` is an observable. `Behavior<T>`, an observable with a current value, is
|
||||
the default for anything a view reads.
|
||||
|
||||
## The view model / view contract
|
||||
|
||||
- Components take `vm: ViewModel<Snapshot>` and read state through `useBehavior`.
|
||||
See `CallFooter`, `InCallView`, `LobbyView`, `SettingsModal`.
|
||||
- A snapshot is `Actions & State`; every field becomes a `foo$` behavior, and none
|
||||
is optional.
|
||||
- An unavailable action is `undefined`, not a separate `canDoThing` flag — the
|
||||
presence of the callback drives the rendering.
|
||||
- Subscribe in an effect only to drive a side effect off an event stream, as
|
||||
`ReactionAudioRenderer` does; never to read state a behavior already holds.
|
||||
|
||||
## Scopes own lifetimes
|
||||
|
||||
- `ObservableScope` bounds every subscription a view model creates.
|
||||
- Reference only the scope defined in the same function; one captured from an
|
||||
enclosing scope outlives its owner, and `no-observablescope-leak` rejects it.
|
||||
|
||||
## Nothing reads the page
|
||||
|
||||
Element Call can be mounted several times inside a host's React tree, so it owns
|
||||
neither window, URL, document nor router. Each seam defaults to the old standalone
|
||||
and widget behaviour.
|
||||
|
||||
| Never | Use |
|
||||
| --------------------------------------------- | ------------------------------------------------------------- |
|
||||
| the `widget` global | `useHostBridge()` — `src/HostBridge.ts` |
|
||||
| `getUrlParams()`, `window.location` | `useUrlParams()` |
|
||||
| `useNavigate("/")`, `<Link to="/">` | `useLeaveToHome()`, `LeaveToHomeLink` |
|
||||
| `document.body` | `useRootElement()` |
|
||||
| `window.innerWidth/Height`, `useMediaQuery` | `useRootSizeMatches()` in views, `windowSize$` in view models |
|
||||
| global `i18next` | the instance in `src/utils/i18n.ts` |
|
||||
| config or analytics reading their environment | `Config.initWith()`, `PosthogAnalytics.configure()` |
|
||||
|
||||
- View models take values as options, never `getUrlParams()`
|
||||
(`callViewModelOptionsFromParams`, `CallViewModelOptions.hostBridge`).
|
||||
- The host bridge is the only channel to the host: `createWidgetHostBridge(widget)`,
|
||||
`nullHostBridge` standalone, `useComponentHostBridge`. State a capability
|
||||
(`supportsReactions`, `supportsProfileChanges`); never infer it from being a
|
||||
widget.
|
||||
- Shortcuts and portals attach to the root element, so two instances don't fight.
|
||||
- Known debt, not precedent: `Grid` measures `window.innerHeight`; `ErrorView`'s
|
||||
reload and `getAbsoluteRoomUrl` use `window.location`; recaptcha appends to
|
||||
`document.body` on the standalone login path.
|
||||
|
||||
## Context differences are options, not checks
|
||||
|
||||
- Named options with per-intent defaults (`configurationForIntent`), never a
|
||||
runtime check for who is hosting.
|
||||
- `controlledAudioDevices`, not the platform, is what makes `MediaDevices` pick
|
||||
`AndroidControlledAudioOutput` / `IOSControlledAudioOutput` over the web
|
||||
`AudioOutput`. Intent presets set it on non-desktop; standalone leaves it off.
|
||||
- URL params are a published contract: change one, update `docs/url_params.md`.
|
||||
|
||||
## Build targets
|
||||
|
||||
Code that builds in only one is a bug.
|
||||
|
||||
- `build:full` — standalone app, also widget mode.
|
||||
- `build:embedded` — `@element-hq/element-call-embedded`.
|
||||
- `build:sdk` — SDK library, entry `sdk/main.ts`.
|
||||
- `build:component` — `@element-hq/element-call-component`, sources in `component/`
|
||||
(its own pnpm project; run pnpm from the repo root). Host API is in the README;
|
||||
`pnpm lint:externals` rejects an import of a `react` / `react-dom` /
|
||||
`matrix-js-sdk` / `livekit-client` subpath the externals list omits.
|
||||
@@ -0,0 +1,74 @@
|
||||
# Code style
|
||||
|
||||
_Clean Code_ (Robert C. Martin): small functions, intention-revealing names, low
|
||||
noise.
|
||||
|
||||
## Reuse before you build
|
||||
|
||||
Look before you write, in this order:
|
||||
|
||||
1. **Compound Web** — `@vector-im/compound-web` for buttons, tooltips, alerts,
|
||||
dialogs, menus, form controls, avatars; icons in
|
||||
`@vector-im/compound-design-tokens/assets/web/icons`.
|
||||
2. **This repo** — `src/button/`, `src/components/`, `src/input/`, `src/form/`,
|
||||
`src/tabs/`, and `Modal`, `Avatar`, `Toast`, `Slider`, `ErrorView`,
|
||||
`FullScreenView` at the root of `src/`.
|
||||
3. **The nearest existing feature** — grep for the same shape first. A hook is
|
||||
often what you want to extract, not a component.
|
||||
|
||||
If nothing fits, name the new shared component in the PR body and say what you
|
||||
rejected and why — never add one silently. Extend or parameterise before forking,
|
||||
and extract shared logic into a hook both callers use before copying it; if you
|
||||
copy, the PR says so.
|
||||
|
||||
## Ordering
|
||||
|
||||
- **Newspaper order.** Headline first, detail down. A file opens with what it
|
||||
exists to provide.
|
||||
- **Stepdown rule.** Caller above callee. A helper used by one function sits
|
||||
beneath it, which needs a `function` declaration, not a `const` arrow. Existing
|
||||
arrow components above their callers are not a pattern to extend — nor to churn.
|
||||
- **Suites.** Test cases first, helpers below. Older suites invert this; follow the
|
||||
rule in new ones, don't reorder old ones.
|
||||
- **Names.** What a thing means, not what it is made of: `naturalLayout$`, not
|
||||
`computedLayout$`. Comments explain why, never what.
|
||||
|
||||
## TypeScript, React and RxJS idiom win on a clash
|
||||
|
||||
- Hooks stay unconditional at the top of a component.
|
||||
- `useCallback` / `useMemo` dependency arrays sometimes force inlining.
|
||||
- An RxJS pipeline stays one expression. Name the behavior, not each operator.
|
||||
- A long factory is fine when it reads as a list of named behaviors.
|
||||
`CallViewModel.ts` is long because the domain is.
|
||||
- Marble test tables are dense on purpose.
|
||||
|
||||
## Enforced by lint
|
||||
|
||||
- Copyright header on every file: `Copyright <current year> Element Creations Ltd.`
|
||||
plus the AGPL / commercial SPDX line.
|
||||
- `logger` from `matrix-js-sdk/lib/logger`, never `console`, and no top-level
|
||||
`logger.getChild()`. The `console` ban is only enforced under `src/*/**`.
|
||||
- No floating or misused promises; async functions typed `Promise<T>`.
|
||||
- Inline type imports (`import { type Foo }`), so matrix-js-sdk stays lazily
|
||||
loadable.
|
||||
- Deep-import `matrix-js-sdk/lib/<module>` as the codebase does. Banned is the bare
|
||||
`matrix-js-sdk/lib`, `lib/index` and anything under `src/`.
|
||||
|
||||
## CSS
|
||||
|
||||
- Compound components and `--cpd-*` tokens in CSS modules. A hardcoded colour or px
|
||||
spacing is a design question, not a licence to inline a hex.
|
||||
- Size against the root, not the window: `@container element-call (…)` and
|
||||
`cqw` / `cqh`, never `@media (width)` or `vw` / `vh`. Media queries stay correct
|
||||
only in standalone-only views — home, login.
|
||||
- Style `[data-element-call-root]`, never `body` or `:root`; the component build
|
||||
makes those stand for the root (`component/build/scopeStylesToRoot.ts`).
|
||||
|
||||
## Strings and a11y
|
||||
|
||||
- Strings through `t()`: add the key, run `pnpm i18n`, fill `locales/en/app.json`.
|
||||
Other locales come from Localazy; never hand-edit them.
|
||||
- `t` from `useTranslation()`, or `src/utils/i18n.ts` outside React. Never the
|
||||
`i18next` global — several instances share a page.
|
||||
- Accessible names on controls, `aria-pressed` on toggles, keyboard reachability.
|
||||
`jsx-a11y` rules are errors; `vitest-axe` is available.
|
||||
@@ -0,0 +1,53 @@
|
||||
# Testing
|
||||
|
||||
Three layers; a user-facing feature needs all three. Unit proves the logic,
|
||||
Storybook proves the rendering, e2e proves the feature. Codecov gates 80% on the
|
||||
lines a PR touches.
|
||||
|
||||
## Unit
|
||||
|
||||
- View model tests use `withTestScheduler` + `testScope()` from `src/utils/test.ts`
|
||||
and read as ASCII timelines. Copy `LayoutSwitchViewModel.test.ts`.
|
||||
- Component tests use Testing Library, queried by role and accessible name.
|
||||
- Reuse the factories in `src/utils/test.ts` — `mockRemoteParticipant`,
|
||||
`mockMatrixRoom`, `mockLivekitRoom`, `MockRTCSession`. Hand-rolled mocks drift.
|
||||
- Snapshots live in `__snapshots__/`; update with `pnpm test <Name> -u`.
|
||||
- How often something redraws is testable: drive the frames and count commits, not
|
||||
render calls. An effect with no dependency array runs once per commit.
|
||||
- `component/**/*.test.ts` runs in the same jsdom project as `src`.
|
||||
|
||||
## Storybook
|
||||
|
||||
A deliverable, not documentation: `pnpm test:storybook` runs every story as a real
|
||||
browser test in the same CI job as the unit suite. **A UI change without a story is
|
||||
incomplete.** Only three stories exist so far, so copy `CallFooter.stories.tsx`.
|
||||
|
||||
- Drive the component from a snapshot via `useStaticViewModel`, never internals.
|
||||
- One named story per state that matters — loading, error, empty, denied, mobile.
|
||||
- Assert in a `play` function with `userEvent` / `expect` from `storybook/test`.
|
||||
- Mobile is `globals: { viewport: { value: "mobile2" } }`, not a second component.
|
||||
- Expose interesting props through `argTypes`.
|
||||
|
||||
`.storybook/preview.tsx` supplies `TooltipProvider`, `src/index.css` and English
|
||||
translations. The browser is Playwright's, so a fresh clone needs
|
||||
`pnpm playwright install` once.
|
||||
|
||||
## End-to-end
|
||||
|
||||
- `playwright/*.spec.ts` — standalone, on Chromium and Firefox.
|
||||
- `playwright/widget/` — widget mode, where cross-context bugs surface.
|
||||
- `playwright/component/` — the component in a host page, via the harness on port
|
||||
3001 that Playwright starts as a second web server. Catches container-relative
|
||||
layout, styles escaping the root, two instances on a page, host-bridge reports.
|
||||
- `playwright/mobile/` — Pixel 7, `mobile` project only.
|
||||
|
||||
Test what a user observes: the peer sees the change, it survives a reconnect, it is
|
||||
right after a reload. Reuse `playwright/spa-helpers.ts`,
|
||||
`playwright/widget/test-helpers.ts`, `playwright/fixtures/`.
|
||||
|
||||
```sh
|
||||
pnpm test # unit + storybook
|
||||
pnpm backend # Synapse + LiveKit, required for e2e
|
||||
pnpm test:playwright # or :open
|
||||
pnpm dev:component # component harness, port 3001
|
||||
```
|
||||
@@ -0,0 +1,50 @@
|
||||
# Agent workflow
|
||||
|
||||
## Scratch files live in `agent-workspace/`
|
||||
|
||||
Git-ignored. One kebab-case subfolder per task, matching the branch topic:
|
||||
`agent-workspace/<slug>/`.
|
||||
|
||||
| File | Purpose |
|
||||
| --------------------------- | -------------------------------------------------------------------------------- |
|
||||
| `plan.md` | The concrete approach — files, symbols, edit order — once direction is confirmed |
|
||||
| `commit-msg.txt` | `git commit -F agent-workspace/<slug>/commit-msg.txt` |
|
||||
| `pr-body.md` | `gh pr create --body-file agent-workspace/<slug>/pr-body.md` |
|
||||
| `implementation-summary.md` | What was built, decisions, trade-offs |
|
||||
| `NN-prompt.md` | Raw prompt text, numbered, when worth keeping |
|
||||
|
||||
- Nothing here is durable. The folder is ignored, so anything worth keeping is
|
||||
promoted into the PR body, a doc, or a feature spec before the task ends.
|
||||
- Never write scratch files to the repo root. Stray `load_test_summary.md` and
|
||||
`config.json_` files are what this folder prevents.
|
||||
- Start `pr-body.md` from `.github/PULL_REQUEST_TEMPLATE.md` and fill every
|
||||
section.
|
||||
- Commit subjects are plain imperative English. No conventional-commits prefixes.
|
||||
- A `plan.md` and a feature spec sit at different altitudes; neither replaces the
|
||||
other. A spec in `FEATURES_SPEC/` is durable and deliberately abstract —
|
||||
behaviour, decisions, acceptance criteria — so the feature can be rebuilt against
|
||||
a `main` nobody has seen yet. A plan is one slice of it landing on today's
|
||||
`main`: the paths, symbols and edit order the spec's decisions and criteria must
|
||||
not name. `FEATURES_SPEC/AGENTS.md` wins wherever the two genuinely overlap.
|
||||
|
||||
## Hand off before the quality pass
|
||||
|
||||
- Implement the change, run the narrowest check that rules out an obviously broken
|
||||
handoff, then stop and ask whether the direction is right.
|
||||
- Full `pnpm lint`, the whole suite, coverage and benchmarks come after the
|
||||
direction is confirmed.
|
||||
- Wider refactors, extra tests and documentation polish are follow-up work, not
|
||||
part of the first handoff.
|
||||
- If a check is needed before feedback, keep it to the touched code and say why.
|
||||
|
||||
## Commit and PR readiness
|
||||
|
||||
- Commit once the user confirms direction, or asks for one. Not before. Where the
|
||||
setup has no git identity, a sandbox included, write the message to
|
||||
`commit-msg.txt` and hand it over instead.
|
||||
- Ready means every gate in [AGENTS.md](../../AGENTS.md#gates) is green and the
|
||||
change is covered at the layers [testing.md](./testing.md) asks for. Read the diff
|
||||
against [code-style.md](./code-style.md) first.
|
||||
- Re-run the whole checklist after any fix. Only on green, either way.
|
||||
- Once a human has started reviewing, fix forward — never force-push a
|
||||
regeneration over a review in progress.
|
||||
Reference in New Issue
Block a user