mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-10 15:05:40 +00:00
xarmian
50e0936b34
* feat(collab): designated-applier protocol for external content updates (TASK-1257) The keystone task for CLI / API / MCP integration during co-edit sessions. When a content update arrives via PATCH while at least one browser tab is connected to the item's collab room, the server can't write items.content directly — the connected tabs would silently overwrite it on the next 5s idle flush using their (now stale) Y.Doc state and the caller's update would be lost. Solution: nominate one connected tab as the "designated applier", send it a JSON control message with the new markdown, the browser does editor.commands.setContent(markdown) which the y-tiptap binding translates into Y.Doc updates that propagate via the regular sync path. Items.content gets refreshed via the next 5s flush (TASK-1261). Architecture: internal/collab/applier.go (new): - ControlMessage struct — JSON envelope for applier_request / applier_ack frames. Carried over WebSocket TextMessage, which is unambiguous against y-protocol's BinaryMessage. - ApplyExternalContent(itemID, markdown) — public entry point. Returns nil on ack, ErrNoActiveRoom when there's no room (caller falls back to direct write), ErrNoApplierAvailable when the room has no live conns, ErrAllAppliersTimedOut when every attempt expired. - Election: pickApplier returns the longest-connected roomConn that hasn't already been tried, with deterministic tiebreak on conn id. Stable choice — longest connection has the most authoritative cumulative Y.Doc state, fewer flicker risks. - Retry loop: applierMaxAttempts=2, applierFirstTimeoutVar=30s, applierRetryTimeoutVar=15s. The Var-suffixed names exist so test helpers can shrink to ms without sleeping a real minute. - Pending-ack tracking: per-room map[requestID]*pendingApplierAck pairing the channel a PATCH handler is waiting on with the conn the ack is expected from. expectedConn check prevents an unrelated peer from spoofing acks for someone else's request. internal/collab/room.go (extended): - roomConn gains connectedAt for the election. - readLoop branches TextMessage → handleControlMessage which decodes the JSON and routes applier_ack to the room's pending tracker. Unknown control types and malformed JSON are silently dropped so a bad client can't break the loop. internal/collab/manager.go: - Registers connectedAt on Join. - Initialises room.pendingAcks alongside conns map. internal/server/handlers_items.go (extended): - handleUpdateItem now branches on input.Content != nil + s.collab != nil: routes through s.applyContentViaCollab; on success, zeros input.Content so UpdateItem's direct write is suppressed. - Field-only PATCHes skip this branch entirely — backward-compatible. internal/server/handlers_collab.go: - applyContentViaCollab wraps mgr.ApplyExternalContent with per-error-class slog warnings so operators can see degraded paths (timeouts → warn; no-room / no-applier → quiet, the common case for non-co-edit CLI updates). - actorIDFromRequest helper for log fields. Tests (5 new): - TestApplyExternalContentNoActiveRoom — sentinel error path. - TestApplyExternalContentHappyPath — applier echo acks within ms. - TestApplyExternalContentTimeoutsThenFails — applier never acks; we hit applierFirstTimeoutVar then ErrAllAppliersTimedOut. - TestApplyExternalContentTimeoutThenSecondAcks — first applier silent, retry picks second-longest-connected, succeeds. - TestApplyExternalContentRejectsAckFromUnexpectedConn — defence- in-depth: peer B forges an ack for peer A's request; the room's expectedConn check rejects it; ApplyExternalContent runs to timeout instead of being satisfied by the forgery. All tests pass under -race. Full suite green. Parent: PLAN-1248. Phase 1 — Backend foundation. * fix(collab): clean pendingAcks on success + expires_at on applier_request per Codex review (round 1) P2 #1: ApplyExternalContent retained the per-request pendingAcks entry on success. Each successful external update therefore leaked a request_id + channel + expected-conn pointer for the remainder of the room's lifetime — across long-lived sessions the map would grow without bound. Add cancelPendingAck to the ack-success path so the entry is released as soon as the request completes. Test added (TestApplyExternalContentCleansPending- AcksOnSuccess) drives 5 successful applies and asserts the pendingAcks map is empty afterwards. P2 #2: applier_request had no client-enforceable expiry, so a backgrounded tab could process a stale request 60s later and overwrite newer edits with old markdown after the server had already retried with a different applier (or fallen back to direct write). Add ExpiresAtMillis to the ControlMessage envelope, populated per attempt with `now + timeouts[attempt]`. The browser-side handler (TASK-1263) is responsible for the client-side Now() check before applying — without that check the field is documentation-only. Added test (TestApplyExternalContentSendsExpiresAt) regression-tests the server stamp. Server-side cleanup is also reinforced: cancelPendingAck on timeout (already present) means a late ack from a timed-out applier is rejected at the room layer (entry is gone). The expires_at_millis is the second line of defence for the case where the browser sends the Y.Doc setContent BEFORE the ack — the request must not be applied at all.