mirror of
https://github.com/tiennm99/rplace.git
synced 2026-10-13 08:15:57 +00:00
plans: drop implemented plan folders
Both plans shipped to production:
- 260509-2309-canvas-on-do-storage (Upstash → DO migration, deployed
2026-05-10, Upstash fully removed in a977adc).
- 260510-0232-fix-do-migration-followups (post-migration P1 fixes,
docs cleanup, full DO test coverage — all completed 2026-05-11).
Git history retains the full content if needed; plans/reports/ kept
as-is for the original research/review artifacts.
This commit is contained in:
tiennm99 committed
1 parent
214a00f9d7
commit
7ef64ecd4f
12 files changed
-1466
No files matched your search
@@ -1,110 +0,0 @@
|
||||
---
|
||||
phase: 1
|
||||
title: "Storage Foundation"
|
||||
status: completed
|
||||
priority: P2
|
||||
effort: "3h"
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Phase 1: Storage Foundation
|
||||
|
||||
## Overview
|
||||
|
||||
Add SQLite-backed storage modules consumable by the DO. Pure logic, no integration yet. Unit-testable in isolation.
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional:**
|
||||
- Read/write canvas bytes via chunked BLOB rows.
|
||||
- Read/write per-user cooldowns with lazy expiry GC.
|
||||
- Lazy-initialize missing chunks (return zero-fill on read; INSERT on first write).
|
||||
|
||||
**Non-functional:**
|
||||
- Pure functions / classes accepting `sql` interface — testable without a live DO.
|
||||
- Write path: at most 1 transaction per `placePixels` call.
|
||||
- O(touched_chunks) writes, not O(total_chunks).
|
||||
|
||||
## Architecture
|
||||
|
||||
Two new modules inside the DO directory:
|
||||
|
||||
- `src/durable-objects/lib/chunk-storage.js` — canvas chunk BLOB R/W, batch-aware
|
||||
- `src/durable-objects/lib/cooldown-store.js` — user_id → expires_at SET-NX semantic
|
||||
|
||||
Constants stay in `src/lib/constants.js` but get new entries:
|
||||
|
||||
```js
|
||||
export const CHUNK_BYTES = 65536; // 64 KB
|
||||
export const CHUNK_COUNT = Math.ceil(TOTAL_PIXELS / CHUNK_BYTES); // 256
|
||||
```
|
||||
|
||||
**Why 64 KB chunks (informed by actual CF DO limits, May 2026):**
|
||||
- BLOB row cap is **2 MB** → `CHUNK_BYTES` could be up to ~2 MB; we use 64 KB (32× under).
|
||||
- Storage caps are **10 GB / DO** and **5 GB / account** (Free) → 16 MB canvas
|
||||
is ~300× under either cap; resize to ~70K×70K is feasible on Free.
|
||||
- Soft request cap is **1,000 req/sec / DO** → hobby workload is 1‰ of this.
|
||||
- Smaller chunks minimize read-modify-write amplification on **clustered** writes
|
||||
(single placements + image patches), which is rplace's actual workload pattern.
|
||||
Bigger chunks would only help uniform-random writes — not a real workload here.
|
||||
|
||||
Schema (created in DO constructor via `sql.exec` if not exists):
|
||||
|
||||
```sql
|
||||
CREATE TABLE IF NOT EXISTS canvas_chunks (
|
||||
chunk_id INTEGER PRIMARY KEY,
|
||||
bytes BLOB NOT NULL
|
||||
);
|
||||
CREATE TABLE IF NOT EXISTS cooldowns (
|
||||
user_id TEXT PRIMARY KEY,
|
||||
expires_at INTEGER NOT NULL
|
||||
);
|
||||
CREATE INDEX IF NOT EXISTS idx_cooldowns_expires ON cooldowns(expires_at);
|
||||
```
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Create:**
|
||||
- `src/durable-objects/lib/chunk-storage.js`
|
||||
- `src/durable-objects/lib/cooldown-store.js`
|
||||
- `src/durable-objects/lib/schema.js` (CREATE TABLE statements, exported as a single `init(sql)` function)
|
||||
- `test/unit/chunk-storage.test.js`
|
||||
- `test/unit/cooldown-store.test.js`
|
||||
|
||||
**Modify:**
|
||||
- `src/lib/constants.js` — add `CHUNK_BYTES`, `CHUNK_COUNT`
|
||||
|
||||
**Delete:** None this phase.
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Add `CHUNK_BYTES`, `CHUNK_COUNT` to `src/lib/constants.js`. Verify `CHUNK_COUNT = 256` for 4096×4096.
|
||||
2. Write `schema.js` with idempotent `CREATE TABLE`/`CREATE INDEX` statements wrapped in single `init(sql)` export.
|
||||
3. Write `chunk-storage.js`:
|
||||
- `readChunk(sql, chunkId) → Uint8Array(CHUNK_BYTES)` — zero-fill if missing.
|
||||
- `readAllChunks(sql) → Uint8Array(TOTAL_PIXELS)` — concatenated buffer.
|
||||
- `writePixels(sql, pixels)` — group by chunk_id, single transaction, UPDATE OR INSERT each touched chunk.
|
||||
- Helper: `pixelToChunk(x, y) → {chunkId, byteOffset}`.
|
||||
4. Write `cooldown-store.js`:
|
||||
- `tryAcquire(sql, userId, ttlMs, now) → {allowed, retryAfter}` — INSERT OR FAIL pattern: if a non-expired row exists, return `{allowed:false}`; else upsert with `expires_at = now + ttlMs`.
|
||||
- `gc(sql, now)` — DELETE WHERE `expires_at < now`. Called opportunistically (e.g., 1% of acquires).
|
||||
5. Write unit tests using `better-sqlite3` or equivalent in-memory SQLite (Vitest):
|
||||
- chunk-storage: write-then-read, partial-chunk write, batch across chunks, lazy zero-init.
|
||||
- cooldown-store: acquire success, acquire blocked, acquire after expiry, GC removes stale rows.
|
||||
6. `npm test` — all green.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] `src/lib/constants.js` exports `CHUNK_BYTES`, `CHUNK_COUNT`
|
||||
- [ ] `chunk-storage.js` and `cooldown-store.js` exist with documented exports
|
||||
- [ ] Unit tests cover happy path + edge cases (lazy init, expiry, batch across chunks)
|
||||
- [ ] `npm test` passes
|
||||
- [ ] No imports from `@upstash/redis` in new modules
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| SQLite BLOB API differs in CF DO vs better-sqlite3 | Keep modules thin; integration test in Phase 2 confirms |
|
||||
| Chunk boundaries off-by-one for non-power-of-2 sizes | `CHUNK_COUNT = ceil()` handles partial last chunk; test it |
|
||||
| Cooldown table grows unbounded | Lazy GC + 1s TTL keeps rows ephemeral; index on expires_at keeps GC cheap |
|
||||
@@ -1,132 +0,0 @@
|
||||
---
|
||||
phase: 2
|
||||
title: "DO Integration & Worker Proxy"
|
||||
status: completed
|
||||
priority: P2
|
||||
effort: "4h"
|
||||
dependencies: [1]
|
||||
---
|
||||
|
||||
# Phase 2: DO Integration & Worker Proxy
|
||||
|
||||
## Overview
|
||||
|
||||
Wire Phase 1 storage modules into `CanvasRoom` DO. Add DO-side methods (`getFullCanvas`, `placePixels`, accept WS) using SQLite. Refactor `worker.js` to a thin validation/routing proxy. End state: app works against DO storage, Upstash still present but unused except for migration in Phase 3.
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional:**
|
||||
- DO exposes 3 internal HTTP endpoints (called by worker via `room.fetch`):
|
||||
- `GET /canvas` — returns 16 MB binary
|
||||
- `POST /place` — body `{userId, pixels}`, does cooldown check + write + broadcast atomically
|
||||
- `GET /ws` — WebSocket upgrade (existing behavior)
|
||||
- Worker validates request shape and forwards to DO. Worker no longer touches storage.
|
||||
- Broadcast still happens after successful write.
|
||||
|
||||
**Non-functional:**
|
||||
- Single round-trip Worker → DO per `/api/place` (no extra trips for cooldown).
|
||||
- DO write + broadcast same transaction-equivalent: cooldown check, pixel write, then broadcast — if write fails, no broadcast.
|
||||
|
||||
## Architecture
|
||||
|
||||
### DO method shape
|
||||
|
||||
```js
|
||||
// canvas-room.js (refactored)
|
||||
import { init as initSchema } from './lib/schema.js';
|
||||
import { readAllChunks, writePixels } from './lib/chunk-storage.js';
|
||||
import { tryAcquire, gc } from './lib/cooldown-store.js';
|
||||
import { REQUEST_COOLDOWN_SEC } from '../lib/constants.js';
|
||||
|
||||
export class CanvasRoom {
|
||||
constructor(state, env) {
|
||||
this.state = state;
|
||||
this.sql = state.storage.sql;
|
||||
initSchema(this.sql);
|
||||
}
|
||||
|
||||
async fetch(request) {
|
||||
const url = new URL(request.url);
|
||||
if (url.pathname === '/canvas') return this.#handleGetCanvas();
|
||||
if (url.pathname === '/place') return this.#handlePlace(request);
|
||||
if (url.pathname === '/ws') return this.#handleWsUpgrade();
|
||||
return new Response('not found', { status: 404 });
|
||||
}
|
||||
|
||||
// ... existing webSocketMessage/Close/Error handlers unchanged
|
||||
}
|
||||
```
|
||||
|
||||
### Worker shape
|
||||
|
||||
```js
|
||||
// worker.js (refactored)
|
||||
app.get('/api/canvas', (c) => forwardToRoom(c, 'GET', '/canvas'));
|
||||
app.post('/api/place', async (c) => {
|
||||
// validate body shape, batch size, pixel ranges (existing logic)
|
||||
const userId = await getUserId(c.req.raw);
|
||||
return forwardToRoom(c, 'POST', '/place', { userId, pixels: body.pixels });
|
||||
});
|
||||
app.get('/api/ws', (c) => forwardToRoom(c, 'GET', '/ws', null, c.req.raw));
|
||||
```
|
||||
|
||||
### Broadcast after write
|
||||
|
||||
Inside DO `#handlePlace`:
|
||||
1. `tryAcquire(sql, userId, 1000, Date.now())`. If blocked → return 429 with retryAfter.
|
||||
2. `writePixels(sql, pixels)`.
|
||||
3. Iterate `state.getWebSockets()` and `ws.send(JSON.stringify({type:'pixels', pixels}))`.
|
||||
4. Return 200 OK.
|
||||
|
||||
Broadcast is now in-DO, not via separate `room.fetch('/broadcast')`. Eliminates the internal `broadcastPixels` round-trip from Phase 0 design.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Modify:**
|
||||
- `src/durable-objects/canvas-room.js` — add `#handleGetCanvas`, `#handlePlace`, refactor `#handleWsUpgrade`. Schema init in constructor.
|
||||
- `src/worker.js` — strip storage logic, add `forwardToRoom` helper, keep validation.
|
||||
|
||||
**Create:**
|
||||
- `test/integration/do-canvas-room.test.js` — Wrangler `unstable_dev` or Vitest CF pool harness, exercising place/get/ws end-to-end.
|
||||
|
||||
**Delete:** None this phase (Upstash code stays for Phase 3 migration).
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Refactor `canvas-room.js`:
|
||||
- Constructor calls `initSchema(state.storage.sql)`.
|
||||
- Replace single `fetch()` body with method dispatch on `url.pathname`.
|
||||
- Implement `#handleGetCanvas`: `readAllChunks(sql)` → return `application/octet-stream` with `Cache-Control: public, max-age=10, s-maxage=10, stale-while-revalidate=30`.
|
||||
- Implement `#handlePlace`: parse body, cooldown check, write pixels, broadcast, return JSON.
|
||||
- Move WS upgrade from worker into `#handleWsUpgrade`.
|
||||
2. Refactor `worker.js`:
|
||||
- Add `forwardToRoom(c, method, path, jsonBody?, rawReq?)` helper.
|
||||
- Keep input validation (body size, pixel ranges, batch cap) in worker — defense in depth.
|
||||
- Replace direct Upstash calls with `forwardToRoom`.
|
||||
- `getUserId` stays in worker (uses request headers).
|
||||
3. Add `c.executionCtx.waitUntil(...)` for opportunistic `gc()` calls (or move GC inside DO with low probability).
|
||||
4. Write integration tests:
|
||||
- Boot Wrangler `unstable_dev` against this code.
|
||||
- Test: GET /api/canvas returns 16 MB, POST /api/place updates pixel, GET /api/canvas reflects update, second place from same user within 1s returns 429.
|
||||
- Test: WS connects, receives broadcast after another client's place.
|
||||
5. Run `npm run test:all`. Existing Upstash-dependent tests will fail; mark them xfail or skip in this phase (will be deleted in Phase 4).
|
||||
6. Run `npm run dev` (wrangler dev) and smoke-test in browser.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] `canvas-room.js` uses SQLite for canvas + cooldown
|
||||
- [ ] `worker.js` has zero `redis*` or `Upstash` references
|
||||
- [ ] Integration test: place pixel → read canvas → byte at expected offset is correct color
|
||||
- [ ] Integration test: rate-limit returns 429 with `retryAfter`
|
||||
- [ ] Browser smoke test: place pixels, see broadcast in second tab
|
||||
- [ ] No regression in WS hibernation behavior
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| `room.fetch` overhead per request | Single round-trip per /api/place; same as today's setup |
|
||||
| WS upgrade routing — worker → DO via fetch | Existing pattern works; passes `c.req.raw` through |
|
||||
| 16 MB GET response in single DO call may hit limits | DO subrequest size cap is 32 MB; canvas is 16 MB → fits |
|
||||
| Body size mismatch worker validation vs DO | Validate at both layers; DO trusts but verifies |
|
||||
| Existing tests break | Expected — will be replaced in Phase 4 |
|
||||
@@ -1,112 +0,0 @@
|
||||
---
|
||||
phase: 3
|
||||
title: "One-Shot Upstash Migration"
|
||||
status: completed
|
||||
priority: P2
|
||||
effort: "2h"
|
||||
dependencies: [2]
|
||||
---
|
||||
|
||||
# Phase 3: One-Shot Upstash Migration
|
||||
|
||||
## Overview
|
||||
|
||||
Copy production canvas bytes from Upstash Redis into the DO's SQLite chunks. Run once, verify, schedule removal. The migration endpoint is token-gated and short-lived — it exists for one execution, then deleted in Phase 4.
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional:**
|
||||
- Admin endpoint `POST /admin/migrate-from-upstash` reads full canvas from Upstash via existing `getFullCanvas` (kept in `lib/legacy-upstash-storage.js` for this phase) and forwards bytes to DO `/import`.
|
||||
- DO `POST /import` accepts a 16 MB body, splits into 256 chunks, INSERT OR REPLACE all rows in single transaction.
|
||||
- Endpoint is gated by a `MIGRATION_TOKEN` env var; reject if missing/wrong.
|
||||
- Idempotent: running twice yields the same final state.
|
||||
|
||||
**Non-functional:**
|
||||
- Migration completes in <30s (well under DO timeout).
|
||||
- Pre-flight checks: refuse to run if `canvas_chunks` already non-empty (unless `?force=1`).
|
||||
- Verification step: known coordinate roundtrip post-migration.
|
||||
|
||||
## Architecture
|
||||
|
||||
### Endpoint flow
|
||||
|
||||
```
|
||||
curl -X POST $WORKER_URL/admin/migrate-from-upstash \
|
||||
-H "Authorization: Bearer $MIGRATION_TOKEN"
|
||||
```
|
||||
|
||||
Worker:
|
||||
1. Verify `Authorization` header matches `c.env.MIGRATION_TOKEN`.
|
||||
2. Read full canvas from Upstash via legacy module (existing 4-chunk GETRANGE).
|
||||
3. POST raw bytes to DO `/import`.
|
||||
4. Verify by reading back a known coordinate from DO and comparing.
|
||||
5. Return JSON `{imported_bytes, sample_check_passed}`.
|
||||
|
||||
DO `/import`:
|
||||
1. Read body as `Uint8Array` (length must equal `TOTAL_PIXELS`).
|
||||
2. Pre-flight: if any rows exist and no `?force=1`, return 409.
|
||||
3. Open transaction; for each chunk index 0..CHUNK_COUNT-1, slice the buffer, `INSERT OR REPLACE INTO canvas_chunks`.
|
||||
4. Commit. Return 200.
|
||||
|
||||
### Files
|
||||
|
||||
```
|
||||
src/lib/legacy-upstash-storage.js ← copy of old canvas-storage.js, used only by migration
|
||||
src/admin/migrate.js ← admin route handler
|
||||
src/worker.js ← mount admin route under /admin/*
|
||||
```
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Create:**
|
||||
- `src/lib/legacy-upstash-storage.js` (renamed from old `lib/canvas-storage.js` — keep Upstash dependency until Phase 4)
|
||||
- `src/admin/migrate.js`
|
||||
- `src/durable-objects/lib/migration-import.js` (DO-side `/import` handler logic)
|
||||
|
||||
**Modify:**
|
||||
- `src/worker.js` — mount `/admin/*` route, add MIGRATION_TOKEN check
|
||||
- `src/durable-objects/canvas-room.js` — handle `/import` path
|
||||
- `wrangler.json` — add `MIGRATION_TOKEN` to `vars` placeholder (real value via secret)
|
||||
|
||||
**Delete:** None this phase.
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Copy current `src/lib/canvas-storage.js` (Upstash version) → `src/lib/legacy-upstash-storage.js`. Rename exports if needed to avoid clash.
|
||||
2. Add `wrangler secret put MIGRATION_TOKEN` (random 32 bytes hex).
|
||||
3. Implement `src/admin/migrate.js`:
|
||||
- Auth check.
|
||||
- Read all bytes via `legacy-upstash-storage.getFullCanvas(env)`.
|
||||
- POST to `room.fetch('http://internal/import', {method:'POST', body: bytes})`.
|
||||
- Sample check: pick a coordinate that's likely set (e.g., `(0,0)`, `(2000,2000)`), read post-migration DO canvas, compare.
|
||||
- Return JSON with stats.
|
||||
4. Implement DO `/import` path in `canvas-room.js` using `migration-import.js`:
|
||||
- Validate body length.
|
||||
- Pre-flight check (refuse if non-empty unless force).
|
||||
- Transactional bulk insert.
|
||||
5. Local test: seed local Upstash with known pattern (test pixel at known coords), run migration against local wrangler dev, verify pixel readable from new DO.
|
||||
6. Production run:
|
||||
- Deploy current branch to staging or use `wrangler dev --remote`.
|
||||
- `curl -X POST -H "Authorization: Bearer $TOKEN" $URL/admin/migrate-from-upstash`.
|
||||
- Verify `sample_check_passed: true`.
|
||||
- Browser smoke test: visible canvas matches pre-migration.
|
||||
7. **Do not delete Upstash data yet.** Keep for 7 days as rollback safety.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] Migration endpoint requires valid `MIGRATION_TOKEN`
|
||||
- [ ] Local migration: canvas_chunks count == CHUNK_COUNT after import
|
||||
- [ ] Local migration: sample pixel at known coordinate matches pre-migration value
|
||||
- [ ] Idempotency: re-running with `?force=1` yields same state
|
||||
- [ ] Production migration successful (run once, log retained)
|
||||
- [ ] Pre-migration screenshot vs post-migration screenshot identical (visual diff)
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| Migration corrupts canvas | Pre-flight refuses to overwrite unless `?force=1`; keep Upstash data 7 days |
|
||||
| Token leaked in logs | Use `Authorization` header (less likely logged than query string); rotate after migration |
|
||||
| Body size 16 MB exceeds worker request limit | CF Workers request body cap is 100 MB on free tier; 16 MB fits |
|
||||
| Partial migration (network failure mid-way) | Transactional bulk insert in DO; either all chunks committed or none |
|
||||
| Sample-check coordinate happens to be unset (0,0) | Use multiple sample coordinates; require ≥1 known-set coordinate to pass |
|
||||
@@ -1,120 +0,0 @@
|
||||
---
|
||||
phase: 4
|
||||
title: "Cleanup & Dependency Removal"
|
||||
status: completed
|
||||
priority: P2
|
||||
effort: "2h"
|
||||
dependencies: [3]
|
||||
---
|
||||
|
||||
# Phase 4: Cleanup & Dependency Removal
|
||||
|
||||
## Overview
|
||||
|
||||
After successful migration, delete all Upstash code paths, drop the `@upstash/redis` and `ioredis` dependencies, remove the migration endpoint, and rewrite the test suite to target DO storage.
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional:**
|
||||
- Zero Upstash references in `src/`.
|
||||
- `package.json` no longer depends on `@upstash/redis` or `ioredis`.
|
||||
- `wrangler.json` and `.env` no longer reference Upstash secrets.
|
||||
- `MIGRATION_TOKEN` secret deleted.
|
||||
- All tests pass against DO storage (no testcontainers Redis).
|
||||
|
||||
**Non-functional:**
|
||||
- No dead code left behind.
|
||||
- `npm run dev` works without `UPSTASH_REDIS_*` env vars set.
|
||||
|
||||
## Architecture
|
||||
|
||||
### Files going away
|
||||
|
||||
```
|
||||
src/lib/canvas-storage.js ← already orphaned by Phase 2
|
||||
src/lib/legacy-upstash-storage.js ← created in Phase 3, now removed
|
||||
src/lib/redis-client.js
|
||||
src/lib/rate-limiter.js
|
||||
src/admin/migrate.js
|
||||
src/durable-objects/lib/migration-import.js
|
||||
test/... ← any testcontainers Redis tests
|
||||
vitest.integration.config.js ← if it only existed for Redis
|
||||
```
|
||||
|
||||
### Test rewrite
|
||||
|
||||
Replace `testcontainers` Redis fixtures with Wrangler `unstable_dev` or Vitest `@cloudflare/vitest-pool-workers`.
|
||||
|
||||
```js
|
||||
// test/integration/canvas-do.test.js
|
||||
import { unstable_dev } from 'wrangler';
|
||||
|
||||
let worker;
|
||||
beforeAll(async () => {
|
||||
worker = await unstable_dev('src/worker.js', { local: true, persist: false });
|
||||
});
|
||||
afterAll(() => worker?.stop());
|
||||
|
||||
test('place pixel persists across reads', async () => {
|
||||
await worker.fetch('/api/place', { method:'POST', body: JSON.stringify({pixels:[{x:10,y:20,color:5}]}) });
|
||||
const res = await worker.fetch('/api/canvas');
|
||||
const buf = new Uint8Array(await res.arrayBuffer());
|
||||
expect(buf[20 * CANVAS_WIDTH + 10]).toBe(5);
|
||||
});
|
||||
```
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Delete:**
|
||||
- `src/lib/canvas-storage.js`
|
||||
- `src/lib/legacy-upstash-storage.js`
|
||||
- `src/lib/redis-client.js`
|
||||
- `src/lib/rate-limiter.js`
|
||||
- `src/admin/migrate.js`
|
||||
- `src/durable-objects/lib/migration-import.js`
|
||||
- Any test file that imports from above
|
||||
|
||||
**Modify:**
|
||||
- `package.json` — remove `@upstash/redis`, `ioredis`, `testcontainers` (if only used for Redis)
|
||||
- `package-lock.json` — regenerate via `npm install`
|
||||
- `wrangler.json` — remove any Upstash env var references
|
||||
- `vitest.config.js` / `vitest.integration.config.js` — drop Redis-specific setup
|
||||
- `src/worker.js` — remove `/admin/*` mount
|
||||
|
||||
**Create:**
|
||||
- `test/integration/canvas-do.test.js` — replaces `testcontainers` Redis tests
|
||||
- `test/integration/cooldown-do.test.js` — rate-limit semantics against DO
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Verify Phase 3 production migration successful and ≥7 days have passed (rollback window).
|
||||
2. Delete the file list above.
|
||||
3. `npm uninstall @upstash/redis ioredis testcontainers`.
|
||||
4. Search-and-destroy: `grep -r -i "upstash\|redis\|cooldown:.*ttl" src/ test/` — must return zero hits.
|
||||
5. Rewrite tests:
|
||||
- `test/integration/canvas-do.test.js` — pixel placement, batch, validation, full canvas read.
|
||||
- `test/integration/cooldown-do.test.js` — rate-limit window, retry-after value.
|
||||
- Delete `vitest.integration.config.js` if no longer needed (or simplify).
|
||||
6. Run `npm run test:all` — all green.
|
||||
7. Run `npm run dev` — verify no Upstash env-var errors at startup.
|
||||
8. `wrangler secret delete UPSTASH_REDIS_REST_URL` and `UPSTASH_REDIS_REST_TOKEN` and `MIGRATION_TOKEN` in production.
|
||||
9. Update `.env.example` — remove Upstash entries, leave only `MIGRATION_TOKEN` placeholder commented as historical (or delete entirely).
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] `grep -r -i "upstash" src/ test/` returns no matches
|
||||
- [ ] `grep -r "redis" src/ test/` returns no matches (case-insensitive)
|
||||
- [ ] `package.json` has no `@upstash/redis`, `ioredis`
|
||||
- [ ] `npm run test:all` green
|
||||
- [ ] `npm run dev` starts without Upstash env vars set
|
||||
- [ ] Production secrets deleted
|
||||
- [ ] `.env.example` clean
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| Premature deletion before migration verified | Wait 7 days post-Phase-3 deploy before this phase |
|
||||
| Test rewrite drops coverage | Match test cases 1:1 with deleted Upstash tests; review diff |
|
||||
| Forgotten Upstash reference somewhere | Final grep across full repo, not just src/ |
|
||||
| Wrangler `unstable_dev` API changes | Pin wrangler version in `devDependencies`; document in README |
|
||||
@@ -1,96 +0,0 @@
|
||||
---
|
||||
phase: 5
|
||||
title: "Deploy & Documentation"
|
||||
status: pending
|
||||
priority: P3
|
||||
effort: "1h"
|
||||
dependencies: [4]
|
||||
---
|
||||
|
||||
# Phase 5: Deploy & Documentation
|
||||
|
||||
## Overview
|
||||
|
||||
Production rollout of the cleaned-up codebase. Update architecture docs to reflect single-DO-with-storage. Document the resize procedure so future canvas growth is one constant change + redeploy.
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional:**
|
||||
- Production deploy of post-Phase-4 code with WS broadcast verified.
|
||||
- `docs/system-architecture.md` updated.
|
||||
- New doc: `docs/canvas-resize-procedure.md`.
|
||||
- README architecture diagram updated.
|
||||
|
||||
**Non-functional:**
|
||||
- Smoke test under realistic load (manual: 2 browsers, 30 placements/min for 5 min).
|
||||
- 7-day post-deploy observation: zero Upstash refs in logs, $0 bill confirmed.
|
||||
|
||||
## Architecture
|
||||
|
||||
Documentation updates only — no code change.
|
||||
|
||||
### Resize procedure (the headline doc)
|
||||
|
||||
```
|
||||
1. Edit src/lib/constants.js:
|
||||
CANVAS_WIDTH = <new> // must be multiple of CHUNK_BYTES width factor
|
||||
CANVAS_HEIGHT = <new>
|
||||
2. Verify new TOTAL_PIXELS / CHUNK_BYTES gives integer (or accept partial last chunk).
|
||||
3. npm run build
|
||||
4. npm run deploy
|
||||
5. New chunks lazy-init on first read (zero-fill).
|
||||
6. Existing pixels remain at original (x,y) coordinates; effective canvas just becomes larger.
|
||||
```
|
||||
|
||||
**Caveat:** shrinking the canvas truncates pixels outside new bounds. They're still in SQLite (orphan rows) — clean up by manually `DELETE FROM canvas_chunks WHERE chunk_id >= NEW_CHUNK_COUNT` via DO admin endpoint if needed (out of scope).
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Modify:**
|
||||
- `docs/system-architecture.md` — replace Upstash mentions with DO SQLite layout
|
||||
- `README.md` — update Tech Stack table (drop Upstash row), update architecture ASCII diagram, update Setup section (remove Upstash steps)
|
||||
|
||||
**Create:**
|
||||
- `docs/canvas-resize-procedure.md`
|
||||
|
||||
**Delete:** None.
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. `npm run deploy` to production.
|
||||
2. Manual smoke test:
|
||||
- Open 2 browser tabs.
|
||||
- Place a pixel in tab A → confirm appears in tab B within 1s (WS broadcast).
|
||||
- Refresh tab B → canvas reflects placed pixel (DO storage persistence).
|
||||
- Spam-place in tab A → confirm 429 after 1st in same second (rate limit).
|
||||
3. Check Cloudflare dashboard:
|
||||
- Workers requests over past 24h within free tier.
|
||||
- DO storage size reported.
|
||||
- No errors in logs.
|
||||
4. Update `README.md`:
|
||||
- Tech Stack table: replace `Upstash Redis` row with `Cloudflare DO (SQLite)`.
|
||||
- Architecture diagram: drop Upstash node, show DO contains canvas + cooldown.
|
||||
- Setup section: drop `wrangler secret put UPSTASH_REDIS_*` lines.
|
||||
5. Update `docs/system-architecture.md` with new component diagram, DO storage schema, request flow.
|
||||
6. Write `docs/canvas-resize-procedure.md` (~30 lines max, KISS).
|
||||
7. Tag a release: `v2.0.0-do-storage` (breaking change in deployment requirements).
|
||||
8. 7-day check: revisit dashboard, confirm $0 bill.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] Production deploy successful, smoke test passes
|
||||
- [ ] Manual broadcast test green (2-tab placement)
|
||||
- [ ] CF dashboard shows zero Upstash-related env vars
|
||||
- [ ] README + system-architecture reflect new architecture
|
||||
- [ ] `docs/canvas-resize-procedure.md` exists and is testable (someone can follow it)
|
||||
- [ ] Release tagged `v2.0.0-do-storage`
|
||||
- [ ] 7-day observation: $0 bill, no error spike
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| Production WS regression unnoticed | Manual 2-tab test in smoke; consider scripted Playwright test in future |
|
||||
| Docs drift from code over time | Resize procedure intentionally short and tied to constants file |
|
||||
| Free tier breach after deploy under unexpected load | Existing `s-maxage=10` cache absorbs; CF dashboard alarm thresholds optional |
|
||||
| Breaking change to deployment (no Upstash needed) | Major version bump (v2.0.0) signals it; release notes call it out |
|
||||
@@ -1,87 +0,0 @@
|
||||
---
|
||||
title: "Migrate canvas storage from Upstash to Durable Object SQLite"
|
||||
status: in-progress
|
||||
priority: P2
|
||||
created: 2026-05-09
|
||||
phases: 5
|
||||
source: brainstorm
|
||||
brainstorm: ../reports/brainstorm-260509-2309-canvas-on-do-storage.md
|
||||
researchReports:
|
||||
- ../reports/researcher-260509-2255-forever-free-hosting.md
|
||||
- ../reports/researcher-260509-2246-vercel-migration-feasibility.md
|
||||
deployment:
|
||||
productionUrl: https://rplace.miti99.workers.dev
|
||||
migratedAt: 2026-05-10
|
||||
postCleanupVersionId: 34cd1900-dd16-47f1-abae-8d195b076b0a
|
||||
---
|
||||
|
||||
# Plan: Migrate canvas storage from Upstash → Durable Object SQLite
|
||||
|
||||
## Goal
|
||||
|
||||
Eliminate Upstash Redis dependency. Move canvas (16 MB) and per-user cooldown state into the existing `CanvasRoom` Durable Object via its SQLite-backed storage. Keep $0/month forever, hobby-scale (50 concurrent / <1 placement/sec). Resize must be config-change-only.
|
||||
|
||||
## Context
|
||||
|
||||
- Brainstorm: `plans/reports/brainstorm-260509-2309-canvas-on-do-storage.md` (approved)
|
||||
- Storage research: `plans/reports/researcher-260509-2255-forever-free-hosting.md`
|
||||
- Migration feasibility: `plans/reports/researcher-260509-2246-vercel-migration-feasibility.md`
|
||||
|
||||
## Architecture (locked)
|
||||
|
||||
```
|
||||
Browser ──HTTP/WS──▶ Worker (Hono, thin proxy)
|
||||
│
|
||||
└─▶ CanvasRoom DO (idFromName('main'))
|
||||
├── canvas_chunks (SQLite BLOB rows × CHUNK_COUNT)
|
||||
├── cooldowns (user_id → expires_at, lazy GC)
|
||||
└── WebSocket hibernation hub (broadcast)
|
||||
```
|
||||
|
||||
Constants drive resize: `CHUNK_COUNT = ceil(CANVAS_WIDTH * CANVAS_HEIGHT / CHUNK_BYTES)`. Bump width/height → redeploy → DO lazy-inits missing chunks.
|
||||
|
||||
## Phases
|
||||
|
||||
| # | Phase | Status | Effort |
|
||||
|---|---|---|---|
|
||||
| 1 | [Storage Foundation](phase-01-storage-foundation.md) | completed | ~3h |
|
||||
| 2 | [DO Integration & Worker Proxy](phase-02-do-integration-worker-proxy.md) | completed | ~4h |
|
||||
| 3 | [One-Shot Upstash Migration](phase-03-one-shot-upstash-migration.md) | completed | ~2h |
|
||||
| 4 | [Cleanup & Dependency Removal](phase-04-cleanup-dependency-removal.md) | completed | ~2h |
|
||||
| 5 | [Deploy & Documentation](phase-05-deploy-documentation.md) | partial (deploy done, READMEs done, 7-day observation pending) | ~1h |
|
||||
|
||||
**Total estimate:** ~12h (1.5 working days)
|
||||
**Status:** Phases 1–4 complete. Phase 5 done except 7-day post-deploy observation window.
|
||||
**Production:** https://rplace.miti99.workers.dev — version 34cd1900, Upstash fully removed.
|
||||
|
||||
## Dependencies
|
||||
|
||||
Phase order is strict: 1 → 2 → 3 → 4 → 5. Each phase blocks the next.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] Zero `@upstash/redis` references in `src/`
|
||||
- [ ] `package.json` no longer depends on `@upstash/redis` or `ioredis`
|
||||
- [ ] `wrangler dev` runs locally without `UPSTASH_REDIS_*` env vars
|
||||
- [ ] Existing pixels preserved through migration (verify a known coordinate)
|
||||
- [ ] WS broadcast still functional end-to-end
|
||||
- [ ] All tests pass (`npm run test:all`)
|
||||
- [ ] Resize procedure documented in `docs/`
|
||||
- [ ] Production deploy verified with smoke test
|
||||
- [ ] $0 monthly bill confirmed after 7 days
|
||||
|
||||
## Risk Register
|
||||
|
||||
| Risk | Severity | Mitigation |
|
||||
|---|---|---|
|
||||
| Workers req/day at 87% of cap on peak day | Med | Keep `s-maxage=10` cache on `/api/canvas`; monitor via CF dashboard |
|
||||
| Test rewrite scope creep | Low | Time-box testcontainers→DO test rewrite to 4h; cut e2e if needed |
|
||||
| Migration corrupts canvas | High | Keep Upstash data 7 days post-migration as rollback |
|
||||
| DO single-region latency regression | Low | Same as today's Upstash (single region) — no change |
|
||||
| Storage exposure if canvas grows | Low | Per-DO 10 GB / per-account 5 GB on Free; current 16 MB has 300× headroom |
|
||||
| BLOB row size cap (2 MB) | None | `CHUNK_BYTES = 64 KB` has 32× headroom from this limit |
|
||||
| Per-DO 1,000 req/sec soft cap | None | Hobby load is ~1 req/sec; edge cache absorbs `/api/canvas` |
|
||||
|
||||
## Rollback Plan
|
||||
|
||||
Phases 1–4 are reversible until Phase 5 deploy. Keep Upstash creds and old code paths in git history for 30 days. If post-deploy issues, `git revert` + redeploy.
|
||||
@@ -1,151 +0,0 @@
|
||||
---
|
||||
phase: 1
|
||||
title: "DO storage atomicity & correctness"
|
||||
status: completed
|
||||
priority: P1
|
||||
effort: "4h"
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Phase 1: DO Storage Atomicity & Correctness
|
||||
|
||||
## Overview
|
||||
Fix the four correctness bugs in the Durable Object storage path: non-atomic multi-chunk writes, BLOB-grow data loss after canvas resize, cooldown burned on storage failure, and orphan-row crash on `/api/canvas`. Sanitize the 500 response body. Replace the bypassable `content-length` cap.
|
||||
|
||||
## Context Links
|
||||
- Reports: `plans/reports/code-reviewer-260510-0211-rplace-do-migration.md` (C3, H1, H2, H3, H5)
|
||||
- Reports: `plans/reports/debugger-260510-0211-rplace-edge-cases.md` (C1, C2, M5)
|
||||
|
||||
## Key Insights
|
||||
- "No `await` between statements" ≠ atomic. Each `sql.exec` auto-commits unless wrapped in `state.storage.transactionSync(...)`.
|
||||
- `new Uint8Array(typedArray)` copies but preserves the source's length, so growing a short BLOB requires explicit re-allocation against `chunkSize(chunkId)`.
|
||||
- Cooldown UPDATE must roll back if the subsequent write throws — otherwise transient storage flakes silently halve image-uploader throughput.
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional**
|
||||
- Multi-chunk batch writes atomic: all chunks commit, or none do.
|
||||
- After `CANVAS_WIDTH`/`CANVAS_HEIGHT` grow + redeploy, new pixels in the formerly-last chunk persist correctly.
|
||||
- Cooldown row reverted when `writePixels` throws.
|
||||
- `GET /api/canvas` returns 200 even with orphan `chunk_id ≥ CHUNK_COUNT` rows present (post-shrink).
|
||||
- `POST /api/place` rejects requests with missing or zero `content-length` (or reads body with hard byte cap).
|
||||
- 500 responses do not echo raw error message.
|
||||
|
||||
**Non-functional**
|
||||
- No regression in happy-path latency (single-chunk batch).
|
||||
- Static assertion `CHUNK_BYTES <= 2_000_000` near constants to prevent future BLOB-cell overflow.
|
||||
|
||||
## Architecture
|
||||
|
||||
```
|
||||
worker.js (edge)
|
||||
├─ guard: content-length present AND > 0 AND ≤ MAX_BODY_BYTES (else 413)
|
||||
└─ forwards to DO
|
||||
|
||||
CanvasRoom.placePixels (canvas-room.js)
|
||||
├─ tryAcquire(userId) → reserves cooldown
|
||||
├─ TRY:
|
||||
│ state.storage.transactionSync(() => writePixels(sql, edits))
|
||||
│ broadcast(edits)
|
||||
└─ CATCH:
|
||||
sql.exec('DELETE FROM cooldowns WHERE user_id = ?', userId) // rollback
|
||||
return Response.json({ error: 'storage_failed' }, { status: 500 })
|
||||
// no message field; full err logged separately
|
||||
```
|
||||
|
||||
`chunk-storage.writePixels` rewritten so `next` is sized to `chunkSize(chunkId)`, with `buf.subarray(0, min(buf.length, expected))` copied into it.
|
||||
|
||||
`chunk-storage.readAllChunks` filters `WHERE chunk_id < ?` bound to `CHUNK_COUNT`.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Modify**
|
||||
- `src/worker.js` — content-length validator (require > 0 OR switch to bounded `arrayBuffer`)
|
||||
- `src/durable-objects/canvas-room.js` — wrap write in transactionSync, rollback cooldown on error, drop `message` from 500
|
||||
- `src/durable-objects/lib/chunk-storage.js` — fix `writePixels` BLOB-grow, bound `readAllChunks`, replace `new Uint8Array(buf)` aliasing comment with `.slice()` or explicit allocation
|
||||
- `src/lib/constants.js` — add static assertion for `CHUNK_BYTES`
|
||||
- `src/durable-objects/canvas-room.js:119` — fix the inverted "pre-2026-04-07" comment (M2 in code-review)
|
||||
|
||||
**Create** — none
|
||||
|
||||
**Delete** — none
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. **`chunk-storage.writePixels` BLOB-grow fix** (review C3)
|
||||
- In the per-chunk loop, compute `expected = chunkSize(chunkId)`.
|
||||
- `const next = new Uint8Array(expected); next.set(buf.subarray(0, Math.min(buf.length, expected)));`
|
||||
- Update inline comment: "size against chunkSize, never trust persisted blob length".
|
||||
|
||||
2. **`chunk-storage.writePixels` atomicity** (debugger C2)
|
||||
- Wrap the entire `for (const [chunkId, edits] of grouped)` block in `state.storage.transactionSync(() => { ... })`.
|
||||
- Verify `transactionSync` exists at compat date `2025-04-01`; if not, fall back to async `state.storage.transaction(async () => {...})` and propagate rejection.
|
||||
- Update the comment at L86-87 to describe the transactional invariant, not "no await".
|
||||
|
||||
3. **`chunk-storage.readAllChunks` orphan guard** (review H3)
|
||||
- Change SQL to `SELECT chunk_id, bytes FROM canvas_chunks WHERE chunk_id < ?` bound to `CHUNK_COUNT`.
|
||||
- Optional secondary defense: skip rows where `chunkId * CHUNK_BYTES + view.length > out.length`.
|
||||
|
||||
4. **`canvas-room.placePixels` cooldown rollback** (review H1, debugger C1)
|
||||
- Add try/catch around the `writePixels` call.
|
||||
- In catch: `sql.exec('DELETE FROM cooldowns WHERE user_id = ?', userId)`. Rethrow or return 500.
|
||||
- Add a one-line comment: `// refund cooldown so transient storage errors don't soft-DOS user`.
|
||||
|
||||
5. **500 response sanitization** (review H2)
|
||||
- Drop `message: String(err)` from the response JSON.
|
||||
- Keep the `console.error` call so debugging info stays in logs.
|
||||
|
||||
6. **`worker.js` content-length guard** (review H5)
|
||||
- Require `content-length` header present AND > 0 AND ≤ `MAX_BODY_BYTES`. Reject with 411 (length required) or 413 (too large).
|
||||
- Alternative: read raw body via `c.req.arrayBuffer()`, check `byteLength`, then `JSON.parse(decoder.decode(buf))`.
|
||||
|
||||
7. **Static `CHUNK_BYTES` cap assertion** (debugger M5)
|
||||
- At top of `constants.js` (or in a small init): `if (CHUNK_BYTES > 2_000_000) throw new Error('CHUNK_BYTES exceeds DO SQLite cell limit');`
|
||||
- Document compat-date-versioned cell limit in the comment.
|
||||
|
||||
8. **WS-close comment correction** (review M2)
|
||||
- `canvas-room.js:119` — replace misleading comment with: `// Required because compatibility_date 2025-04-01 predates the 2026-04-07 default-close cutoff. Remove if/when wrangler.json bumps past that date.`
|
||||
- Add try/catch around the `ws.close(...)` call to handle already-closed sockets (debugger M7).
|
||||
|
||||
9. **Compile + smoke**
|
||||
- `npm run build` — must pass.
|
||||
- Local `wrangler dev`, hit `POST /api/place` with a 2-pixel batch spanning 2 chunks; verify success.
|
||||
- Force a write error (e.g. temporarily throw inside `writePixels`); verify cooldown row deleted, 500 returned without raw message.
|
||||
- Hit `GET /api/canvas` after manually inserting an orphan row; verify 200.
|
||||
|
||||
## Todo List
|
||||
|
||||
- [ ] Implement BLOB-grow fix in `chunk-storage.writePixels`
|
||||
- [ ] Wrap multi-chunk write in `transactionSync` (or async fallback)
|
||||
- [ ] Bound `readAllChunks` query by `chunk_id < CHUNK_COUNT`
|
||||
- [ ] Add cooldown rollback in `canvas-room.placePixels` catch path
|
||||
- [ ] Drop `message` from 500 response
|
||||
- [ ] Replace `content-length` guard with present+nonzero check or bounded body read
|
||||
- [ ] Add `CHUNK_BYTES <= 2_000_000` static assertion
|
||||
- [ ] Fix inverted WS-close comment + add try/catch
|
||||
- [ ] `npm run build` passes
|
||||
- [ ] Manual smoke: 2-chunk batch success
|
||||
- [ ] Manual smoke: forced write error → cooldown refunded, no message leak
|
||||
- [ ] Manual smoke: orphan row → `/api/canvas` 200
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] No regressions in existing 94-test suite
|
||||
- [ ] `transactionSync` (or async transaction) confirmed available at compat date 2025-04-01
|
||||
- [ ] All bullets in Todo List checked
|
||||
- [ ] PR includes file:line evidence linking each change to the originating finding
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Risk:** `transactionSync` not available at our compat date → fix back-compat.
|
||||
**Mitigation:** Verify via `wrangler` runtime docs first; fall back to `state.storage.transaction(async () => {...})` and ensure `placePixels` is awaited end-to-end.
|
||||
- **Risk:** Cooldown rollback DELETE race against concurrent insert from same user.
|
||||
**Mitigation:** Single-DO is single-threaded per request; rollback runs synchronously before response. No lock needed.
|
||||
- **Risk:** Bounded body read changes Hono request-handling shape; could break the existing tests.
|
||||
**Mitigation:** Keep `c.req.json()` if guard is sufficient; only refactor to `arrayBuffer()` if guard alone leaves a gap.
|
||||
|
||||
## Security Considerations
|
||||
|
||||
- 500 sanitization closes a low-severity info-leak.
|
||||
- content-length guard removes a DOS-amplification vector against the JSON parser.
|
||||
- Atomicity fix prevents broadcast/persistence divergence that could be probed for state inference.
|
||||
@@ -1,153 +0,0 @@
|
||||
---
|
||||
phase: 2
|
||||
title: "Cookie+IP identity & broadcast sequence numbers"
|
||||
status: completed
|
||||
priority: P1
|
||||
effort: "3h"
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Phase 2: Cookie+IP Identity & Broadcast Sequence Numbers
|
||||
|
||||
## Overview
|
||||
Replace IP-only rate-limit identity with cookie-preferred / IP-fallback to unblock NAT/CGNAT/mobile users. Add monotonically-increasing sequence numbers to WebSocket broadcast frames so clients can detect missed pixels and refetch.
|
||||
|
||||
## Context Links
|
||||
- Reports: `plans/reports/debugger-260510-0211-rplace-edge-cases.md` (C3 NAT, H2 dev bucket, H3 hibernation gap)
|
||||
- Reports: `plans/reports/code-reviewer-260510-0211-rplace-do-migration.md` (M4 dev bucket)
|
||||
|
||||
## Key Insights
|
||||
- Cookie identity isn't a security boundary — it's a usability fix to break NAT collisions. Trivially defeated by clearing cookies; that's acceptable scope.
|
||||
- Broadcast sequence numbers don't require persistence: a per-DO instance counter works because reconnects after hibernation refetch the canvas anyway.
|
||||
- `cf-connecting-ip` missing in production is an alarm condition, not a fallback path. Fail-closed in prod, soft-fall in dev.
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional**
|
||||
- First request without `rplace_id` cookie: server issues `Set-Cookie: rplace_id=<uuid>; HttpOnly; Secure; SameSite=Lax; Path=/; Max-Age=31536000`.
|
||||
- Subsequent requests with cookie: identity = `cookie:<uuid>`. Without cookie but with valid `cf-connecting-ip`: identity = `ip:<ip>`. Neither in prod: 500 with `{ error: "no_identity" }`. Neither in dev: identity = `dev:<random-per-process>`.
|
||||
- WS broadcast frames carry monotonic `seq` field (uint32, wraps at 2^32).
|
||||
- Client tracks last-seen `seq`; on gap (`seq != lastSeq + 1`) triggers `refetchCanvas()`.
|
||||
|
||||
**Non-functional**
|
||||
- Cookie issuance adds zero latency (Set-Cookie on existing 200 response).
|
||||
- No new dependencies.
|
||||
|
||||
## Architecture
|
||||
|
||||
```
|
||||
src/lib/get-user-id.js
|
||||
resolveIdentity(request, env):
|
||||
if (cookie 'rplace_id' present) → "cookie:" + uuid
|
||||
else if (cf-connecting-ip present) → "ip:" + sha256(ip).slice(0,16)
|
||||
else if (env is production) → throw NoIdentityError → caller maps to 500
|
||||
else → "dev:<isolate-stable-uuid>"
|
||||
|
||||
needsCookieIssue(request) → true if no rplace_id cookie
|
||||
|
||||
src/worker.js
|
||||
/api/canvas handler:
|
||||
if needsCookieIssue → response.headers.append('Set-Cookie', issueCookie(uuid))
|
||||
|
||||
src/durable-objects/canvas-room.js
|
||||
state #seq = 0 (in-memory, resets on hibernation rehydrate)
|
||||
broadcast(edits):
|
||||
#seq = (#seq + 1) >>> 0
|
||||
payload = { type: 'pixels', seq: #seq, edits }
|
||||
|
||||
src/client/App.svelte
|
||||
ws.onmessage:
|
||||
if (msg.seq != null) {
|
||||
if (lastSeq != null && msg.seq !== ((lastSeq + 1) >>> 0)) {
|
||||
refetchCanvas() // gap detected
|
||||
}
|
||||
lastSeq = msg.seq
|
||||
}
|
||||
```
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Modify**
|
||||
- `src/lib/get-user-id.js` — cookie-first resolution + production fail-closed
|
||||
- `src/worker.js` — issue cookie on `/api/canvas` when missing; pass identity into DO
|
||||
- `src/durable-objects/canvas-room.js` — instance `#seq` counter; include in broadcast frames
|
||||
- `src/client/App.svelte` — track `lastSeq`, refetch on gap
|
||||
- `src/client/components/CanvasRenderer.svelte` — accept `seq`-aware events if needed (likely not)
|
||||
|
||||
**Create**
|
||||
- `src/lib/cookie.js` — minimal helpers: `parseCookie(header)`, `formatSetCookie(name, value, opts)`. ~30 lines, kebab-case per code standards.
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. **Cookie helpers** (`src/lib/cookie.js`)
|
||||
- `parseCookie(header) → Map<name, value>` — handle missing header, malformed pairs.
|
||||
- `formatSetCookie(name, value, { httpOnly, secure, sameSite, path, maxAge })` → string.
|
||||
- Export both as named exports.
|
||||
- Add tests in `test/lib/cookie.test.js` (Phase 4).
|
||||
|
||||
2. **Identity resolution rewrite** (`get-user-id.js`)
|
||||
- Import `parseCookie`.
|
||||
- `resolveIdentity(request, env)` returns `{ id, issueCookie?: { name, value } }`.
|
||||
- Production path: if no cookie AND no `cf-connecting-ip` → throw `NoIdentityError`.
|
||||
- Dev path: stable `dev:<uuid>` per-isolate (cache module-level).
|
||||
- When identity comes from `ip:`, mark `issueCookie = { name: 'rplace_id', value: crypto.randomUUID() }` so the worker can attach Set-Cookie even before the user has one.
|
||||
|
||||
3. **Worker integration** (`worker.js`)
|
||||
- On `/api/canvas` and `/api/place`: call `resolveIdentity(c.req.raw, c.env)`. If it throws `NoIdentityError`, return 500 `{ error: "no_identity" }`.
|
||||
- Pass `id` into the DO body / header.
|
||||
- On `/api/canvas` only: if `issueCookie` is set, append `Set-Cookie` header to the response.
|
||||
- On `/api/place`: don't issue cookies (POST shouldn't mutate cookies in this design).
|
||||
|
||||
4. **DO sequence counter** (`canvas-room.js`)
|
||||
- Add `#seq = 0` private field.
|
||||
- In `#broadcastPixels(edits)`: `this.#seq = (this.#seq + 1) >>> 0; const message = JSON.stringify({ type: 'pixels', seq: this.#seq, edits });`
|
||||
- Note: counter resets on DO hibernation rehydrate; that's fine — clients refetch on reconnect.
|
||||
|
||||
5. **Client gap detection** (`App.svelte`)
|
||||
- Track `let lastSeq = null;`.
|
||||
- In WS message handler: if `msg.seq != null && lastSeq != null && msg.seq !== ((lastSeq + 1) >>> 0)` → call `canvasRenderer.refetchCanvas()`. Always update `lastSeq = msg.seq`.
|
||||
- On reconnect: reset `lastSeq = null` (it's already a fresh canvas fetch).
|
||||
|
||||
6. **Compile + smoke**
|
||||
- `npm run build` passes.
|
||||
- Cookie issuance: open `wrangler dev`, GET `/api/canvas`, confirm `Set-Cookie` present. Subsequent request shows cookie.
|
||||
- Identity: 2 tabs, 1 cookie each → independent rate limits even on same IP.
|
||||
- Sequence gap: temporarily log `seq` on client; place pixel, confirm seq increments.
|
||||
|
||||
## Todo List
|
||||
|
||||
- [ ] Create `src/lib/cookie.js` (parseCookie + formatSetCookie)
|
||||
- [ ] Refactor `src/lib/get-user-id.js` to cookie-first / IP-fallback / fail-closed
|
||||
- [ ] Worker issues `Set-Cookie` on `/api/canvas` when missing
|
||||
- [ ] Worker forwards resolved identity to DO
|
||||
- [ ] DO maintains `#seq` counter; broadcast includes `seq`
|
||||
- [ ] Client tracks `lastSeq`, refetches on gap
|
||||
- [ ] Client resets `lastSeq` on reconnect
|
||||
- [ ] `npm run build` passes
|
||||
- [ ] Manual smoke: cookie issued, identity changes per cookie
|
||||
- [ ] Manual smoke: seq gap → client refetches
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] Two browsers on the same IP can place pixels independently within their own 1Hz cooldown.
|
||||
- [ ] In production env (`env.ENVIRONMENT === 'production'`), missing both cookie and `cf-connecting-ip` returns 500 with `no_identity`.
|
||||
- [ ] Forced WS gap (drop a frame in dev console) triggers `refetchCanvas`.
|
||||
- [ ] No regression in existing tests.
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Risk:** Browser cookie blocking (private mode, strict tracker prevention) → falls back to IP, NAT users still collide.
|
||||
**Mitigation:** Acceptable per Q&A scope; document. Future plan can add HMAC-signed token for stricter envs.
|
||||
- **Risk:** Cookie collision if two users share a stolen cookie value (e.g., copy-paste curl).
|
||||
**Mitigation:** Cookies are HttpOnly — not exfiltrable from JS. The opaque UUID is high-entropy.
|
||||
- **Risk:** Seq counter wraparound after 2^32 broadcasts.
|
||||
**Mitigation:** ~136 years at 1 broadcast/sec. Wrap is deliberately handled via `>>> 0` so wrap doesn't trigger false gap.
|
||||
- **Risk:** Production fail-closed behind a misconfigured proxy denies all traffic.
|
||||
**Mitigation:** Add a `wrangler tail` check before deploy; if 500s spike, hotfix to fall back to a synthetic identity.
|
||||
|
||||
## Security Considerations
|
||||
|
||||
- Cookie is `HttpOnly`, `Secure`, `SameSite=Lax` — standard hygiene.
|
||||
- Cookie is opaque (UUID v4) — no PII, no prediction.
|
||||
- Identity is used only for rate-limiting; no privilege gating, so cookie theft doesn't escalate.
|
||||
- No CSRF concern — POST `/api/place` doesn't depend on cookie identity for authorization, only for rate-limit bucketing. (Worth confirming in PR review.)
|
||||
@@ -1,153 +0,0 @@
|
||||
---
|
||||
phase: 3
|
||||
title: "WebSocket hardening & client race fix"
|
||||
status: completed
|
||||
priority: P1
|
||||
effort: "3h"
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Phase 3: WebSocket Hardening & Client Race Fix
|
||||
|
||||
## Overview
|
||||
Fix the WebSocket-during-initial-fetch race that silently drops pixels. Add Origin allowlist + per-identity connection cap on WS upgrade. Add a minimal heartbeat so dead connections fire `onclose` promptly.
|
||||
|
||||
## Context Links
|
||||
- Reports: `plans/reports/code-reviewer-260510-0211-rplace-do-migration.md` (C2 race, H4 conn cap, M3 heartbeat)
|
||||
- Reports: `plans/reports/debugger-260510-0211-rplace-edge-cases.md` (H5 origin/amplification, L6 conn cap, M3 heartbeat — duplicate)
|
||||
|
||||
## Key Insights
|
||||
- The pre-allocated `committedColors` zero array gets overwritten by the post-fetch `new Uint8Array(indices)` — any WS edits between WS-open and fetch-resolve are lost. Fix: buffer WS edits, replay after replacement.
|
||||
- CF DO `state.acceptWebSocket(ws, [tag1, tag2])` lets `getWebSockets(tag)` filter — perfect for per-identity caps.
|
||||
- Hibernation API may auto-ping at TCP level, but app-level heartbeat is cheaper insurance and gives clients a way to detect zombies.
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional**
|
||||
- WS messages received during the initial canvas fetch are applied (not dropped) once the fetch resolves.
|
||||
- WS upgrade rejected (403) if `Origin` header is present and not in allowlist.
|
||||
- WS upgrade rejected (429) if the requesting identity already has ≥ N (default 5) live sockets.
|
||||
- Server accepts `ping` text message and responds `pong`. Client sends ping every 30s; if no pong in 60s, closes WS to trigger reconnect.
|
||||
|
||||
**Non-functional**
|
||||
- Allowlist configurable via `wrangler.json` env vars (`ALLOWED_ORIGINS`, comma-separated).
|
||||
- Conn cap configurable (`MAX_WS_PER_IDENTITY`, default 5).
|
||||
|
||||
## Architecture
|
||||
|
||||
```
|
||||
Worker /api/ws
|
||||
├─ resolve identity (Phase 2 helper)
|
||||
├─ origin check: if Origin present AND not in env.ALLOWED_ORIGINS → 403
|
||||
└─ forward to DO with identity in header
|
||||
|
||||
CanvasRoom #handleWsUpgrade
|
||||
├─ existing = state.getWebSockets(identity)
|
||||
├─ if existing.length >= MAX_WS_PER_IDENTITY → 429
|
||||
├─ state.acceptWebSocket(server, [identity])
|
||||
└─ return 101
|
||||
|
||||
CanvasRoom webSocketMessage(ws, msg)
|
||||
├─ if msg === 'ping' → ws.send('pong'); return
|
||||
└─ else → ws.close(1003, 'unsupported message')
|
||||
|
||||
Client (CanvasRenderer.svelte loadCanvas)
|
||||
let pendingWsEdits = [];
|
||||
ws.onmessage during fetch → push to pendingWsEdits (don't apply)
|
||||
fetch resolves:
|
||||
committedColors = new Uint8Array(indices);
|
||||
apply pendingWsEdits to committedColors
|
||||
pendingWsEdits = null
|
||||
flag "live mode" — onmessage now applies directly
|
||||
|
||||
Client (App.svelte)
|
||||
setInterval(() => ws.send('ping'), 30_000)
|
||||
trackPongTimer; if no pong in 60s → ws.close()
|
||||
```
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Modify**
|
||||
- `src/worker.js` — origin check before WS upgrade forwarding
|
||||
- `src/durable-objects/canvas-room.js` — per-identity conn cap, ping handling, tag-aware acceptWebSocket
|
||||
- `src/client/components/CanvasRenderer.svelte` — buffer-and-replay during initial fetch (around lines 465–484)
|
||||
- `src/client/App.svelte` — ping interval, pong watchdog
|
||||
- `src/lib/constants.js` — add `MAX_WS_PER_IDENTITY = 5`
|
||||
- `wrangler.json` — add `vars: { ALLOWED_ORIGINS: "https://rplace.miti99.workers.dev" }`
|
||||
|
||||
**Create** — none
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. **Origin allowlist in worker** (debugger H5)
|
||||
- Read `env.ALLOWED_ORIGINS` (comma-separated). Parse to Set at module top.
|
||||
- In `/api/ws` handler: if `Origin` header present and not in allowlist, return `c.text('forbidden_origin', 403)`. Empty allowlist → allow all (dev default).
|
||||
- Document in `wrangler.json` comment.
|
||||
|
||||
2. **Per-identity WS cap in DO** (review H4, debugger L6)
|
||||
- In `#handleWsUpgrade(request, identity)`: `const existing = this.state.getWebSockets(identity);`
|
||||
- If `existing.length >= MAX_WS_PER_IDENTITY` → return `new Response('too_many_sockets', { status: 429 })`.
|
||||
- Replace `state.acceptWebSocket(server)` with `state.acceptWebSocket(server, [identity])`.
|
||||
|
||||
3. **Server-side heartbeat** (review M3, debugger M3)
|
||||
- In `webSocketMessage(ws, message)`: if `message === 'ping'` → `ws.send('pong'); return;`. Else keep current close behavior.
|
||||
- Note: this works under hibernation because messages auto-rehydrate the DO.
|
||||
|
||||
4. **Client buffer-and-replay** (review C2)
|
||||
- In `loadCanvas` (`CanvasRenderer.svelte:465-484`):
|
||||
- Add `let pendingWsEdits = [];` and `let isLive = false;` at top of `loadCanvas`.
|
||||
- Expose `pushWsEdit(edit)` from the component: if `!isLive` → `pendingWsEdits.push(edit)`; else apply directly.
|
||||
- After fetch resolves and `committedColors = new Uint8Array(indices)`, replay: for each `edit` in `pendingWsEdits`, write to `committedColors[edit.idx] = edit.color` AND update `imageData`. Then `isLive = true; pendingWsEdits = null;`.
|
||||
- In parent (`App.svelte`), route ws.onmessage pixel events to `canvasRenderer.pushWsEdit(...)` instead of applying directly when first connect.
|
||||
|
||||
5. **Client heartbeat** (review M3)
|
||||
- In `App.svelte` WS open handler: start `setInterval(() => ws.readyState === 1 && ws.send('ping'), 30_000)`. Track `lastPongAt = Date.now()`.
|
||||
- On message `'pong'`: `lastPongAt = Date.now()`.
|
||||
- Watchdog: if `Date.now() - lastPongAt > 60_000` → `ws.close()` to trigger reconnect logic.
|
||||
- Clear interval/watchdog on `onclose`.
|
||||
|
||||
6. **Compile + smoke**
|
||||
- `npm run build` passes.
|
||||
- Open dev console, throttle network to "Slow 3G", reload, place pixel from a second tab during fetch, confirm pixel appears in tab 1 once fetch completes.
|
||||
- Try opening 6 WS connections from same browser → 6th gets 429.
|
||||
- Try opening WS from a different origin (curl with `Origin: https://evil.example`) → 403.
|
||||
- Confirm `ping`/`pong` round-trips in dev console.
|
||||
|
||||
## Todo List
|
||||
|
||||
- [ ] Origin allowlist parsing + worker check
|
||||
- [ ] Per-identity WS cap (`MAX_WS_PER_IDENTITY`) with `acceptWebSocket(server, [identity])`
|
||||
- [ ] Server `ping` handler returns `pong`
|
||||
- [ ] `wrangler.json` `vars.ALLOWED_ORIGINS`
|
||||
- [ ] Client buffer-and-replay for WS during initial fetch
|
||||
- [ ] Client 30s ping / 60s pong watchdog
|
||||
- [ ] `npm run build` passes
|
||||
- [ ] Manual smoke: race-fix verified by slow network reload + remote pixel placement
|
||||
- [ ] Manual smoke: 6th WS rejected with 429
|
||||
- [ ] Manual smoke: foreign-origin WS rejected with 403
|
||||
- [ ] Manual smoke: ping/pong visible in dev tools
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] No pixel placed during the initial-fetch window is dropped (verified via instrumented log).
|
||||
- [ ] WS upgrade from disallowed origin returns 403 in production.
|
||||
- [ ] Per-identity cap enforced; logs show `too_many_sockets` when triggered.
|
||||
- [ ] `onclose` fires within ~60s of network drop (verified via airplane-mode toggle).
|
||||
- [ ] No regression in 94-test suite.
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Risk:** Origin allowlist set too tight → legitimate clients (preview deployments, custom domains) get 403.
|
||||
**Mitigation:** Empty `ALLOWED_ORIGINS` allows all — start with empty in dev/preview, populate before production deploy.
|
||||
- **Risk:** Cap on identity blocks tab-power-users (5 tabs is normal for some folks).
|
||||
**Mitigation:** Cap is configurable; bump to 10 if support tickets appear.
|
||||
- **Risk:** Heartbeat interval too aggressive → battery drain on mobile.
|
||||
**Mitigation:** 30s ping is well below the typical mobile-radio-wakeup penalty; keeping interval >= 25s avoids extra wakes.
|
||||
- **Risk:** Buffer-and-replay logic interacts oddly with the existing `imageData` invalidation in CanvasRenderer.
|
||||
**Mitigation:** Replay loop must call the same path the live message handler does (write to both `committedColors` AND `imageData`); add a unit test in Phase 4.
|
||||
|
||||
## Security Considerations
|
||||
|
||||
- Origin check is a usability/cost barrier, not a security one — WS protocol allows non-browser clients to spoof Origin. Real defense is the per-identity cap + Worker request budget.
|
||||
- Per-identity cap prevents broadcast amplification (debugger H5, L6).
|
||||
- Heartbeat surface is a single text-equality check; no parser exposure.
|
||||
@@ -1,161 +0,0 @@
|
||||
---
|
||||
phase: 4
|
||||
title: "Full DO surface test coverage"
|
||||
status: completed
|
||||
priority: P2
|
||||
effort: "6h"
|
||||
dependencies: [1, 2, 3]
|
||||
---
|
||||
|
||||
# Phase 4: Full DO Surface Test Coverage
|
||||
|
||||
## Overview
|
||||
Close the test gap: today only `test/worker-validation.test.js` exists. The DO surface (chunk-storage, cooldown-store, WS hub, identity resolution) has zero unit tests. Add a vitest harness that runs against `wrangler unstable_dev` so DO state is real, not mocked.
|
||||
|
||||
## Context Links
|
||||
- Reports: `plans/reports/code-reviewer-260510-0211-rplace-do-migration.md` (positive observations re tests; gap implicit)
|
||||
- Reports: `plans/reports/debugger-260510-0211-rplace-edge-cases.md` (L7 — explicit DO test gap)
|
||||
|
||||
## Key Insights
|
||||
- Mocked DO unit tests (current style) won't catch the bugs Phase 1–3 fix. Need real DO state.
|
||||
- `wrangler unstable_dev` lets vitest hit a live local Worker + DO via fetch. Heavier but accurate.
|
||||
- For pure-function modules (`pixel-buffer`, `chunk-storage` math), keep classic vitest unit tests — fast loop.
|
||||
- The 200-line file rule applies to test files too; split per concern (one test file per source file).
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional**
|
||||
- Tests cover: write atomicity, BLOB-grow, orphan-row read, cooldown refund, cooldown TTL, WS broadcast frame format with `seq`, WS per-identity cap, ping/pong, identity cookie/IP/dev fallback, gap detection.
|
||||
- All tests pass on `npm test`.
|
||||
|
||||
**Non-functional**
|
||||
- Total test runtime under 30s on a dev machine.
|
||||
- No hard-coded sleeps > 100ms; use polling helpers where needed.
|
||||
- Tests are deterministic — no flakes when run 10× consecutively.
|
||||
|
||||
## Architecture
|
||||
|
||||
```
|
||||
test/
|
||||
├── lib/
|
||||
│ ├── cookie.test.js # NEW — parseCookie/formatSetCookie
|
||||
│ ├── get-user-id.test.js # EXTEND — cookie/ip/dev fallback paths
|
||||
│ ├── pixel-buffer.test.js # EXISTING — keep
|
||||
│ └── ... (existing image-* tests stay)
|
||||
├── durable-objects/
|
||||
│ ├── chunk-storage.test.js # NEW — pure-function unit tests via mocked sql
|
||||
│ ├── cooldown-store.test.js # NEW — TTL math, GC sample, race
|
||||
│ └── canvas-room.integration.test.js # NEW — wrangler unstable_dev
|
||||
├── worker-validation.test.js # EXISTING — keep
|
||||
└── helpers/
|
||||
└── do-harness.js # NEW — boot wrangler unstable_dev once per file
|
||||
```
|
||||
|
||||
`do-harness.js` exports `setupDO()` returning `{ worker, fetch, close }`. Suite uses `beforeAll` / `afterAll` to share the harness.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Create**
|
||||
- `test/helpers/do-harness.js`
|
||||
- `test/lib/cookie.test.js`
|
||||
- `test/durable-objects/chunk-storage.test.js`
|
||||
- `test/durable-objects/cooldown-store.test.js`
|
||||
- `test/durable-objects/canvas-room.integration.test.js`
|
||||
|
||||
**Modify**
|
||||
- `test/lib/get-user-id.test.js` — extend for new resolution logic
|
||||
- `vitest.config.js` — add `testTimeout: 30_000`, include `test/durable-objects/**/*`
|
||||
- `package.json` — no new deps; `wrangler` is already a devDependency
|
||||
|
||||
**Delete** — none
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
### Pure-function tests
|
||||
|
||||
1. **`test/lib/cookie.test.js`**
|
||||
- `parseCookie('a=1; b=2')` → Map `{a:'1', b:'2'}`
|
||||
- `parseCookie('')` → empty Map
|
||||
- `parseCookie(undefined)` → empty Map
|
||||
- Malformed: `parseCookie('a; b=2')` → Map `{b:'2'}`
|
||||
- `formatSetCookie('rplace_id', 'uuid', { httpOnly: true, secure: true, sameSite: 'Lax', maxAge: 31536000, path: '/' })` → exact string match.
|
||||
|
||||
2. **`test/lib/get-user-id.test.js`** (extend)
|
||||
- Cookie present → `cookie:<uuid>`
|
||||
- No cookie, IP present → `ip:<hash>`
|
||||
- Production env, neither → throws `NoIdentityError`
|
||||
- Dev env, neither → `dev:*`
|
||||
- Cookie + IP both present → cookie wins
|
||||
- Cookie issuance flag set when caller should `Set-Cookie`
|
||||
|
||||
3. **`test/durable-objects/chunk-storage.test.js`** (mock sql)
|
||||
- Build a fake `sql` with an in-memory `Map<chunkId, Uint8Array>` backing `INSERT OR REPLACE` and `SELECT`.
|
||||
- `writePixels` single chunk: write 3 pixels, read back identical bytes.
|
||||
- `writePixels` 2-chunk batch: pixels at offset 0 and offset 65535+1, both persist.
|
||||
- `writePixels` BLOB-grow: pre-seed last chunk with a short blob (8KB), grow via fake `chunkSize` returning 64KB, write a pixel at byte 30000, read back exactly 64KB blob with byte 30000 set.
|
||||
- `readAllChunks` orphan: pre-seed `chunk_id = 999`, `CHUNK_COUNT = 256`, read returns valid `Uint8Array(TOTAL_PIXELS)` without throwing.
|
||||
- `chunkSize(255)` for current dims = 65536; `chunkSize(0..254)` = 65536.
|
||||
- Test the bound query: orphan row not selected.
|
||||
|
||||
4. **`test/durable-objects/cooldown-store.test.js`** (mock sql)
|
||||
- `tryAcquire(userId, now)` first call returns `{ ok: true }`.
|
||||
- Second call within 1s returns `{ ok: false, retryAfter: <ms remaining> }`.
|
||||
- Second call after 1s returns `{ ok: true }`.
|
||||
- GC sample with deterministic RNG: pre-seed expired rows, run `tryAcquire` with mocked `Math.random()` returning 0 (always GC) → expired rows deleted.
|
||||
- INSERT cursor symmetry: ensure both branches drain (after fix in Phase 1's M1).
|
||||
|
||||
### Integration tests (real DO)
|
||||
|
||||
5. **`test/helpers/do-harness.js`**
|
||||
- `import { unstable_dev } from 'wrangler'`
|
||||
- `export async function setupDO() { const worker = await unstable_dev('src/worker.js', { config: 'wrangler.json', experimental: { disableExperimentalWarning: true } }); return { worker, fetch: worker.fetch.bind(worker), close: () => worker.stop() }; }`
|
||||
- Add helper `placePixel(worker, { x, y, color, cookie? })` returning `{ status, body, setCookie }`.
|
||||
|
||||
6. **`test/durable-objects/canvas-room.integration.test.js`**
|
||||
- **Cookie issuance:** GET `/api/canvas` without cookie → response has `Set-Cookie: rplace_id=...`. Subsequent GET with cookie → no new Set-Cookie.
|
||||
- **Cooldown isolation:** two cookies, same simulated IP → both place pixels in the same second.
|
||||
- **Cooldown refund on error:** force a write error path (test-only env flag throws inside `writePixels`) → cooldown row deleted, second attempt succeeds immediately.
|
||||
- **Multi-chunk atomicity:** force-error mid-batch on second chunk → `GET /api/canvas` shows none of the batch applied, broadcast not fired.
|
||||
- **Orphan row:** seed orphan via DO debug endpoint (test-only) → `GET /api/canvas` returns 200.
|
||||
- **content-length=0 rejected:** POST `/api/place` with `Content-Length: 0` → 411 or 413.
|
||||
- **WS broadcast carries seq:** open WS, place a pixel, receive frame with `{ type:'pixels', seq:1, edits:[...] }`. Place another → `seq:2`.
|
||||
- **WS per-identity cap:** open 5 WS with same cookie → all accepted. 6th → 429 / connection-close.
|
||||
- **WS ping/pong:** send `'ping'`, receive `'pong'`.
|
||||
- **WS gap → client refetch:** simulate dropped frame by manually skipping a `seq`; client-side gap detection is a unit-test concern (covered in `test/lib/seq-gap.test.js` if extracted; otherwise verify protocol shape only).
|
||||
- **Origin allowlist:** WS upgrade with `Origin: https://evil.example` → 403; with no Origin → allowed.
|
||||
|
||||
## Todo List
|
||||
|
||||
- [ ] Decide test-only env flag mechanism (e.g., `env.TEST_FORCE_WRITE_ERROR === '1'`) and add gated branches
|
||||
- [ ] Write `test/helpers/do-harness.js`
|
||||
- [ ] Write `test/lib/cookie.test.js`
|
||||
- [ ] Extend `test/lib/get-user-id.test.js`
|
||||
- [ ] Write `test/durable-objects/chunk-storage.test.js`
|
||||
- [ ] Write `test/durable-objects/cooldown-store.test.js`
|
||||
- [ ] Write `test/durable-objects/canvas-room.integration.test.js`
|
||||
- [ ] Update `vitest.config.js` (timeout + include patterns)
|
||||
- [ ] `npm test` — all green
|
||||
- [ ] Run 10× back-to-back; no flakes
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] All Phase 1–3 fixes have at least one test that fails on the pre-fix code and passes after.
|
||||
- [ ] Coverage report shows DO source files at ≥ 80% line coverage.
|
||||
- [ ] Total test runtime under 30s on dev hardware.
|
||||
- [ ] CI (if configured) passes; otherwise local 10× green.
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Risk:** `wrangler unstable_dev` is not stable across versions; tests may break on upgrade.
|
||||
**Mitigation:** Pin `wrangler` version in `package.json` (already pinned to `^4.14.1`); update tests when intentionally bumping.
|
||||
- **Risk:** Test-only env flags leak into production.
|
||||
**Mitigation:** Gate behind `env.NODE_ENV === 'test'` AND require an explicit `env.TEST_HOOKS === 'enabled'` second flag. Document; reject in deploy script.
|
||||
- **Risk:** Integration tests slow down dev loop, devs skip them.
|
||||
**Mitigation:** Add `npm run test:unit` (excludes `**/integration.*`) for fast inner loop.
|
||||
- **Risk:** WS testing in vitest is awkward (`unstable_dev` returns Worker, not Server).
|
||||
**Mitigation:** Use `worker.fetch` for upgrade and treat the returned `WebSocket` directly. If tooling doesn't allow that, fall back to spawning `wrangler dev` in a child process.
|
||||
|
||||
## Security Considerations
|
||||
|
||||
- Test-only hooks must never ship to production. Add a build-time assertion (e.g., a fail-fast check at worker startup if `TEST_HOOKS` is enabled in production env).
|
||||
- Tests must not commit any secrets — use `crypto.randomUUID()` for ephemeral test cookies.
|
||||
@@ -1,121 +0,0 @@
|
||||
---
|
||||
phase: 5
|
||||
title: "Docs cleanup & legacy plan archival"
|
||||
status: completed
|
||||
priority: P2
|
||||
effort: "30m"
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Phase 5: Docs Cleanup & Legacy Plan Archival
|
||||
|
||||
## Overview
|
||||
Strike stale Upstash references from `README.md` and `.env.example`. Verify the `docs/` directory matches current code (the docs-manager and code-reviewer reports disagree — verify file by file). Mark the migration plan as `completed` after the 7-day rollback observation window.
|
||||
|
||||
## Context Links
|
||||
- Reports: `plans/reports/docs-manager-260510-0211-rplace-docs-drift.md` (primary)
|
||||
- Reports: `plans/reports/code-reviewer-260510-0211-rplace-do-migration.md` (C1 — claims docs/*.md still mention Upstash; conflicts with docs-manager finding)
|
||||
|
||||
## Key Insights
|
||||
- Docs-manager said `docs/system-architecture.md`, `deployment-guide.md`, `code-standards.md` are accurate. Code-reviewer C1 said they still reference Upstash. Verify directly before deciding.
|
||||
- Migration endpoint already removed from worker; documenting it as "transitional" misleads onboarding.
|
||||
- `.env.example` placeholders for deleted services cause new-developer confusion.
|
||||
|
||||
## Requirements
|
||||
|
||||
**Functional**
|
||||
- README's Project Structure block lists only files that exist in `src/`.
|
||||
- README does not advertise `/admin/migrate-from-upstash` as transitional.
|
||||
- `.env.example` either deleted or replaced with a comment explaining no external secrets.
|
||||
- All `docs/*.md` files verified accurate; any Upstash references that describe current state (not history) removed.
|
||||
- `plans/260509-2309-canvas-on-do-storage/plan.md` frontmatter `status: completed` (after 2026-05-17, the rollback window cutoff).
|
||||
|
||||
**Non-functional**
|
||||
- Edits target only the lines in question; no unrelated reformatting.
|
||||
- Git history preserves clear "docs:" commit type per `.claude` convention (drop `docs:` from `.claude/` paths only — not relevant here).
|
||||
|
||||
## Architecture
|
||||
N/A — text edits.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Modify**
|
||||
- `README.md` — Project Structure tree (lines 79–111), API section (lines 146–151)
|
||||
- `.env.example` — content
|
||||
- `docs/system-architecture.md` — only if verification finds Upstash-as-current text
|
||||
- `docs/deployment-guide.md` — only if verification finds Upstash-as-current text
|
||||
- `docs/code-standards.md` — only if verification finds Upstash-as-current text
|
||||
- `plans/260509-2309-canvas-on-do-storage/plan.md` — frontmatter `status` field (after rollback window)
|
||||
|
||||
**Create** — none
|
||||
|
||||
**Delete** — possibly `.env.example` (Option A in docs-manager report)
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. **Verify docs/ accuracy**
|
||||
- `grep -ni 'upstash\|redis\|@upstash' docs/*.md`
|
||||
- For each match: read context, classify as (a) historical mention OK, (b) presented as current — needs edit.
|
||||
- Decide whether to keep migration narrative as historical or scrub entirely.
|
||||
|
||||
2. **README.md Project Structure** (docs-manager STALE finding 1)
|
||||
- Replace lines 79–111 with the corrected tree from docs-manager report (no `src/admin/`, no legacy lib files; include image pipeline files).
|
||||
- Use the actual `find src -type f` output as ground truth.
|
||||
|
||||
3. **README.md API Section** (docs-manager MISLEADING finding 2)
|
||||
- Delete the `### POST /admin/migrate-from-upstash (transitional)` block (lines 146–151).
|
||||
- Verify no other doc references this section.
|
||||
|
||||
4. **.env.example** (docs-manager STALE finding 3)
|
||||
- Choose Option B (keep as documentation): replace contents with a single comment block:
|
||||
```
|
||||
# No external secrets required.
|
||||
# Canvas + cooldown state live inside CanvasRoom Durable Object (SQLite).
|
||||
# All configuration is in src/lib/constants.js.
|
||||
```
|
||||
- Rationale: discoverability. New devs find `.env.example` and learn there's nothing to set.
|
||||
|
||||
5. **docs/ targeted edits** (only if step 1 found drift)
|
||||
- Edit each flagged section. Keep migration mentioned in `deployment-guide.md` "Optional One-Shot Migration from Upstash" as historical — do not delete the narrative if it's already framed as past.
|
||||
|
||||
6. **Migration plan archival** (docs-manager finding 4)
|
||||
- Today is 2026-05-10; rollback window per migration-plan ends ~2026-05-17.
|
||||
- Add a TODO note in this phase saying "after 2026-05-17, change `plans/260509-2309-canvas-on-do-storage/plan.md` frontmatter `status` to `completed`".
|
||||
- For now (before 2026-05-17), leave `status: in-progress` BUT add `deployment.cleanupAt: 2026-05-10` (already present per existing frontmatter) and verify it's accurate.
|
||||
|
||||
7. **Sanity sweep**
|
||||
- `grep -rni 'upstash\|@upstash\|redis-client\|rate-limiter\.js\|migrate-from-upstash' README.md docs/ .env.example src/` — should return only intentional historical refs.
|
||||
- `npm run build` — must still pass (sanity check that a doc edit didn't break a `<script src>` reference or similar).
|
||||
|
||||
## Todo List
|
||||
|
||||
- [ ] Run grep sweep on `docs/` for upstash/redis terms
|
||||
- [ ] Classify each match (historical vs current-as-of-today)
|
||||
- [ ] Edit README.md Project Structure tree
|
||||
- [ ] Delete README.md `/admin/migrate-from-upstash` API section
|
||||
- [ ] Rewrite `.env.example` with documentation comment
|
||||
- [ ] Apply targeted edits in `docs/*.md` if step 1 found current-as-of-today refs
|
||||
- [ ] Add reminder note (this phase) to flip migration plan status to `completed` after 2026-05-17
|
||||
- [ ] Final grep sweep — no stale refs remain
|
||||
- [ ] `npm run build` passes
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] `find src -type f` matches the README Project Structure tree
|
||||
- [ ] No `/admin/migrate-from-upstash` reference in `README.md`
|
||||
- [ ] `.env.example` is documentation-only or absent
|
||||
- [ ] `docs/*.md` accurately describes current code (no current-tense Upstash refs)
|
||||
- [ ] Final grep sweep shows only intentional historical mentions
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Risk:** Editing docs accidentally drops useful historical context for future operators.
|
||||
**Mitigation:** Keep "Optional One-Shot Migration from Upstash" sections as past-tense narrative; only scrub anything that says "use this NOW".
|
||||
- **Risk:** Premature archival of migration plan loses rollback information.
|
||||
**Mitigation:** Wait until 2026-05-17 (7-day window). Frontmatter already records `migratedAt` and `postCleanupVersionId` — those stay regardless of `status`.
|
||||
|
||||
## Security Considerations
|
||||
N/A — docs only.
|
||||
|
||||
## Next Steps
|
||||
None — this is the cleanup tail.
|
||||
@@ -1,70 +0,0 @@
|
||||
---
|
||||
title: "Fix critical bugs and security gaps from DO migration code review"
|
||||
status: completed
|
||||
priority: P1
|
||||
created: 2026-05-10
|
||||
completed: 2026-05-11
|
||||
phases: 5
|
||||
source: skill
|
||||
sourceReports:
|
||||
- ../reports/code-reviewer-260510-0211-rplace-do-migration.md
|
||||
- ../reports/debugger-260510-0211-rplace-edge-cases.md
|
||||
- ../reports/docs-manager-260510-0211-rplace-docs-drift.md
|
||||
blockedBy: []
|
||||
blocks: []
|
||||
---
|
||||
|
||||
# Plan: Fix DO Migration Code-Review Follow-ups
|
||||
|
||||
## Goal
|
||||
|
||||
Close the 3 Critical + 5 High findings from the post-migration triple-review (code-reviewer, debugger, docs-manager). Add full DO surface test coverage. Scrub stale Upstash references from `README.md` and `.env.example`.
|
||||
|
||||
## Decisions Locked (from validation Q&A)
|
||||
|
||||
- **Identity:** Cookie + IP fallback. Issue opaque per-browser cookie on first `/api/canvas`; rate-limit by cookie when present, fall back to IP otherwise.
|
||||
- **Daily pixel cap:** Deferred to a follow-up plan (product input needed).
|
||||
- **Test scope:** Full DO surface coverage (chunk-storage, cooldown-store, WS hub, get-user-id, integration).
|
||||
|
||||
## Phases
|
||||
|
||||
| # | File | Title | Status | Priority | Blocks |
|
||||
|---|---|---|---|---|---|
|
||||
| 1 | [phase-01-do-storage-atomicity.md](phase-01-do-storage-atomicity.md) | DO storage atomicity & correctness | completed | P1 | 4 |
|
||||
| 2 | [phase-02-cookie-ip-identity.md](phase-02-cookie-ip-identity.md) | Cookie+IP identity & broadcast sequence | completed | P1 | 4 |
|
||||
| 3 | [phase-03-ws-hardening-client-race.md](phase-03-ws-hardening-client-race.md) | WebSocket hardening & client race fix | completed | P1 | 4 |
|
||||
| 4 | [phase-04-do-surface-tests.md](phase-04-do-surface-tests.md) | Full DO surface test coverage | completed | P2 | — |
|
||||
| 5 | [phase-05-docs-cleanup.md](phase-05-docs-cleanup.md) | Docs cleanup & legacy plan archival | completed | P2 | — |
|
||||
|
||||
Phases 1, 2, 3 are independent and can ship in any order or in parallel. Phase 4 depends on the API surfaces stabilized in 1–3. Phase 5 is independent — can land first.
|
||||
|
||||
## Findings Coverage Map
|
||||
|
||||
| Phase | Critical | High | Medium |
|
||||
|---|---|---|---|
|
||||
| 1 | C2 (review C3 BLOB-grow), C2 (debugger atomicity), C1 (debugger cooldown burn) | review-H1, H2, H3, H5 | review-M1 |
|
||||
| 2 | C3 (debugger NAT) | debugger-H2, H3 | review-M4 |
|
||||
| 3 | C2 (review WS race) | review-H4, debugger-H5 | review-M2, M3, M7 |
|
||||
| 4 | — | L7 test gap | — |
|
||||
| 5 | C1 (review stale docs) | — | — |
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- Per-IP daily pixel quota (deferred per Q&A).
|
||||
- Edge-cache strategy change (`s-maxage` tuning) — needs product call on live-fresh vs cheap.
|
||||
- Multi-room sharding (`idFromName('main')` stays single-DO).
|
||||
- Signed cookie / HMAC identity — opaque cookie is sufficient for this round.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- All listed Critical + High findings resolved with file:line citations in PR description.
|
||||
- `npm test` passes; new DO tests cover write atomicity, BLOB-grow, cooldown refund, WS hub, identity.
|
||||
- README + `.env.example` purged of Upstash refs; docs/ verified accurate.
|
||||
- Production deploy + 24h soak shows no new error class in CF logs.
|
||||
- Migration plan `260509-2309-canvas-on-do-storage/plan.md` marked `status: completed`.
|
||||
|
||||
## Related Reports
|
||||
|
||||
- `plans/reports/code-reviewer-260510-0211-rplace-do-migration.md`
|
||||
- `plans/reports/debugger-260510-0211-rplace-edge-cases.md`
|
||||
- `plans/reports/docs-manager-260510-0211-rplace-docs-drift.md`
|
||||
Reference in new issue
Block a user