Make the plan update rate limit visible #373
- MA
## Why
`submit()` in `apps/server/src/plan/service.ts` dropped every update over 200 a second per socket. It sent no reply and logged nothing. In the Decisions audit this cost a user's typing (see #358): Yjs holds back later updates until a dropped one arrives, so nothing else that client typed got through, and nothing in the log showed why. #358 stops current clients from going over the limit and makes them resend. A tab still running an older bundle only replays its outbox when it reopens.
## What changed
- **Refusal on the wire.** A dropped update gets `fail(ws, rid, "rate limited")`. - **Log line.** One warning per one-second window of drops, naming the handle and document. - **Forced reconnect.** After drops in three consecutive windows, the server closes the socket with `4429 plan updates too fast`. The web client retries every close except 4404, so an older client reconnects, reopens the document and replays its unacknowledged updates. - **Accepted updates are untouched.** Queueing, the 5 ms grouping, and persistence before acknowledgement or relay work as before.
## Server flow

The server reports dropped updates and lets an older client recover by reconnecting after sustained over-limit traffic.
## Testing
- New `apps/server/src/plan/rate-limit.test.ts` (3 tests) covers: - 205 updates in one window: 200 acknowledged, 5 refused with `rate limited` and their rids, one warning, no close - drops in three consecutive windows close the socket with 4429 - a quiet gap resets the count
The first two fail without the change. - `bun test apps/server/src/plan`: 191 pass. - `bun run types`: clean. `bun run ci`: passes. `bun run fix`: no further changes. Final rebased head `818564d0` passed hosted validation, E2E, and container checks.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
MaggieAppletonreferenced this in Keep fast typing from being silently dropped #358last month on GitHub
- MA
**Hold — please don't merge yet.** Server-side rate-limit/socket-close change; a review is in progress. I'll post the verdict here.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
- MA
**Review:** Looks good
Checked against AGENTS.md (runtime model, persistence before publication, collaboration traps) and #358's client batching and resend.
**1. Can a real user trip the close?** In practice, no. - Only browser `plan:update` frames reach `submit()` (`main.ts`). Planner and MCP edits are server-authored and skip the meter. - The meter is keyed per `(plan, ws)`. Each tab has its own socket and its own meter. - A paste is one Yjs transaction. IME composition and key repeat (about 30/s) stay far below 200/s. - With #358, `SEND_MS = 10` caps a client at about 100 flushes a second, plus merged resends with backoff. 200/s is out of reach for normal use.
**2. Reconnect-storm risk:** low. - The meter belongs to the socket, so a reconnect starts again with zero strikes. - An old client's reopen replays at most 1,000 outbox items in one burst. That counts as one strike, and the close needs three consecutive windows. - `Wire` retries with jittered exponential backoff, starting at 250–750 ms. `#attempts` resets on open, so a client that is really sustained over the limit would cycle about every 3 s. That is bounded and logged.
**3. Logging:** contains the handle and document ID only, with no update content or tokens. At most one line per second per socket.
**4. Reply shape:** `fail()` sends a `session:error` with the request's `rid`, which matches `Protocol.Failure`. Older clients never register a pending entry for `plan:update` rids, and on main nothing listens to `session:error`, so the reply is ignored. #358's listener only parks on `implementation is active`, so `rate limited` leaves the resend path in charge, which is correct.
**5. Tests:** meaningful. I reverted `service.ts` to main and the first two tests failed. The quiet-gap test covers the consecutive-window reset. `bun test apps/server/src/plan`: 191 pass locally. CI is green.
**Non-blocking note:** an old bundle that goes over the limit for only one or two windows still strands its later edits until it next reopens, because the close needs three windows. Humans can't type that fast, so I'd leave it as is.
- MA
Hold lifted.
- MA
Duplicate of the review above, posted twice by mistake.
MaggieAppletonhead-ref-force-pushed 818564d0bda4last month on GitHub
MaggieAppletonmerged 1eba6521091flast month on GitHub
MaggieAppletonhead-ref-deletedlast month on GitHub