From 7ef64ecd4f261203bfa7a0447751b3f545f45536 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Mon, 11 May 2026 16:40:04 +0700 Subject: [PATCH] plans: drop implemented plan folders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../phase-01-storage-foundation.md | 110 ------------ .../phase-02-do-integration-worker-proxy.md | 132 -------------- .../phase-03-one-shot-upstash-migration.md | 112 ------------ .../phase-04-cleanup-dependency-removal.md | 120 ------------- .../phase-05-deploy-documentation.md | 96 ----------- .../260509-2309-canvas-on-do-storage/plan.md | 87 ---------- .../phase-01-do-storage-atomicity.md | 151 ---------------- .../phase-02-cookie-ip-identity.md | 153 ----------------- .../phase-03-ws-hardening-client-race.md | 153 ----------------- .../phase-04-do-surface-tests.md | 161 ------------------ .../phase-05-docs-cleanup.md | 121 ------------- .../plan.md | 70 -------- 12 files changed, 1466 deletions(-) delete mode 100644 plans/260509-2309-canvas-on-do-storage/phase-01-storage-foundation.md delete mode 100644 plans/260509-2309-canvas-on-do-storage/phase-02-do-integration-worker-proxy.md delete mode 100644 plans/260509-2309-canvas-on-do-storage/phase-03-one-shot-upstash-migration.md delete mode 100644 plans/260509-2309-canvas-on-do-storage/phase-04-cleanup-dependency-removal.md delete mode 100644 plans/260509-2309-canvas-on-do-storage/phase-05-deploy-documentation.md delete mode 100644 plans/260509-2309-canvas-on-do-storage/plan.md delete mode 100644 plans/260510-0232-fix-do-migration-followups/phase-01-do-storage-atomicity.md delete mode 100644 plans/260510-0232-fix-do-migration-followups/phase-02-cookie-ip-identity.md delete mode 100644 plans/260510-0232-fix-do-migration-followups/phase-03-ws-hardening-client-race.md delete mode 100644 plans/260510-0232-fix-do-migration-followups/phase-04-do-surface-tests.md delete mode 100644 plans/260510-0232-fix-do-migration-followups/phase-05-docs-cleanup.md delete mode 100644 plans/260510-0232-fix-do-migration-followups/plan.md diff --git a/plans/260509-2309-canvas-on-do-storage/phase-01-storage-foundation.md b/plans/260509-2309-canvas-on-do-storage/phase-01-storage-foundation.md deleted file mode 100644 index 62477c0..0000000 --- a/plans/260509-2309-canvas-on-do-storage/phase-01-storage-foundation.md +++ /dev/null @@ -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 | diff --git a/plans/260509-2309-canvas-on-do-storage/phase-02-do-integration-worker-proxy.md b/plans/260509-2309-canvas-on-do-storage/phase-02-do-integration-worker-proxy.md deleted file mode 100644 index 538bb59..0000000 --- a/plans/260509-2309-canvas-on-do-storage/phase-02-do-integration-worker-proxy.md +++ /dev/null @@ -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 | diff --git a/plans/260509-2309-canvas-on-do-storage/phase-03-one-shot-upstash-migration.md b/plans/260509-2309-canvas-on-do-storage/phase-03-one-shot-upstash-migration.md deleted file mode 100644 index 2795ca0..0000000 --- a/plans/260509-2309-canvas-on-do-storage/phase-03-one-shot-upstash-migration.md +++ /dev/null @@ -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 | diff --git a/plans/260509-2309-canvas-on-do-storage/phase-04-cleanup-dependency-removal.md b/plans/260509-2309-canvas-on-do-storage/phase-04-cleanup-dependency-removal.md deleted file mode 100644 index f2af358..0000000 --- a/plans/260509-2309-canvas-on-do-storage/phase-04-cleanup-dependency-removal.md +++ /dev/null @@ -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 | diff --git a/plans/260509-2309-canvas-on-do-storage/phase-05-deploy-documentation.md b/plans/260509-2309-canvas-on-do-storage/phase-05-deploy-documentation.md deleted file mode 100644 index f2cda6c..0000000 --- a/plans/260509-2309-canvas-on-do-storage/phase-05-deploy-documentation.md +++ /dev/null @@ -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 = // must be multiple of CHUNK_BYTES width factor - CANVAS_HEIGHT = -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 | diff --git a/plans/260509-2309-canvas-on-do-storage/plan.md b/plans/260509-2309-canvas-on-do-storage/plan.md deleted file mode 100644 index 9d466e5..0000000 --- a/plans/260509-2309-canvas-on-do-storage/plan.md +++ /dev/null @@ -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. diff --git a/plans/260510-0232-fix-do-migration-followups/phase-01-do-storage-atomicity.md b/plans/260510-0232-fix-do-migration-followups/phase-01-do-storage-atomicity.md deleted file mode 100644 index bcb5e26..0000000 --- a/plans/260510-0232-fix-do-migration-followups/phase-01-do-storage-atomicity.md +++ /dev/null @@ -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. diff --git a/plans/260510-0232-fix-do-migration-followups/phase-02-cookie-ip-identity.md b/plans/260510-0232-fix-do-migration-followups/phase-02-cookie-ip-identity.md deleted file mode 100644 index d645248..0000000 --- a/plans/260510-0232-fix-do-migration-followups/phase-02-cookie-ip-identity.md +++ /dev/null @@ -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=; HttpOnly; Secure; SameSite=Lax; Path=/; Max-Age=31536000`. -- Subsequent requests with cookie: identity = `cookie:`. Without cookie but with valid `cf-connecting-ip`: identity = `ip:`. Neither in prod: 500 with `{ error: "no_identity" }`. Neither in dev: identity = `dev:`. -- 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:" - - 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` — 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:` 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.) diff --git a/plans/260510-0232-fix-do-migration-followups/phase-03-ws-hardening-client-race.md b/plans/260510-0232-fix-do-migration-followups/phase-03-ws-hardening-client-race.md deleted file mode 100644 index 30ce7ef..0000000 --- a/plans/260510-0232-fix-do-migration-followups/phase-03-ws-hardening-client-race.md +++ /dev/null @@ -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. diff --git a/plans/260510-0232-fix-do-migration-followups/phase-04-do-surface-tests.md b/plans/260510-0232-fix-do-migration-followups/phase-04-do-surface-tests.md deleted file mode 100644 index 2f6752f..0000000 --- a/plans/260510-0232-fix-do-migration-followups/phase-04-do-surface-tests.md +++ /dev/null @@ -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:` - - No cookie, IP present → `ip:` - - 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` 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: }`. - - 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. diff --git a/plans/260510-0232-fix-do-migration-followups/phase-05-docs-cleanup.md b/plans/260510-0232-fix-do-migration-followups/phase-05-docs-cleanup.md deleted file mode 100644 index 21397b0..0000000 --- a/plans/260510-0232-fix-do-migration-followups/phase-05-docs-cleanup.md +++ /dev/null @@ -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 `