cleanup spec

.kiro/specs/migrate-to-y-websocket/design.md

@@ -173,6 +173,7 @@ stateDiagram-v2
 |-----------|-------------|--------|-------------|-----------------|-----------|
 | YjsService | Server / Service | Orchestrates Yjs document lifecycle, exposes public API | 1.1-1.5, 2.1, 6.1-6.4 | ws (P0), y-websocket/bin/utils (P0), MongodbPersistence (P0) | Service |
 | UpgradeHandler | Server / Auth | Authenticates and authorizes WebSocket upgrade requests | 3.1-3.4, 2.3 | express-session (P0), passport (P0), Page model (P0) | Service |
+| guardSocket | Server / Util | Prevents socket closure by other upgrade handlers during async auth | 2.3 | — | Utility |
 | PersistenceAdapter | Server / Data | Bridges MongodbPersistence to y-websocket persistence interface; handles sync-on-load and awareness registration | 4.1-4.5, 6.3, 5.2, 5.3 | MongodbPersistence (P0), syncYDoc (P0), Socket.IO io (P1) | Service, Event |
 | AwarenessBridge | Server / Events | Bridges y-websocket awareness events to Socket.IO rooms | 5.2, 5.3 | Socket.IO io (P0), docs Map (P1) | Event |
 | use-collaborative-editor-mode | Client / Hook | Manages WebsocketProvider lifecycle and awareness | 2.2, 2.4, 5.1, 5.4 | y-websocket (P0), yjs (P0) | State |
@@ -240,18 +241,15 @@ interface IYjsService {
 | Requirements | 3.1, 3.2, 3.3, 3.4 |
 
 **Responsibilities & Constraints**
-- Parses session cookie from the HTTP upgrade request
-- Loads session from the session store (Redis or MongoDB)
-- Deserializes the user via passport
+- Runs express-session and passport middleware against the raw upgrade request via `runMiddleware` helper
 - Checks page access using `Page.isAccessiblePageByViewer`
 - Extracts `pageId` from the URL path (`/yjs/{pageId}`)
-- Rejects unauthorized requests before `wss.handleUpgrade`
+- Writes HTTP error responses for unauthorized requests (`writeErrorResponse`) — does NOT close the socket; socket lifecycle is managed by the caller (YjsService) to work correctly with `guardSocket`
 
 **Dependencies**
 - Inbound: YjsService — called on upgrade event (P0)
-- Outbound: Session Store (Redis/MongoDB) — session lookup (P0)
+- Outbound: express-session + passport — session/user deserialization (P0)
 - Outbound: Page model — access check (P0)
-- External: cookie (npm) — cookie parsing (P0)
 
 **Contracts**: Service [x]
 
@@ -259,31 +257,45 @@ interface IYjsService {
 
 ```typescript
 type AuthenticatedRequest = IncomingMessage & {
-  user: IUserHasId | null;
+  user?: IUserHasId;
 };
 
 type UpgradeResult =
   | { authorized: true; request: AuthenticatedRequest; pageId: string }
   | { authorized: false; statusCode: number };
 
-interface IUpgradeHandler {
-  handleUpgrade(
-    request: IncomingMessage,
-    socket: Duplex,
-    head: Buffer,
-  ): Promise<UpgradeResult>;
-}
+// Factory function — returns an async handler
+const createUpgradeHandler = (sessionConfig: SessionConfig) =>
+  async (request, socket, head): Promise<UpgradeResult> => { ... };
 ```
 
 - Preconditions: Request has valid URL matching `/yjs/{pageId}`
-- Postconditions: Returns authorized result with deserialized user and pageId, or rejection with HTTP status
-- Invariants: Never calls `wss.handleUpgrade` for unauthorized requests
+- Postconditions: Returns authorized result with deserialized user and pageId, or rejection with HTTP error written to socket
+- Invariants: Never calls `wss.handleUpgrade` for unauthorized requests; never calls `socket.destroy()` (caller responsibility)
 
 **Implementation Notes**
-- Use `cookie` package to parse `request.headers.cookie`
-- Use the session store's `get(sessionId, callback)` to load session data
-- Attach `user` to `request` object for downstream use in `setupWSConnection`
-- Guest access: if `user` is null but page allows guest access, proceed with authorization
+- Uses `runMiddleware` helper to execute Connect-style middleware (express-session, passport.initialize, passport.session) against raw `IncomingMessage` with a stub `ServerResponse`
+- `writeErrorResponse` writes HTTP status line only — socket cleanup is deferred to the caller so that `guardSocket` (see below) can intercept `socket.destroy()` during async auth
+- Guest access: if `user` is undefined but page allows guest access, proceed with authorization
+
+#### guardSocket
+
+| Field | Detail |
+|-------|--------|
+| Intent | Prevents other synchronous upgrade handlers from closing the socket during async auth |
+| Requirements | 2.3 (coexistence with other servers) |
+
+**Why this exists**: Node.js EventEmitter fires all `upgrade` listeners synchronously. When the Yjs handler (async) yields at its first `await`, Next.js's `NextCustomServer.upgradeHandler` runs and calls `socket.end()` for paths it does not recognize. This destroys the socket before Yjs auth completes. `prependListener` cannot solve this because it only changes listener order — it cannot prevent subsequent listeners from executing.
+
+**How it works**: Temporarily replaces `socket.end()` and `socket.destroy()` with no-ops before the first `await`. After auth completes, calls `restore()` to reinstate the original methods, then proceeds with either `wss.handleUpgrade` (success) or `socket.destroy()` (failure).
+
+```typescript
+const guard = guardSocket(socket);
+const result = await handleUpgrade(request, socket, head);
+guard.restore();
+```
+
+**Test coverage**: `guard-socket.spec.ts` verifies that a hostile upgrade handler calling `socket.end()` does not prevent WebSocket connection establishment.
 
 #### PersistenceAdapter
 
@@ -432,38 +444,7 @@ No changes to data models. The `yjs-writings` MongoDB collection schema, indexes
 - Log persistence errors at `error` level
 - Existing Socket.IO event monitoring unchanged
 
-## Testing Strategy
-
-### Unit Tests
-- UpgradeHandler: cookie parsing, session loading, access check for authorized/unauthorized/guest users
-- PersistenceAdapter: bindState loads and applies persisted state, writeState flushes document
-- AwarenessBridge: awareness event triggers correct Socket.IO room emission
-- WebSocket URL construction in use-collaborative-editor-mode
-
-### Integration Tests
-- Full connection flow: WebSocket upgrade → auth → document creation → sync step 1/2
-- Multi-client sync: Two clients connect, both receive each other's updates via same Y.Doc
-- Reconnection: Client disconnects and reconnects, receives updates missed during disconnection
-- Persistence round-trip: Document persisted on disconnect, restored on next connection
-
-### Concurrency Tests
-- Simultaneous connections: Multiple clients connect at the same instant — verify single Y.Doc instance (the race condition fix)
-- Disconnect during connect: Client disconnects while another is initializing — verify no document corruption
-
-## Security Considerations
-
-- **Authentication boundary**: Auth check happens in the HTTP upgrade handler BEFORE WebSocket connection is established — unauthorized clients never receive any Yjs data
-- **Session fixation**: Uses same session mechanism as the rest of GROWI; no new attack surface
-- **Data leakage**: PageId extracted from URL path is validated against `Page.isAccessiblePageByViewer` — same check as current y-socket.io middleware
-- **DoS**: WebSocket connections are subject to the same connection limits as Socket.IO (enforced at HTTP level)
-
-## Migration Strategy
-
-This is a code-level replacement, not a data migration. No changes to the `yjs-writings` MongoDB collection.
-
-**Phase 1**: Implement server-side changes (YjsService, UpgradeHandler, PersistenceAdapter, AwarenessBridge)
-**Phase 2**: Implement client-side changes (use-collaborative-editor-mode, ViteDevConfig)
-**Phase 3**: Remove y-socket.io dependency, update package.json classifications
-**Phase 4**: Test all collaborative editing scenarios
+## Security Notes
 
-Rollback: Revert the code changes; no data migration to undo.
+- Auth check happens in the HTTP upgrade handler BEFORE WebSocket connection is established — unauthorized clients never receive any Yjs data
+- Uses same session mechanism as the rest of GROWI; no new attack surface

.kiro/specs/migrate-to-y-websocket/requirements.md

@@ -77,7 +77,7 @@ This specification defines the requirements for migrating the collaborative edit
 
 1. The Yjs Service shall continue to expose `getYDocStatus(pageId)` returning the correct status (ISOLATED, NEW, DRAFT, SYNCED, OUTDATED).
 2. The Yjs Service shall continue to expose `getCurrentYdoc(pageId)` returning the in-memory Y.Doc instance if one exists.
-3. When the `document-loaded` event fires (or equivalent), the Yjs Service shall call `syncYDoc` to synchronize the document with the latest revision based on YDoc status.
+3. When a Y.Doc is loaded from persistence (within `bindState`), the Yjs Service shall call `syncYDoc` to synchronize the document with the latest revision based on YDoc status.
 4. The Yjs Service shall continue to expose `syncWithTheLatestRevisionForce(pageId)` for API-triggered force synchronization.
 
 ### Requirement 7: Development Environment Support

.kiro/specs/migrate-to-y-websocket/research.md

@@ -128,6 +128,28 @@
 - **Risk**: Document cleanup race when last client disconnects and a new client immediately connects
   - **Mitigation**: y-websocket's `getYDoc` atomic pattern handles this — new client gets a fresh doc if cleanup completed, or the existing doc if not yet cleaned up
 
+## Implementation Discoveries
+
+### Next.js NextCustomServer.upgradeHandler Race Condition
+- **Context**: WebSocket connections to `/yjs/{pageId}` failed with "could not establish connection" in dev mode
+- **Root Cause**: Next.js's `NextCustomServer.upgradeHandler` (in `next/dist/server/lib/router-server.js:657`) registers an `upgrade` listener on the HTTP server. When the Yjs async handler yields at `await handleUpgrade(...)`, Next.js's synchronous handler runs and calls `socket.end()` for paths it does not recognize.
+- **Evidence**: Stack trace confirmed `socket.end()` called from `NextCustomServer.upgradeHandler` during the 6ms async auth gap
+- **Solution**: `guardSocket` pattern — temporarily replace `socket.end()`/`socket.destroy()` with no-ops before the first `await`, restore after auth completes
+- **Why alternatives don't work**:
+  - `httpServer.prependListener('upgrade', ...)` — only changes listener order, cannot prevent subsequent listeners from executing
+  - Removing Next.js's listener — fragile, breaks HMR
+  - Synchronous auth — impossible (requires async MongoDB/session store queries)
+- **Test**: `guard-socket.spec.ts` reproduces the scenario with a hostile upgrade handler
+
+### React Render-Phase Violation in use-collaborative-editor-mode
+- **Context**: `Cannot update a component (EditingUsers) while rendering a different component (Y)` warning
+- **Root Cause**: Provider creation and awareness event handlers were inside `setProvider(() => { ... })` — a functional state updater that React may call during rendering. `awareness.setLocalStateField()` triggered synchronous awareness events, which called `onEditorsUpdated()`, updating `EditingUsers` state during render.
+- **Solution**: Moved all side effects (provider creation, awareness setup, event handler registration) out of the `setProvider` updater into the `useEffect` body. `setProvider(_provider)` is called with a plain value after setup completes.
+
+### writeErrorResponse Pattern (Socket Lifecycle Separation)
+- **Context**: `rejectUpgrade` originally called both `socket.write()` and `socket.destroy()`, but during `guardSocket` protection, `destroy` was a no-op — creating confusing dual-destroy semantics
+- **Solution**: Renamed to `writeErrorResponse` with only `socket.write()`. Socket cleanup (`destroy`) is exclusively managed by the caller (`yjs.ts`) after `guard.restore()`, ensuring correct behavior regardless of guard state.
+
 ## References
 - [y-websocket GitHub](https://github.com/yjs/y-websocket) — official Yjs WebSocket provider
 - [y-websocket-server GitHub](https://github.com/yjs/y-websocket-server) — server-side utilities (yjs v14)

.kiro/specs/migrate-to-y-websocket/spec.json

@@ -1,9 +1,10 @@
 {
   "feature_name": "migrate-to-y-websocket",
   "created_at": "2026-03-19T00:00:00.000Z",
-  "updated_at": "2026-03-19T00:00:00.000Z",
+  "updated_at": "2026-03-24T00:00:00.000Z",
   "language": "en",
   "phase": "implementation-complete",
+  "cleanup_completed": true,
   "approvals": {
     "requirements": {
       "generated": true,