From d831d2b8dc32a81745e1fd59aa121399d2b13f7c Mon Sep 17 00:00:00 2001 From: Depo-dev Date: Sun, 30 Aug 2026 03:19:19 +0100 Subject: [PATCH 1/4] docs(contracts): document contract upgrade and versioning Adds contracts/docs/concepts-upgrades.md covering token_transfer's upgrade(env, new_wasm_hash) entrypoint end to end. - Authorization: the single admin.require_auth() gate, that the admin is fixed at initialize with no rotation path, and what a compromised admin key means (total, unrecoverable loss of control over the contract). - Procedure: build with the pinned release profile, upload to obtain the wasm hash, verify that hash locally with sha256sum and via an independent reproduction, invoke as the admin, then confirm by reading state back. - Storage compatibility: soroban-sdk encodes contracttype enums by variant-name symbol and structs as name-keyed sorted maps, so renames and type changes corrupt or orphan state while reordering is harmless. Tables split safe from unsafe changes on that basis, and note the rename of DataKey::Admin as the unrecoverable case. - Records that only token_transfer is upgradeable; group_treasury and proposals have no upgrade entrypoint, so changing them means a fresh deployment and repointing every consumer. - Notes the absence of any version() entrypoint or upgrade event: the wasm hash is the only on-chain version identifier. --- contracts/docs/concepts-upgrades.md | 337 ++++++++++++++++++++++++++++ 1 file changed, 337 insertions(+) create mode 100644 contracts/docs/concepts-upgrades.md diff --git a/contracts/docs/concepts-upgrades.md b/contracts/docs/concepts-upgrades.md new file mode 100644 index 0000000..fb42a06 --- /dev/null +++ b/contracts/docs/concepts-upgrades.md @@ -0,0 +1,337 @@ +# Contract upgrades & versioning + +How a deployed Soroban contract in this repo is replaced with new code, who is +allowed to do it, and which changes to the contract's storage layout are safe +across that swap. + +The whole mechanism is a single function, `upgrade`, on **one** contract: +`token_transfer`. Everything below is anchored to +[`contracts/contracts/token_transfer/src/lib.rs`](../contracts/token_transfer/src/lib.rs). + +--- + +## 1. Which contracts are upgradeable + +| Contract | Path | Upgradeable | Mechanism | +| ---------------- | ------------------------------------ | ----------- | -------------------------------------------------------------------------- | +| `token_transfer` | `contracts/contracts/token_transfer` | **Yes** | `upgrade(env, new_wasm_hash)` — admin-gated `update_current_contract_wasm` | +| `group_treasury` | `contracts/contracts/group_treasury` | **No** | No upgrade entrypoint exists | +| `proposals` | `contracts/contracts/proposals` | **No** | No upgrade entrypoint exists | + +`group_treasury` and `proposals` expose no function that calls +`env.deployer().update_current_contract_wasm(...)`. Their deployed wasm is +immutable for the life of the contract instance. Changing their behaviour means +**deploying a new contract instance** and repointing every consumer at the new +contract ID (see [§7](#7-what-to-do-for-a-non-upgradeable-contract)). + +`proposals` carries an admin address that its source comments describe as a hook +"a future upgrade can wire" for governance parameters. That is a note about +future work, not an upgrade path — there is no upgrade entrypoint on that +contract today. + +--- + +## 2. Authorization: who may call `upgrade` + +```rust +pub fn upgrade(env: Env, new_wasm_hash: BytesN<32>) { + let admin: Address = env + .storage() + .instance() + .get(&DataKey::Admin) + .expect("not initialized"); + admin.require_auth(); + env.deployer().update_current_contract_wasm(new_wasm_hash); +} +``` + +The gate is exactly one thing: `admin.require_auth()`, where `admin` is the +address written to `DataKey::Admin` by `initialize` and **never written again**. +`token_transfer` has no `set_admin`, no admin-transfer function, and no +multi-signer or timelock logic. The admin is fixed at initialization for the +lifetime of the contract instance. + +Consequences that follow directly from that: + +- **Any caller who can produce a valid signature for the admin address can + replace the contract's entire code.** There is no second approval, no delay, + and no on-chain veto. +- **The admin key cannot be rotated.** If the key is compromised there is no + in-contract remedy — the attacker and the legitimate operator have identical + authority, and the attacker can upgrade the contract to wasm that removes the + legitimate operator's access. +- **A compromised admin key is total loss of control over the contract.** The + attacker can upgrade to wasm that redirects every `transfer` to an address + they control, or that drains any allowance the contract holds. Existing + storage (`Admin`, `TokenContract`) survives the swap, so the new code starts + with the contract's full state. +- **`set_token_contract` shares the same key.** The same compromise lets an + attacker point the router at a malicious token contract without upgrading + anything. + +Because a compromised admin is unrecoverable, the admin address should be held to +the same standard as a treasury key — hardware-backed, or a Stellar multisig +account so that no single signer can act alone. This is an operational control; +the contract itself enforces nothing beyond a single signature. + +### Verified by tests + +[`contracts/contracts/token_transfer/src/test.rs`](../contracts/token_transfer/src/test.rs) +covers both sides of the gate: + +- `test_upgrade_requires_admin_auth` — with the admin's auth mocked, the call + passes the auth check and fails later at the wasm lookup, proving admin auth + is _sufficient_. +- `test_upgrade_non_admin_panics` — with only a non-admin address authorized, + the call panics at `require_auth()`, proving admin auth is _necessary_. + +--- + +## 3. End-to-end upgrade procedure + +All commands run from `contracts/` and use the Stellar CLI, matching +[the deployment guide](./api-deployment-invocation.md). Replace `testnet` with +your target network. + +### 3.1 Build the new wasm + +```bash +cargo build -p token_transfer --target wasm32-unknown-unknown --release +``` + +Output: `target/wasm32-unknown-unknown/release/token_transfer.wasm`. + +Build with the workspace's release profile as committed — `contracts/Cargo.toml` +pins `opt-level = "z"`, `lto = true`, and `strip = "symbols"`. Changing profile +flags changes the binary and therefore its hash, so a hash produced under a +different profile will not match one anybody else builds. + +Run the same gates CI runs before uploading anything: + +```bash +cargo test -p token_transfer +cargo fmt --all -- --check +cargo clippy --workspace --target wasm32-unknown-unknown -- -D warnings -A dead_code -A clippy::too-many-arguments +``` + +### 3.2 Produce the wasm hash + +Uploading the binary installs the code on the ledger without deploying an +instance, and returns its hash — this is the value `upgrade` takes: + +```bash +stellar contract upload \ + --wasm target/wasm32-unknown-unknown/release/token_transfer.wasm \ + --source \ + --network testnet +# → 6ddb28e0980f643bb97350f7e3bacb0ff1fe74d846c6d4f2c625e766210fbb5b +``` + +The 32-byte value it prints is the `BytesN<32>` that `upgrade` expects. + +### 3.3 Verify the hash before invoking + +The hash is the _only_ thing binding the on-chain code to the source that was +reviewed. Verify it locally rather than trusting the upload output: + +```bash +# The uploaded hash is the sha256 of the wasm file bytes. +sha256sum target/wasm32-unknown-unknown/release/token_transfer.wasm +``` + +That value must equal the hash printed by `stellar contract upload`. If they +differ, the binary that was uploaded is not the one on disk — stop. + +A reviewer should independently reproduce the build from the reviewed commit and +confirm they get the same hash. Because the release profile is pinned in +`Cargo.toml` and the toolchain is pinned in +[`contracts/rust-toolchain.toml`](../rust-toolchain.toml), an independent build +of the same source should reproduce it. A mismatch means the toolchain or the +source differs, and the upgrade should not proceed until that is explained. + +### 3.4 Invoke `upgrade` + +```bash +stellar contract invoke \ + --id \ + --source \ + --network testnet \ + -- upgrade --new_wasm_hash +``` + +`--source` must be the admin identity — any other signer panics at +`require_auth()`. + +### 3.5 Confirm the upgrade took effect + +The contract emits no upgrade event, so confirmation is by observation: + +```bash +# 1. State survived the swap — this must still return the configured token. +stellar contract invoke --id --source --network testnet \ + -- token_contract + +# 2. New behaviour is live — call a function whose behaviour changed, +# or one that only exists in the new wasm. +stellar contract invoke --id --source --network testnet \ + -- +``` + +A read of `token_contract` that panics with `not initialized` after an upgrade +means the new wasm is reading a different storage key than the old wasm wrote — +see [§4](#4-storage-layout-compatibility). Treat that as a live incident: the +contract still holds the old data, but the new code cannot see it. + +The contract ID does **not** change across an upgrade. Backend and frontend +configuration (`TOKEN_TRANSFER_CONTRACT_ID`, the corresponding `NEXT_PUBLIC_*`) +needs no update. Clients holding a generated contract client **do** need +regenerating if function signatures changed. + +--- + +## 4. Storage layout compatibility + +`update_current_contract_wasm` swaps the code and **leaves storage untouched**. +The new wasm inherits every key the old wasm wrote, byte for byte. Nothing +migrates automatically and nothing is validated — a mismatch is silent until a +read fails or, worse, succeeds with the wrong data. + +`token_transfer` stores two instance-scoped keys, defined in +[`storage.rs`](../contracts/token_transfer/src/storage.rs): + +```rust +#[contracttype] +pub enum DataKey { + TokenContract, // Address of the SEP-41 token this contract routes through + Admin, // Address permitted to upgrade / set the token contract +} +``` + +Both are read via `env.storage().instance()`. There is no persistent or +temporary storage, so TTL and rent do not enter into the compatibility question +here. + +### 4.0 How the key encoding actually works + +The compatibility rules below follow from one fact about `#[contracttype]`, and +it is worth stating plainly because the intuition from other chains is wrong +here: **Soroban encodes by name, not by position.** + +- A `#[contracttype]` **enum** encodes each variant as an `ScSymbol` of the + variant's _identifier_ — `DataKey::Admin` is stored under the symbol + `"Admin"`. Declaration order plays no part in the encoding. +- A `#[contracttype]` **struct** encodes as an `ScMap` keyed by the field names, + sorted by name. Declaration order plays no part there either. + +So the dangerous edits are the ones that change a **name** or a **type**, not the +ones that change an order. (`#[contracterror]` / `contracttype` _integer_ enums +are the exception — those do encode by discriminant — but `token_transfer` uses +neither.) + +### 4.1 Safe changes + +| Change | Why it is safe | +| -------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Adding a **new** `DataKey` variant | Existing variants keep their own name symbols, so old keys still resolve. The new key simply has no value yet — reads must handle absence. | +| **Reordering** existing `DataKey` variants | Variants encode by name, so order is not part of the key. Harmless, though a pointless diff. | +| **Reordering** fields in a stored struct | Struct fields encode as a name-keyed map sorted by name, so declaration order is not part of the encoding. | +| Adding, removing, or changing contract **functions** | Functions are not storage. Callers of a removed function break, but stored state is unaffected. | +| Changing function **bodies** (logic, validation, events emitted) | No storage encoding is involved. | +| Adding a field to an **event** struct such as `TransferEvent` | Events are emitted, never read back from storage. Off-chain consumers must tolerate the new shape, but no on-ledger state is corrupted. | +| Adding a field to a **stored** struct, if reads tolerate its absence | Old values decode without the new key. The decode fails unless the field is optional or the read path handles the missing key, so this needs a deliberate migration path — see [§4.3](#43-when-a-layout-change-is-unavoidable). | +| Changing the release profile or the SDK **patch** version | Produces a different wasm hash, not a different storage layout. | + +### 4.2 Unsafe changes — these corrupt or orphan existing state + +| Change | What breaks | +| ----------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Renaming** a `DataKey` variant | The key is the variant name. Renaming `Admin` to `Owner` makes the new wasm read a key nothing was ever stored under: `upgrade` and `set_token_contract` panic with `not initialized`, and the old admin value is orphaned on the ledger permanently. | +| **Removing** a variant that still has a value on the ledger | The value stays on-ledger, unreachable and unrecoverable, and continues to occupy the instance entry. | +| Changing a stored value's **type** (`Address` → `BytesN<32>`, `i128` → `u64`) | The old bytes are decoded as the new type. This either panics on read or, where the encodings happen to overlap, silently yields a wrong value. | +| Changing an enum variant's **payload** (unit → tuple, or its inner types) | `DataKey::Admin` and `DataKey::Admin(u32)` are different keys with different encodings. The old value becomes unreachable. | +| **Renaming** or **retyping** a field in a stored struct | Same as the enum: the map key is the field name. The old field's value is orphaned and the new field reads as missing. `token_transfer` stores no structs today, so this is a rule to preserve rather than a present hazard. | +| Moving a key between **storage scopes** (instance → persistent/temporary) | The scopes are separate namespaces. The old value stays in instance storage and the new read finds nothing. | +| Re-running `initialize` after an upgrade | It panics — `initialize` guards on `DataKey::Admin` already existing. There is no re-initialization path, by design. | + +The worst case in that table is the rename, because it fails _closed_ in the most +expensive way possible: `Admin` renamed means nobody can authorize an `upgrade` +any more, so the mistake cannot be fixed by upgrading again. That is +unrecoverable for `token_transfer`. Treat any rename of a `DataKey` variant as a +change that must not ship. + +### 4.3 When a layout change is unavoidable + +There is no migration hook on this contract: no post-upgrade initializer, no +stored schema version, and no way to run code once at the moment of the swap. If +the layout must change, the migration has to be written into the new wasm itself +— an admin-only function invoked in the same operational window as the upgrade: + +1. Upgrade to a wasm that understands **both** layouts and exposes a `migrate` + function. +2. Invoke `migrate` as the admin. It reads the old keys, writes the new ones, and + records that it ran so it cannot run twice. +3. Optionally upgrade again to a wasm that has dropped the old-layout code. + +Rehearse the whole sequence on testnet, against a contract instance holding +representative state, before touching mainnet. + +--- + +## 5. Versioning + +The contract exposes no version. There is no `version()` function, no stored +version key, and no event announcing an upgrade — an observer cannot ask a +deployed `token_transfer` which build it is running. + +In practice the deployed version is identified by the **wasm hash**, which is +what the ledger records and what `stellar contract upload` returns. Keep an +operational record mapping each upgrade to the git commit it was built from, the +hash that build produced, the network, and the date. The chain does not keep that +mapping for you. + +Adding a `version()` entrypoint returning a compile-time constant is a safe +change under [§4.1](#41-safe-changes); storing the version under a new `DataKey` +variant is safe too — it is a new name, so nothing existing is disturbed. + +--- + +## 6. Pre-upgrade checklist + +- [ ] Change is reviewed and merged; the build is from a known commit. +- [ ] `cargo test -p token_transfer`, `cargo fmt --check`, and `cargo clippy` pass. +- [ ] Storage diff reviewed against [§4.2](#42-unsafe-changes--these-corrupt-or-orphan-existing-state) — no rename, removal, payload change, or type change of any `DataKey` variant. +- [ ] Local `sha256sum` matches the hash returned by `stellar contract upload`. +- [ ] A reviewer independently reproduced the build and got the same hash. +- [ ] Full sequence rehearsed on testnet against an initialized contract holding state. +- [ ] Admin key access confirmed and the signer identified before the invoke. +- [ ] Post-upgrade reads (`token_contract`, `balance`) planned as the confirmation step. + +--- + +## 7. What to do for a non-upgradeable contract + +`group_treasury` and `proposals` cannot be swapped in place. Changing them means: + +1. Deploy a new instance from the new wasm — `contracts/scripts/deploy_group_treasury.sh` + or `contracts/scripts/deploy_proposals.sh`. +2. Initialize it and re-establish its state. Members, thresholds, and balances do + not carry across. Funds held by a `group_treasury` instance must be withdrawn + through the old contract's own rules before the old instance is abandoned. +3. Update `GROUP_TREASURY_CONTRACT_ID` / `PROPOSALS_CONTRACT_ID` in the backend + env and the corresponding `NEXT_PUBLIC_*` values in the frontend, per + [§6 of the deployment guide](./api-deployment-invocation.md#6-how-a-deployed-contract-id-reaches-the-frontend--backend). +4. Accept that in-flight proposals on the old instance are stranded — they can + still be voted on and executed against the old contract, which no client is + pointed at any more. + +Because there is no in-place path, treat a `group_treasury` or `proposals` +deployment as final and get the logic right before mainnet. + +--- + +## 8. Related documents + +- [Contract build, deployment & invocation guide](./api-deployment-invocation.md) — toolchain, build, deploy, and the invoke syntax used above +- [Token transfer storage layout & token interface](./contracts-token-transfer-storage.md) — the storage keys these compatibility rules apply to +- [Token transfer API](./api-token-transfer.md) — the function surface an upgrade may change +- [Token transfer flow](./concepts-token-transfer-flow.md) — how the contract is used end to end From 46cb98f2396444257da8fc8394ae381f2ed0c7c1 Mon Sep 17 00:00:00 2001 From: Depo-dev Date: Sun, 30 Aug 2026 03:22:48 +0100 Subject: [PATCH 2/4] docs: add API versioning and deprecation policy Adds docs/api-versioning.md. States the current position plainly: REST routes are mounted unprefixed in app.ts, socket events are addressed by bare name with no version in the payload or handshake, and no deprecation machinery exists anywhere in apps/backend/src. Records the three breaking changes that already shipped without one - sequenceNumber dropped from GET /sync envelopes, device made required on POST /auth/verify, and JWT-only registration retired at POST /devices. Defines what breaks for REST responses, socket payloads, and database-backed shapes, including the case where a migration changes a response without any route being edited, and the asymmetry that strict request schemas make an added optional field breaking in the other direction. Gives the deprecation procedure: announce with a sunset date, mark per surface, ship the replacement first, hold the window, then retire behind an explicit error naming the replacement. The retired POST /devices handler, which still answers 403 pointing at the link-challenge flow, is the precedent the procedure generalises. Sets a 90-day minimum window, derived from the 7-day envelope retention floor plus stale tabs and cached service workers. Documents device capability negotiation as the mechanism that already ships protocol changes without breaking older clients, and names the properties that make it work: a universal sealed_box baseline, absence normalising to that baseline rather than erroring, unknown values preserved and ignored, and pairwise per-message selection. --- docs/api-versioning.md | 347 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 347 insertions(+) create mode 100644 docs/api-versioning.md diff --git a/docs/api-versioning.md b/docs/api-versioning.md new file mode 100644 index 0000000..b9232de --- /dev/null +++ b/docs/api-versioning.md @@ -0,0 +1,347 @@ +# API versioning & deprecation policy + +The backend's REST routes and socket events are consumed by a web client that +ships on its own schedule. Once a user has a tab open, or a service worker +cached, the client talking to production is not necessarily the client that was +built alongside the running server. This document states where the API stands on +compatibility today, what counts as a breaking change, and the procedure for +retiring a field, route, or event. + +--- + +## 1. Current position + +**REST routes are unversioned.** There is no `/v1` prefix, no `Accept` header +negotiation, and no version query parameter. Every router in +[`apps/backend/src/app.ts`](../apps/backend/src/app.ts) mounts at a bare path: + +```ts +app.use('/auth', authRouter); +app.use('/conversations', conversationsRouter); +app.use('/devices', devicesRouter); +app.use('/messages', messagesRouter); +app.use('/users', usersRouter); +app.use('/treasury', treasuryRouter); +app.use('/uploads', uploadsRouter); +app.use('/files', filesRouter); +app.use('/push', pushRouter); +app.use('/sync', syncRouter); +app.use('/user-devices', userDevicesRouter); +app.use('/security', securityRouter); +``` + +**Socket events are unversioned too.** Events are addressed by bare name +(`send_message`, `new_message`, `read_receipt`, …) with no version field in the +payload and none in the handshake. The `dispatch` envelope carries `type`, +`payload`, and `eventId` — no schema version. + +**There is no deprecation machinery.** The string `deprecat` does not appear +anywhere in `apps/backend/src`. No route sets a `Deprecation` or `Sunset` header, +no response carries a warning field, and no sunset dates are recorded anywhere. + +**Breaking changes have already shipped without a policy.** Three that are +visible in the current tree and its history: + +| Change | What broke for an older client | +| ------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `sequenceNumber` removed from `GET /sync` envelopes | A client ordering an offline catch-up by `sequenceNumber` gets `undefined` on every envelope. `apps/backend/src/__tests__/sync.routes.test.ts` asserts the field's absence, so the removal is deliberate and enforced. The field still exists on socket receipt payloads, so it is gone from one surface and present on another. | +| `device` became **required** on `POST /auth/verify` | `VerifySchema` moved from `device: DeviceSchema.partial().optional()` to `device: DeviceSchema`. A client that authenticated without a device block now gets a 400 at login — the hardest possible failure, because it locks the user out entirely rather than degrading a feature. | +| `POST /devices` retired in favour of the link-challenge flow | JWT-only device registration is gone; devices must complete `POST /devices/link/challenge` then `POST /devices/link/verify`. | + +The third of those is the one that was handled well, and it is the model this +policy generalises — see [§4.1](#41-the-existing-precedent-post-devices). + +Note also that `VerifySchema` and the message schemas are `.strict()`: an +unrecognised field in a request body is rejected outright rather than ignored. +That is a deliberate safety property for key material (a client bug that sends +private state should be surfaced, not silently dropped) but it means **adding an +optional field to a strict request body is a breaking change for any client that +sends it to an older server**. Request bodies and response bodies do not follow +the same compatibility rules here. + +### The intended policy going forward + +1. **Additive-by-default.** New behaviour ships as new fields, new events, or new + routes. Existing shapes keep working. +2. **No silent removals.** Anything removed goes through the deprecation + procedure in [§4](#4-deprecation-procedure), with an announced sunset. +3. **Negotiate rather than version, where the change is in the protocol.** The + device capability mechanism ([§5](#5-capability-negotiation-the-working-precedent)) + already ships protocol changes without breaking older clients, and it is the + preferred tool. +4. **Version the path only as a last resort.** Introducing `/v2/` beside + the existing route is reserved for a change that cannot be made additive or + negotiated. When it happens, the unprefixed route remains the v1 route — it is + not retroactively moved — and it is deprecated on the normal schedule. + +The policy applies to what the server promises going forward. It does not +retroactively re-open the three changes above. + +--- + +## 2. What counts as a breaking change + +### 2.1 REST responses + +Breaking: + +- Removing a field, or making a previously always-present field optional. +- Renaming a field. (This is a removal plus an addition, and it is the most + common accidental break.) +- Changing a field's type or its representation — `number` → `string`, an ISO + timestamp → epoch millis, a bare id → an object. +- Narrowing an enum-like value set, or adding a new value to one that clients + switch on exhaustively. +- Changing a success status code (`200` → `204`), or turning a previously + successful request into an error. +- Changing pagination semantics — cursor format, page size limits, ordering. + `GET /sync` encodes its cursor as `:`; a client persists that + string across sessions, so changing the format strands every stored cursor. +- Moving a route to a different path, or adding a newly required parameter. +- Tightening validation on an existing field so previously accepted input is + rejected. + +Not breaking: + +- Adding a new optional field to a response. +- Adding a new route. +- Adding a new optional field to a **non-strict** request body. +- Making a previously required request field optional. +- Loosening validation. +- Changing an error _message_ string while the status code and error shape hold. + +### 2.2 Socket event payloads + +The same field-level rules apply, plus: + +Breaking: + +- Removing an outbound event, or renaming one. A client listening for the old + name simply never fires — there is no error, so this fails silently and is + worse than a REST break. +- Removing a field from an outbound payload, or changing its type. +- Requiring a new field on an inbound event. +- Changing which room or audience an event is broadcast to, so a client that + used to receive it no longer does. +- Changing the ack/response contract of an inbound event. +- Changing the handshake requirements. Auth already rejects tokens without + `deviceId` — `verifyToken` throws `Token missing deviceId — re-authentication +required` — which is a hard break for any pre-device-auth token. + +Not breaking: + +- Adding a new event. +- Adding an optional field to an outbound payload. +- Accepting a new optional field on an inbound event. + +Socket breaks deserve extra caution for two reasons: a long-lived connection +means an old client can be attached to a new server for hours, and a missing +event produces no error the client can detect or report. + +### 2.3 Database-backed shapes + +Some response shapes are the database schema in a thin wrapper, so a migration +can break the API without anybody editing a route. Treat a migration as an API +change whenever the column is reachable from a response: + +- Dropping or renaming a column that a route serialises is a **response-breaking + change**, even though the diff touches only `apps/backend/drizzle/`. +- Changing a column's type or nullability changes the response's type or + optionality. +- Changing a default changes what clients observe for existing rows. +- Removing an enum value from a column constraint narrows a value set clients may + already be switching on. + +Two rules follow: + +1. **Expand, migrate, contract.** Add the new column, dual-write, migrate readers, + and only then drop the old column — with the drop treated as a deprecation + under [§4](#4-deprecation-procedure) rather than as a schema tidy-up. +2. **Never serialise a table row directly.** Route handlers should map explicitly + to a response shape, so that a column rename is a compile error rather than a + silent contract change. `apps/backend/src/lib/messages.ts` already does this + deliberately for `content`, destructuring it out so a legacy plaintext column + can never reach a serialised response. + +--- + +## 3. Compatibility windows + +| Consumer | Assume it can be stale for | +| ---------------------------- | ------------------------------------------------------------------------- | +| Open browser tab | Hours to days — until reload | +| Cached service worker | Until the next activation | +| Device with queued envelopes | The envelope retention window, `ENVELOPE_TTL_SECONDS` (7 days by default) | + +The retention window is a floor, not a ceiling: a device offline for the whole +window reconnects, syncs, and immediately starts speaking whatever protocol it +knew a week ago. **Minimum support window for a deprecated field, event, or +route: 90 days** from the announcement in [§4.2](#42-procedure). Shorten it only +for a security fix, and say so explicitly when you do. + +--- + +## 4. Deprecation procedure + +### 4.1 The existing precedent: `POST /devices` + +When JWT-only device registration was retired, the route was not deleted. It was +left mounted, returning an explicit, actionable error: + +```ts +// ─── POST /devices — retired (#333) ────────────────────────────────────────── +// Kept only to return an explicit, actionable error: bare JWT-only device +// registration is gone. Clients must complete the link challenge instead. + +devicesRouter.post('/', (_req: AuthRequest, res) => { + res.status(403).json({ + error: + 'Device registration requires a fresh wallet signature. Use POST /devices/link/challenge then POST /devices/link/verify.', + }); +}); +``` + +An old client hitting it gets a status it can branch on and a message naming its +replacement, instead of a 404 that is indistinguishable from a typo or an outage. +That is the standard for every retirement: **the removed thing keeps answering, +and its answer says what to do instead.** + +### 4.2 Procedure + +1. **Announce.** Open an issue describing the change, the reason, the + replacement, and the sunset date (≥ 90 days out). Record it in the + [deprecation register](#6-deprecation-register) in this file. +2. **Mark it.** How depends on what is being deprecated: + + | Surface | How it is marked | + | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------- | + | REST route | Respond with `Deprecation: true` and `Sunset: ` headers, plus a `Link: ; rel="deprecation"` pointing at the issue. Keep serving normally. | + | REST field | Keep returning it, populated. Document it as deprecated in the route's doc under `apps/backend/docs/` and note the replacement field. | + | Socket event | Keep emitting it alongside its replacement. There is no header channel on a socket, so the register in this file and the event catalog are the notice. | + | Socket field | Keep populating it alongside the replacement field in the same payload. | + | DB column | Stop reading it, keep writing it, and mark it deprecated in the schema. | + +3. **Ship the replacement first.** The new field, event, or route must be live and + documented before the old one is marked, so a client can migrate the moment it + sees the notice. +4. **Notify.** Update the relevant doc under `apps/backend/docs/` (the + [WebSocket event catalog](../apps/backend/docs/api-websocket-events.md) for + events, the matching `api-*.md` for routes), add the register entry here, and + flag it in the PR description. There is no runtime client-notification channel + today; adding a deprecation array to `GET /health` would be the natural place + if one is wanted. +5. **Wait out the window.** Both paths work for the full period. +6. **Retire.** Replace the implementation with an explicit error in the style of + §4.1 — a `410 Gone` for a removed route, `403` where a different flow must be + used — naming the replacement. Do not delete the handler. Move the register + entry to _Retired_. + +### 4.3 Emergency changes + +A security fix may skip the window. It must still: ship an explicit error rather +than a silent removal, get a register entry recording that the window was +skipped and why, and state what a stranded client should do. `verifyToken` +rejecting tokens without `deviceId` is an example of this class — the break is +deliberate and the error message tells the client to re-authenticate. + +--- + +## 5. Capability negotiation: the working precedent + +For protocol-level changes, the codebase already has a mechanism that ships +breaking protocol work without breaking older clients: +[`apps/backend/src/lib/capabilities.ts`](../apps/backend/src/lib/capabilities.ts). + +Each device advertises a small JSON document at registration, and can update it +later without re-registering its identity key: + +```ts +protocols; // e.g. ["sealed_box", "signal", "mls"] — schemes this device can decrypt +ciphersuites; // MLS/Signal ciphersuite identifiers, meaningful when "mls" is present +fileTransfer; // file-encryption scheme versions, e.g. ["file-v1"] +``` + +The properties that make it work are worth naming, because they are what any +future negotiation mechanism should copy: + +- **A universal baseline.** `sealed_box` is the scheme every device in the + codebase implements, and it is the floor of every negotiation. +- **Absence is a valid answer.** A device that never advertised capabilities — + including rows written before the column existed — normalises to the + sealed_box-only baseline instead of erroring. `normalizeCapabilities` returns + the baseline for `null`, `undefined`, and malformed input alike. +- **Unknown values are preserved and ignored.** `selectProtocol` walks a known + priority list rather than rejecting names it does not recognise, so an older + server meeting a newer client's advertisement degrades instead of failing. +- **Negotiation is pairwise and per-message.** `selectProtocol(a, b)` picks the + strongest scheme _both_ sides support, so a rollout proceeds device by device + with no flag day. +- **Advertising is a non-breaking addition.** `capabilities` is optional on + `DeviceSchema`; omitting it is well-defined. + +The same shape applies beyond encryption. `fileTransfer` already carries versioned +scheme identifiers (`file-v1`) checked by `supportsFileTransfer`, which is exactly +how a versioned payload format should be gated. + +**Use this first.** A protocol or payload-format change that can be expressed as +a capability should be, rather than as a versioned route: it needs no client +coordination, no sunset date, and no dual-serving window. + +Its limit is that it is scoped to devices and to encryption/file-transfer +concerns. It says nothing about REST response shapes, and there is no equivalent +negotiation for a client's understanding of a JSON field. Those still need the +deprecation procedure in [§4](#4-deprecation-procedure). + +A second, narrower precedent lives in the socket layer: the +[envelope-wrapper vs. legacy-raw-emit](../apps/backend/docs/api-websocket-events.md#envelope-wrapper-vs-legacy-raw-emit) +split, where both emission styles are supported concurrently and new client code +is directed at the envelope. That is the dual-serving half of §4.2 already in +practice — what it lacks is an announced sunset for the legacy style. + +--- + +## 6. Deprecation register + +The canonical list of what is deprecated, what replaces it, and when it goes. +Every deprecation adds a row here as step 1 of [§4.2](#42-procedure). + +### Active deprecations + +| Surface | Deprecated | Replacement | Announced | Sunset | +| --------------- | ---------- | ----------- | --------- | ------ | +| _none recorded_ | | | | | + +Legacy-raw-emit on the socket is a candidate for the first entry: it is already +dual-served and already documented as non-preferred, but has no announced sunset. + +### Retired + +| Surface | Retired | Replacement | Behaviour now | +| ------------------------------------ | ------------------------------------------------- | ------------------------------------------------------------------ | -------------------------------------------------------------------------- | +| `POST /devices` | JWT-only device registration (#333) | `POST /devices/link/challenge` → `POST /devices/link/verify` | `403` naming the replacement flow | +| `GET /sync` → `sequenceNumber` | Per-conversation sequence number as a sync cursor | Opaque `nextCursor` (`:`), stable across conversations | Field absent; asserted by `sync.routes.test.ts` | +| `POST /auth/verify` without `device` | Deviceless authentication | `device` block required on every verify | `400` from schema validation | +| JWT without `deviceId` | Pre-device-aware tokens | Re-authenticate | `verifyToken` throws `Token missing deviceId — re-authentication required` | + +--- + +## 7. Checklist for a change that touches the API + +- [ ] Diff reviewed against [§2](#2-what-counts-as-a-breaking-change) — response fields, socket payloads, and any migration. +- [ ] If breaking: can it be additive instead? Can it be a capability instead? +- [ ] If it must break: replacement shipped and documented first. +- [ ] Register row added with a sunset date ≥ 90 days out. +- [ ] Route doc under `apps/backend/docs/` updated; event catalog updated for socket changes. +- [ ] Retirement returns an explicit error naming the replacement — never a bare 404 or a silently missing event. +- [ ] Migrations reviewed as API changes, not just schema changes. + +--- + +## 8. Related documents + +- [WebSocket event catalog](../apps/backend/docs/api-websocket-events.md) — every event, its payload, and the envelope/legacy split +- [REST schemas](../apps/backend/docs/contracts-rest-schemas.md) — request/response shapes this policy governs +- [WebSocket payload contracts](../apps/backend/docs/contracts-websocket-payloads.md) +- [Devices & prekeys API](../apps/backend/docs/api-devices.md) — the routes carrying the retired `POST /devices` and the link flow that replaced it +- [Message sync API](../apps/backend/docs/api-messages-sync.md) — the cursor contract referenced in §2.1 +- [Message encryption migration](../apps/backend/docs/message-encryption-migration.md) and [Signal migration](../apps/backend/docs/signal-migration.md) — capability negotiation applied to a live protocol rollout From f473432b9d47f8418240a99f13c87457de0f5274 Mon Sep 17 00:00:00 2001 From: Depo-dev Date: Sun, 30 Aug 2026 03:27:30 +0100 Subject: [PATCH 3/4] docs(ai_agent): add configuration and environment reference Adds apps/ai_agent/docs/configuration.md. Documents that OPENAI_API_KEY is the only environment variable main.py reads, with required status, no default, and no .env loading, and lists the variables the service does not read - model selection, Weaviate host/port/key, HOST and PORT - since setting those silently does nothing. The model choices, bind address, collection name, search limit, high-value threshold and per-call timeouts are tabulated as hardcoded values, with the note that the 0.0.0.0:8000 literals only apply to direct python main.py execution and not to fastapi/uvicorn launches. Documents the missing-API-key behaviour precisely: the key is read per-request inside _openai_client(), so the process starts clean, /health returns 200 and every OpenAI-backed endpoint returns 500. Both halves are pinned by tests. Spells out the load-balancer consequence - an instance with a bad key passes its liveness probe, joins the pool and serves nothing but 500s - and records the endpoint-by-endpoint matrix, including the high-value /transfers/analyse branch that answers 200 without ever constructing a client. Documents the Weaviate connection as an unconfigurable per-request connect_to_local() on the client's localhost defaults, the 503 failure mode, that infra/docker-compose.yml ships no Weaviate service, and that a containerised agent cannot reach a sibling container over loopback. Cross-links the repo-wide .env.example as the single environment reference. --- apps/ai_agent/docs/configuration.md | 289 ++++++++++++++++++++++++++++ 1 file changed, 289 insertions(+) create mode 100644 apps/ai_agent/docs/configuration.md diff --git a/apps/ai_agent/docs/configuration.md b/apps/ai_agent/docs/configuration.md new file mode 100644 index 0000000..e83c80f --- /dev/null +++ b/apps/ai_agent/docs/configuration.md @@ -0,0 +1,289 @@ +# AI agent configuration & environment reference + +Every setting the `ai_agent` service reads, what happens when one is missing, and +which settings are not configurable at all. + +The headline is short, and it is deliberately stated up front because it is the +opposite of what most services look like: **the service reads exactly one +environment variable, `OPENAI_API_KEY`.** Everything else — the model, the +Weaviate connection, the bind host and port — is hardcoded in +[`apps/ai_agent/main.py`](../main.py). Changing any of them is a code change, not +a deployment setting. + +--- + +## 1. Environment variables + +| Variable | Required | Default | Read at | Effect when unset | +| ---------------- | -------- | ------- | ---------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `OPENAI_API_KEY` | Yes\* | _none_ | Per request, inside `_openai_client()` — not at import and not at boot | The process still starts and `/health` still returns 200; every OpenAI-backed endpoint returns 500. See [§3](#3-startup-and-missing-api-key-behaviour). | + +\* Required for the service to do anything useful. It is _not_ required for the +process to start, and that distinction is the single most important thing in this +document. + +It is the same key the backend uses — it is declared once in the repo-root +[`.env.example`](../../../.env.example) under `# AI Service`: + +```bash +# AI Service +OPENAI_API_KEY= +``` + +There is no `.env` loading in `main.py` — no `python-dotenv`, no +`pydantic-settings`. The variable must be present in the process environment. In +local development that is whatever your shell or process manager exports; in +Docker or Kubernetes it is the container environment. + +### Variables the service does **not** read + +Worth stating explicitly, because their absence is easy to mistake for a bug: + +| Variable | Status | +| -------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | +| `OPENAI_MODEL` (or any model-selection variable) | Not read. Models are hardcoded — see [§2](#2-hardcoded-settings). | +| `WEAVIATE_URL` / `WEAVIATE_HOST` / `WEAVIATE_PORT` | Not read. The connection is `weaviate.connect_to_local()` — see [§4](#4-weaviate-connection). | +| `WEAVIATE_API_KEY` | Not read. The client connects unauthenticated. | +| `HOST` / `PORT` | Not read. See [§2](#2-hardcoded-settings). | +| `OPENAI_BASE_URL` | Not read. `OpenAI(api_key=...)` is constructed with no base URL override, so the SDK's own environment handling is the only way to redirect it. | + +Setting any of these has no effect on this service. + +--- + +## 2. Hardcoded settings + +| Setting | Value | Where | +| -------------------- | ------------------------------------------------------ | ---------------------------------------------------- | +| Chat model | `gpt-4o-mini` | `chat`, `analyse_transfer`, `summarise_proposal` | +| Embedding model | `text-embedding-3-small` | `index_message`, `search_messages` | +| Bind host | `0.0.0.0` | `uvicorn.run(...)` under `if __name__ == "__main__"` | +| Bind port | `8000` | same | +| Weaviate collection | `Message` | `index_message`, `search_messages` | +| Search result limit | `5` | `search_messages` | +| High-value threshold | `10_000.0` XLM | `_HIGH_VALUE_THRESHOLD`, used by `analyse_transfer` | +| Request timeouts | 30 s for `/chat`, 10 s for the two JSON-mode endpoints | passed per call as `timeout=` | +| System prompt | `_SYSTEM_PROMPT` | module level | + +Two consequences: + +- **Model changes require a code change and a deploy.** There is no way to move + to a different model, or to run two environments on different models, through + configuration. +- **The host and port literals only apply to `python main.py`.** Running the + module directly executes the `__main__` block and binds `0.0.0.0:8000`. Under + any ASGI server invoked against the app object — which is how the repo README + runs it (`uv run fastapi dev main.py`) and how a container normally runs it — + the `__main__` block never executes and the host/port come from that command: + + ```bash + # __main__ block runs: binds 0.0.0.0:8000 + python main.py + + # __main__ block does NOT run: fastapi/uvicorn decides the bind address + uv run fastapi dev main.py # defaults to 127.0.0.1:8000 + uv run uvicorn main:app --host 0.0.0.0 --port 9000 + ``` + + So the port is effectively set by the launch command, and the `8000` in + `main.py` is a default only for the direct-execution path. A deployment that + needs a different port sets it on the server command line. + +--- + +## 3. Startup and missing-API-key behaviour + +`OPENAI_API_KEY` is read **per request**, inside the `_openai_client()` helper: + +```python +def _openai_client(): + api_key = os.environ.get("OPENAI_API_KEY") + if not api_key: + raise HTTPException(status_code=500, detail="OPENAI_API_KEY is not configured") + if OpenAI is None: + raise HTTPException(status_code=500, detail="openai package is not installed") + + return OpenAI(api_key=api_key) +``` + +Nothing validates it at import time or at startup. The service therefore **starts +successfully with no API key at all** and fails only when an OpenAI-backed +endpoint is called. + +### The exact behaviour, endpoint by endpoint + +| Endpoint | With no `OPENAI_API_KEY` | +| --------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `GET /health` | **200** `{"status": "ok"}` | +| `POST /chat` | **500** `{"detail": "OPENAI_API_KEY is not configured"}` | +| `POST /transfers/analyse` | **500** — unless `amount > 10000`, which is answered by the rule-based branch **before** any OpenAI client is constructed, and still returns **200** | +| `POST /proposals/summarise` | **500** | +| `POST /index/message` | **500** if Weaviate is reachable (the embedding call needs the key); **503** if Weaviate is unreachable, because the connection is attempted first | +| `GET /search` | Same as above — **503** on a Weaviate failure, **500** on the missing key when Weaviate is up, and **200** with `{"results": []}` when the `Message` collection does not exist, because that path returns before any embedding is requested | + +### This is deliberate, and it is load-balancer-visible + +`/health` is a liveness probe: it answers whether the process is up and serving, +not whether every downstream dependency is configured. Because it does not touch +`_openai_client()`, a misconfigured deployment presents as: + +- **healthy** to any load balancer, orchestrator probe, or uptime monitor + pointed at `/health`, and +- **completely broken** to every real caller, with a 500 on each request. + +That combination is the failure mode to watch for. An instance rolled out with a +missing or misspelled key will pass its health check, be added to the pool, and +serve nothing but 500s. Kubernetes will not restart it; a load balancer will not +drain it. + +The test suite pins this behaviour rather than leaving it incidental — +`apps/ai_agent/tests/test_health.py::test_health_works_without_api_key` asserts +the 200, and `apps/ai_agent/tests/test_chat.py::test_missing_api_key_returns_500` +asserts the 500. Changing either is a deliberate contract change. + +**Operational recommendation.** Do not rely on `/health` to catch a +misconfiguration. Either: + +- assert the key is present at deploy time (a startup check in your orchestration, + or a required secret rather than an optional env var), or +- monitor the 5xx rate on `/chat` separately from the liveness probe, or +- add a readiness endpoint that checks configuration — deliberately distinct from + `/health`, which should keep its current semantics. + +### Related failure: the `openai` package missing + +`main.py` imports `openai` inside a `try/except ImportError` and sets +`OpenAI = None` when it is absent, so an environment without the dependency also +starts cleanly and fails per-request with **500** +`{"detail": "openai package is not installed"}`. Same shape of problem, same +`/health`-still-green consequence. + +--- + +## 4. Weaviate connection + +### How it connects + +```python +client = weaviate.connect_to_local() +``` + +No host, no port, no API key, no gRPC configuration. `connect_to_local()` from +`weaviate-client` (pinned at 4.22.0 in [`uv.lock`](../uv.lock), declared as +`>=4.0.0` in [`pyproject.toml`](../pyproject.toml)) uses its own defaults — +HTTP on `localhost:8080` and gRPC on `localhost:50051` — and this service +overrides none of them. + +A connection is opened **per request** on `POST /index/message` and `GET /search`, +and closed in a `finally` block. There is no pooled or long-lived client, and no +connection is attempted at startup. + +**There is no Weaviate service in [`infra/docker-compose.yml`](../../../infra/docker-compose.yml).** +That file brings up Postgres, Redis, and MinIO only. A Weaviate instance must be +run separately, and it must be reachable on the loopback defaults above from the +perspective of the `ai_agent` process — which means a containerised `ai_agent` +talking to a Weaviate in another container will not connect without host +networking or a code change, since `localhost` inside the container is the +container itself. + +### Failure mode when Weaviate is unreachable + +Both Weaviate-backed endpoints wrap the connection in `try/except` and translate +any failure to **503**: + +```python +try: + client = weaviate.connect_to_local() +except Exception: + raise HTTPException(status_code=503, detail="Weaviate connection failed") +``` + +| Condition | Response | +| ----------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- | +| Weaviate down / unreachable | **503** `{"detail": "Weaviate connection failed"}` | +| Weaviate up, later operation fails | **503** with the underlying exception text as `detail` — the second `except Exception as e` block re-raises as 503 with `str(e)` | +| Weaviate up, `Message` collection missing | `GET /search` → **200** `{"results": []}`; `POST /index/message` creates the collection and proceeds | +| Weaviate up, `OPENAI_API_KEY` missing | **500** — the embedding call fails after the connection succeeds | + +Two points that matter operationally: + +- **`/health` does not check Weaviate either.** A deployment with no Weaviate + reachable is green on its liveness probe and serves 503s on `/search` and + `/index/message`, exactly as with a missing API key. Chat and proposal + summarisation are unaffected. +- **The 503 detail can leak internals.** The second handler passes `str(e)` + straight into the response body, so a client can see raw client-library error + text. Treat that as an internal-facing detail, not a stable contract. + +`apps/ai_agent/tests/test_search.py::test_weaviate_connection_failure_returns_503` +pins the 503 behaviour, and `test_missing_collection_returns_empty_results` pins +the empty-result path, including that no query is issued. Note that the +equivalent failure on `POST /index/message` has no test of its own — the coverage +is on the `/search` side only. + +### Collection schema + +The collection is created on demand by `POST /index/message` with +`client.collections.create(name="Message")` — no property schema and no +vectoriser, because vectors are supplied explicitly from OpenAI embeddings. +Properties written per object are `conversationId`, `messageId`, `senderId`, and +`content`; searches filter on `conversationId` and return the top 5 by vector +similarity. See +[`contracts-weaviate-schema.md`](./contracts-weaviate-schema.md) for the full +shape and [`concepts-rag-search-architecture.md`](./concepts-rag-search-architecture.md) +for how it is used. + +--- + +## 5. Running the service + +From the repo root, as documented in the [main README](../../../README.md): + +```bash +cd apps/ai_agent && uv run fastapi dev main.py +``` + +With the API key set for the process: + +```bash +OPENAI_API_KEY=sk-... uv run fastapi dev main.py +``` + +Tests (Python 3.12+, per `requires-python` in `pyproject.toml`): + +```bash +cd apps/ai_agent +uv sync --group dev +uv run pytest +``` + +The suite sets `OPENAI_API_KEY=test-key` for every test through an autouse +fixture in [`tests/conftest.py`](../tests/conftest.py) and mocks both `OpenAI` +and `weaviate.connect_to_local`, so **no network access and no real key are +needed** — and, as a corollary, a real misconfiguration will never be caught by +running the tests. + +CI runs ruff, ruff format, mypy, and pytest with coverage via +[`.github/workflows/ai-agent-ci.yml`](../../../.github/workflows/ai-agent-ci.yml). + +--- + +## 6. Configuration checklist for a deployment + +- [ ] `OPENAI_API_KEY` is present in the process environment — verified by something other than `/health`. +- [ ] The `openai` package is installed in the runtime image (`uv sync`), or every request 500s. +- [ ] If `/search` or `/index/message` are used: a Weaviate instance is running and reachable at the client's local defaults **from the ai_agent process's own network namespace**. +- [ ] Bind host and port set on the server command line, not expected from env. +- [ ] Monitoring watches the 5xx rate on `/chat` and the 503 rate on `/search`, not just liveness. +- [ ] Model choice reviewed as a code change — `gpt-4o-mini` and `text-embedding-3-small` are compiled in. + +--- + +## 7. Related documents + +- [Repo-wide environment reference](../../../.env.example) — the single place every service's variables are declared, including the `OPENAI_API_KEY` this service shares with the backend. There is no separate env-reference document; `.env.example` is it. +- [Main README](../../../README.md) — how the AI service is started alongside the web and backend apps +- [`POST /chat`](./api-chat.md), [`POST /transfers/analyse`](./api-transfers-analyse.md), [`POST /proposals/summarise`](./api-proposals-summarise.md), [`/index` & `/search`](./api-index-search.md) — per-endpoint request/response contracts +- [Weaviate schema contract](./contracts-weaviate-schema.md) — the `Message` collection this service creates and queries +- [RAG search architecture](./concepts-rag-search-architecture.md) — how indexing and search fit together +- [Pydantic model contracts](./contracts-pydantic-models.md) — request/response models referenced above From 077fd46be4d9caf71f4cb0995d28da63741777eb Mon Sep 17 00:00:00 2001 From: Depo-dev Date: Sun, 30 Aug 2026 03:30:16 +0100 Subject: [PATCH 4/4] docs(web): add Tailwind and styling conventions Adds apps/web/docs/styling.md. Documents the Tailwind v4 setup: no tailwind.config file, the single @tailwindcss/postcss plugin, and globals.css as the only global stylesheet - with the distinction between its :root variable block and its @theme inline mapping, which is why both bg-card and bg-[var(--card)] compile. Gives guidance on which idiom to use, since the arbitrary-value form is roughly twice as common and is the only one that takes an opacity modifier. Documents the seven colour tokens with their roles and how heavily each is used, the spacing ladder and typography scale the existing screens actually follow, and the Badge tinted-surface recipe that stands in for absent status tokens. Records the ~30 off-token gray/slate and raw-hex occurrences as drift rather than precedent. Covers breakpoints - defaults, mobile-first, only sm/md/lg in use with zero xl or 2xl - and the hand-followed class order, noting that nothing enforces it since there is no prettier-plugin-tailwindcss and no Tailwind ESLint rule. States extraction rules in both directions, anchored on the three- occurrence threshold, variants, accessibility behaviour and non-utility CSS, and notes the absence of any cn/clsx helper. Records that dark mode does not exist: one :root palette that is itself dark, no prefers-color-scheme block, no darkMode config, no theme provider or toggle. The single dark: variant in CopyButton can never activate, so nobody should assume dark-mode support works or write new dark: variants. --- apps/web/docs/styling.md | 389 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 389 insertions(+) create mode 100644 apps/web/docs/styling.md diff --git a/apps/web/docs/styling.md b/apps/web/docs/styling.md new file mode 100644 index 0000000..a29e7c0 --- /dev/null +++ b/apps/web/docs/styling.md @@ -0,0 +1,389 @@ +# Tailwind & styling conventions + +How styling works in `apps/web`: the Tailwind v4 setup, where global styles live, +the design tokens that are actually in use, the breakpoints in play, and the +conventions for writing and extracting classes. + +--- + +## 1. Setup + +| Thing | Value | +| ----------------- | ---------------------------------------------------------------------------------------------- | +| Tailwind version | **v4** — `tailwindcss: ^4` and `@tailwindcss/postcss: ^4` in [`package.json`](../package.json) | +| Config file | **None.** There is no `tailwind.config.js`/`.ts`. v4 configures itself from CSS. | +| PostCSS | [`postcss.config.mjs`](../postcss.config.mjs), one plugin: `@tailwindcss/postcss` | +| Global stylesheet | [`src/app/globals.css`](../src/app/globals.css), imported once from `src/app/layout.tsx` | +| Framework | Next.js 16 App Router, React 19 | +| Fonts | `Geist` and `Geist_Mono` via `next/font/google`, wired in `layout.tsx` | + +```js +// postcss.config.mjs — the entire build-side setup +const config = { + plugins: { + '@tailwindcss/postcss': {}, + }, +}; + +export default config; +``` + +The v4 detail worth internalising: **there is no JavaScript config to edit.** +Theme extension happens in CSS via `@theme`, and content scanning is automatic — +you do not maintain a `content: []` array. If you reach for +`tailwind.config.js`, you are working against the setup. + +### Global styles + +`globals.css` is short and holds everything global: + +```css +@import 'tailwindcss'; + +:root { + --background: #0a0a0f; + --foreground: #f0f0f5; + --accent: #7c5cfc; + --accent-light: #a78bfa; + --muted: #3f3f50; + --card: #13131f; + --border: #1e1e2e; +} + +@theme inline { + --color-background: var(--background); + --color-foreground: var(--foreground); + --color-accent: var(--accent); + --color-accent-light: var(--accent-light); + --color-muted: var(--muted); + --color-card: var(--card); + --color-border: var(--border); + --font-sans: var(--font-geist-sans); + --font-mono: var(--font-geist-mono); +} + +body { + background: var(--background); + color: var(--foreground); + font-family: var(--font-sans), Arial, Helvetica, sans-serif; +} + +html { + scroll-behavior: smooth; +} +``` + +Two layers, and the difference matters: + +- The `:root` block declares the **raw CSS variables**. These are what + `[var(--accent)]` reads. +- The `@theme inline` block maps those into **Tailwind's theme namespace**, which + is what generates utilities like `bg-accent`, `text-foreground`, and + `border-border`. + +Both layers exist, so both idioms work. See +[§3](#3-two-ways-to-reach-a-token-and-when-to-use-each). + +Anything not global belongs in a component. The only component-level CSS in the +codebase is styled-jsx in +[`Spinner.tsx`](../src/components/ui/Spinner.tsx) and +[`SkeletonLoader.tsx`](../src/components/ui/SkeletonLoader.tsx), both for +keyframe animations that utilities cannot express — including a +`prefers-reduced-motion` guard. That is the bar for reaching past utilities: +**a keyframe or a media query Tailwind has no utility for.** + +--- + +## 2. Design tokens + +Seven colour tokens, all defined in `globals.css`. There is no secondary palette, +no semantic status colours in the theme, and no spacing or typography tokens +beyond Tailwind's built-in scales. + +| Token | Value | Role | In use | +| ---------------- | --------- | --------------------------------------- | ------ | +| `--background` | `#0a0a0f` | Page background — near-black, blue-cast | ~23 | +| `--foreground` | `#f0f0f5` | Primary text | ~143 | +| `--accent` | `#7c5cfc` | Primary action / brand violet | ~71 | +| `--accent-light` | `#a78bfa` | Accent text on accent-tinted surfaces | ~10 | +| `--muted` | `#3f3f50` | Muted surface / de-emphasised element | ~15 | +| `--card` | `#13131f` | Raised surface — cards, panels | ~42 | +| `--border` | `#1e1e2e` | Hairline borders and dividers | ~71 | + +Counts are utility occurrences across `src/**/*.tsx`, summing both idioms in +[§3](#3-two-ways-to-reach-a-token-and-when-to-use-each). They are here to show +which tokens carry the UI, not as a target. + +**Status colours are not tokens.** Success, warning, and danger use Tailwind's +stock palette at fixed opacities, established by +[`Badge.tsx`](../src/components/ui/Badge.tsx): + +```ts +const VARIANT_CLASS: Record = { + default: 'border-[var(--accent)]/30 bg-[var(--accent)]/15 text-[var(--accent-light)]', + success: 'border-green-500/30 bg-green-500/15 text-green-300', + warning: 'border-yellow-500/30 bg-yellow-500/15 text-yellow-200', + danger: 'border-red-500/30 bg-red-500/15 text-red-300', +}; +``` + +That `/30` border, `/15` fill, `-300` text recipe is the house style for a tinted +status surface. Reuse `` rather than re-deriving it. + +### Spacing + +No custom spacing scale — Tailwind's default. What the existing screens actually +use, in descending frequency: + +| Purpose | Values in use | +| ----------------- | ------------------------------------------------------------------------------------------ | +| Control padding | `px-4 py-2` (most common), `px-3 py-2`, `px-5 py-2`, `px-6 py-3` | +| Container padding | `p-6` for cards and panels, `p-4` and `p-5` for tighter ones, `p-3`/`p-2` for compact rows | +| Flex/grid gaps | `gap-3` (most common), then `gap-4`, `gap-2`, `gap-1`, `gap-6` | +| Vertical rhythm | `space-y-4`, then `space-y-3` | +| Section padding | `py-32` on landing sections | + +Stay on this ladder — `2 / 3 / 4 / 6` covers nearly everything. Reach for an +arbitrary value only when matching a specific visual, and prefer the nearest step +otherwise. + +### Typography + +Two families, both from `next/font/google` and exposed through `@theme` as +`--font-sans` and `--font-mono`. `body` sets sans by default, so `font-sans` is +rarely written explicitly; `font-mono` is for addresses, hashes, and amounts. + +| Scale | Where it is used | +| -------------------- | --------------------------------------------------------------------------- | +| `text-xs` | Metadata, timestamps, badge-adjacent labels — very common | +| `text-sm` | **The default for body and UI copy** — the most-used size by a wide margin | +| `text-base` | Rare; `text-sm` is the norm, so reaching for `base` is a deliberate step up | +| `text-lg` | Sub-headings and emphasised rows | +| `text-xl`–`text-3xl` | Section and page headings; `text-3xl` is the common heading size | +| `text-4xl`+ | Landing page hero only | + +| Weight | Use | +| --------------- | -------------------------------------------------------- | +| `font-semibold` | The default for anything emphasised — most common by far | +| `font-medium` | Softer emphasis, secondary labels | +| `font-bold` | Headings and hero copy | + +Normal body text carries no weight class. If you find yourself writing +`font-normal`, something upstream is over-weighted. + +### Radius + +`rounded-full` (pills, avatars, badges) and `rounded-lg` dominate, with +`rounded-2xl` for cards and panels and `rounded-xl` for medium controls. Prefer +`rounded-2xl` for a new card and `rounded-full` for a new pill or button — that +is what surrounding screens do. + +### De-facto tokens that are not in `globals.css` + +Two things recur without being declared, and both are worth knowing before you +copy them: + +- **Opacity-modified tokens.** `bg-[var(--card)]/30`, + `text-[var(--foreground)]/50`, `border-[var(--accent)]/30` are used constantly + to derive a surface or a muted text colour from an existing token. This is + preferred over inventing a new colour: it stays on-palette automatically. +- **Off-token colours.** About 30 occurrences of `gray-*`/`slate-*` and a handful + of raw hex values (`[#13131f]`, `[#0F172A]`, `[#0C3F51]`) exist, mostly in + older components — `CopyButton.tsx` is entirely on a light-mode grey palette + that does not belong to this theme. **These are drift, not precedent.** Use the + tokens for new work, and prefer `[#13131f]` → `[var(--card)]` when you are + already editing a file that has one. + +--- + +## 3. Two ways to reach a token, and when to use each + +Because `globals.css` declares both the raw variables and the `@theme` mapping, +both of these compile and both appear in the codebase: + +```tsx +
{/* theme utility */} +
{/* arbitrary value */} +``` + +The arbitrary-value form is roughly twice as common in the current tree +(~193 occurrences vs ~107). Neither is wrong. Pick on this basis: + +- **Need an opacity modifier?** Use `[var(--token)]/NN`. This is why the + arbitrary form dominates — `bg-[var(--card)]/30` has no short theme-utility + equivalent here. +- **Plain, full-opacity colour?** Either works; `bg-card` is shorter and reads + better. +- **Editing an existing file?** Match what that file already does. Mixing both + idioms inside one `className` is the thing to avoid. + +--- + +## 4. Responsive breakpoints + +Tailwind's defaults, unmodified: `sm` 640px, `md` 768px, `lg` 1024px, `xl` +1280px, `2xl` 1536px. + +Only `sm`, `md`, and `lg` are used. There are **zero** `xl:` or `2xl:` +occurrences in `src/`, so the widest layout you can currently rely on having been +designed is the `lg` one. `max-w-6xl` is the usual page container cap. + +The codebase is **mobile-first**: base classes describe the narrow layout and +breakpoint prefixes widen it. Follow that — never write a desktop base and narrow +it with `max-*` variants. + +Established patterns: + +| Pattern | Examples in use | +| ---------------------- | ------------------------------------------------------------------------- | +| Grid columns | `sm:grid-cols-2`, `md:grid-cols-2/3/5`, `lg:grid-cols-3/4/5` | +| Progressive disclosure | `hidden md:inline`, `hidden sm:flex`, `hidden md:flex`, `hidden lg:block` | +| Container width | `max-w-6xl` for page shells; `max-w-sm`/`md`/`lg` for cards and dialogs | + +Chat and conversation views assume a working narrow layout, so test any change +there at a phone width before a desktop one. + +--- + +## 5. Class ordering + +**Nothing enforces class order.** There is no `prettier-plugin-tailwindcss` in +[`package.json`](../../../package.json) and no Tailwind ESLint rule in +[`eslint.config.mjs`](../eslint.config.mjs) — the config is +`eslint-config-next` core-web-vitals plus TypeScript, nothing more. Ordering is a +convention you follow by hand, and no CI job will correct you. + +The order the existing components follow, and the one to write: + +1. **Layout & display** — `flex`, `grid`, `inline-flex`, `hidden`, `block` +2. **Box model / sizing** — `h-full`, `w-full`, `min-h-40`, `max-w-md` +3. **Flex & grid children** — `items-center`, `justify-between`, `flex-col`, `gap-3` +4. **Spacing** — `p-6`, `px-4`, `py-2`, `mt-2`, `space-y-4` +5. **Border & radius** — `rounded-2xl`, `border`, `border-dashed`, `border-[var(--border)]` +6. **Background** — `bg-[var(--card)]/30` +7. **Typography** — `text-sm`, `font-semibold`, `leading-relaxed`, `text-[var(--foreground)]` +8. **Effects** — `shadow-lg`, `blur-[120px]`, `transition-opacity` +9. **State & responsive variants last** — `hover:opacity-90`, `focus:ring-2`, `md:inline`, `lg:grid-cols-3` + +[`EmptyState.tsx`](../src/components/ui/EmptyState.tsx) is a clean reference: + +```tsx +className = + 'flex h-full min-h-40 w-full flex-col items-center justify-center gap-3 rounded-2xl border border-dashed border-[var(--border)] bg-[var(--card)]/30 p-6 text-center'; +``` + +Keeping variants at the end matters most in practice — it makes the base +(mobile) layout readable in one glance. + +--- + +## 6. When to extract a component + +There is **no `cn()`/`clsx`/`tailwind-merge` helper** in this codebase. Classes +are composed with template literals and an appended `className` prop: + +```tsx +className={`... base classes ... ${VARIANT_CLASS[variant]} ${className ?? ''}`} +``` + +Follow that pattern rather than introducing a class-merging utility for a single +component; if one is added, it should land as its own change with every call site +converted. + +### Extract when + +- **The same cluster appears three or more times.** Two occurrences are a + coincidence; three is a component. The tinted-pill recipe hit that bar and + became ``. +- **It has variants.** The moment you write a conditional that picks between + class strings, that belongs in a `Record` map inside a + component — the shape `Badge` and `Spinner` both use. +- **It carries accessibility or behaviour.** `Spinner` exists as much for its + `role="status"`/`aria-hidden` logic and reduced-motion handling as for its + look. Anything with ARIA, focus management, or an animation should not be + copy-pasted. +- **It needs non-utility CSS.** Keyframes or a media query utilities cannot + express force a component boundary anyway. +- **It is a recognisable UI noun.** Avatar, badge, modal, empty state, skeleton, + spinner — if you can name it, it belongs in + [`src/components/ui/`](../src/components/ui/). + +### Repeat the utilities when + +- **It is used once or twice.** Do not build an abstraction for a single caller. +- **The variation is layout, not identity.** A card that is `p-4` here and `p-6` + there with a different grid is two layouts, not one component with two props. +- **Extracting would need more props than the classes it saves.** A wrapper + taking six props to configure spacing is worse than the spacing. +- **It is page-specific composition.** Section layout in a route belongs in that + route. + +### Where components live + +| Directory | Contents | +| --------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------- | +| [`src/components/ui/`](../src/components/ui/) | Generic primitives — `Avatar`, `Badge`, `CopyButton`, `EmptyState`, `Modal`, `ProposalCard`, `SkeletonLoader`, `Spinner` | +| `src/components//` | Feature-scoped components — `auth/`, `chat/`, `conversations/`, `landing/`, `messaging/`, `search/`, `treasury/`, `wallet/` | + +A component reused across two features moves to `ui/`; one that knows about a +domain type stays in its feature directory. + +--- + +## 7. Dark mode + +**There is no dark-mode support, and there is nothing to toggle.** The app is +**dark-only** by design: + +- The single `:root` palette in `globals.css` _is_ the dark theme — `#0a0a0f` + background, `#f0f0f5` foreground. There is no light palette anywhere. +- There is **no `@media (prefers-color-scheme: ...)` block** in the codebase. +- There is **no `darkMode` configuration** — v4 would configure it in CSS via a + custom variant, and none is declared. +- There is **no theme provider, no `next-themes`, and no theme toggle.** + `layout.tsx` wraps the tree in `WalletProvider`, `AuthProvider`, and + `ToastProvider` only. +- `` carries no `class="dark"` or `data-theme` attribute. + +Exactly **one** `dark:` variant exists in the whole of `src/` — +`text-emerald-600 dark:text-emerald-500` in +[`CopyButton.tsx`](../src/components/ui/CopyButton.tsx). Because no dark-mode +strategy is configured, **that variant never activates.** It is a leftover from a +component authored against light-mode assumptions, which is also why that file +uses `slate-50`, `gray-100`, and `#0C3F51` — colours that belong to no theme +here. + +Consequences for new work: + +- **Do not write `dark:` variants.** They are dead code. Style for the dark + palette directly. +- **Do not assume a light background.** A component built with light-mode + defaults will be illegible; `CopyButton` is what that looks like in practice. +- **Adding light mode is a project, not a patch.** It would need a light palette, + a variant strategy declared in `globals.css`, a theme provider with persistence, + and an audit of every hardcoded colour listed in + [§2](#de-facto-tokens-that-are-not-in-globalscss). Nothing today is written to + support it. + +--- + +## 8. Checklist for a new component + +- [ ] Colours come from the seven tokens — theme utility or `[var(--token)]`, with `/NN` for tints. No new raw hex, no `gray-*`/`slate-*`. +- [ ] Spacing is on the `2 / 3 / 4 / 6` ladder; padding matches the `px-4 py-2` / `p-6` conventions. +- [ ] Type is `text-sm` unless there is a reason; emphasis is `font-semibold`. +- [ ] Radius matches neighbours — `rounded-2xl` for cards, `rounded-full` for pills. +- [ ] Written mobile-first, with `sm:`/`md:`/`lg:` widening it. No `xl:`/`2xl:` unless you are also designing that width. +- [ ] Classes ordered per [§5](#5-class-ordering); variants last. +- [ ] No `dark:` variants. +- [ ] Third occurrence of the same cluster? Extract it to `src/components/ui/`. +- [ ] Non-utility CSS is styled-jsx inside the component, with a `prefers-reduced-motion` guard on any animation. + +--- + +## 9. Related documents + +- [Wallet & treasury UI](./concepts-wallet-treasury-ui.md) — the screens most of these conventions were established on +- [Auth & device lifecycle](./concepts-auth-device-lifecycle.md) +- [Message pipeline](./concepts-message-pipeline.md) — the chat views to test at narrow widths +- [`src/app/globals.css`](../src/app/globals.css) — the source of truth for every token above