80fc30633b44acaa976801d5382dccb6fb5e6309
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 claude code agent 227
|
d7fa6738e5 |
fix(comment): transactional childless-delete race fix + client dismiss gate + DB int-spec (#329 review round 2)
F4 [critical] — the anti-join `DELETE … WHERE NOT EXISTS(child)` was still racy under Postgres READ COMMITTED: a reply INSERT holds FOR KEY SHARE on the parent; the DELETE's start snapshot doesn't see the uncommitted child (NOT EXISTS true), blocks on the reply's lock, and when the reply commits the parent was only LOCKED (not modified) so EvalPlanQual does NOT re-check → the DELETE proceeds and CASCADE destroys the just-committed reply. Replaced with a transaction: SELECT the parent FOR UPDATE (conflicts with the reply's FOR KEY SHARE → serializes the concurrent reply), re-check for a child with a FRESH statement in the same tx (a new RC snapshot sees a just-committed reply), delete only if still childless (return 1) else return 0 (caller resolves). The FOR UPDATE lock is held to end-of-tx so no reply can insert between the re-check and the delete. Signature unchanged, so the service + its mocked unit tests are untouched; docstrings updated. F5 [warning] — the client Dismiss button was gated only on canComment, but the server now gates dismiss on owner-or-space-admin, so a non-owner non-admin saw a button the server 403s. `canShowDismiss` now also requires `isOwnerOrAdmin = currentUser?.user?.id === comment.creatorId || userSpaceRole === "admin"` (the same gate the comment delete-menu already uses); threaded into both call sites. F6 [warning] — added a REAL-DB int-spec (apps/server/test/integration/comment-delete-if-childless.int-spec.ts, + a createComment seeder): (a) childless → returns 1, row gone; (b) committed reply → returns 0, parent+reply survive; (c) CONCURRENCY — a second connection inserts a reply (FOR KEY SHARE) and commits mid-operation while deleteCommentIfChildless blocks on FOR UPDATE → asserts it returns 0 and both rows survive (a blind anti-join would lose the reply here). Ran against live Postgres — 3/3 pass. server tsc clean; comment jest 53 + int-spec 3 (live Postgres) pass. client tsc clean; comment vitest 56 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
claude code agent 227
|
8d8ecaed82 |
feat(comment): ephemeral suggestion-edits — Apply/Dismiss remove the comment (#329)
Agent suggestion-edits (comments with suggestedText, #315) piled up: Apply auto-resolved the thread, cluttering the resolved tab, and the anchors stayed in the document. Make them ephemeral: resolving (Apply OR the new Dismiss) makes the comment DISAPPEAR — hard-delete + remove the Yjs `comment` mark — UNLESS the thread has replies, in which case resolve it (preserve the discussion). Manual Resolve is unchanged. Scope: only comments with `suggestedText`. Server: - New collab event `deleteCommentMark` (collaboration.handler) mirroring resolveCommentMark, wiring the existing removeYjsMarkByAttribute to strip the anchor from the doc. - `finalizeAppliedSuggestion` forks on `hasChildren`: replies → apply + resolve (outcome 'resolved'); none → apply + hard-delete + mark removal (outcome 'deleted'). - New `dismissSuggestion` (validates top-level + suggestedText + not applied/not resolved) with the same fork; permission `canComment` (NOT canEdit — dismiss doesn't change page text); audit COMMENT_SUGGESTION_DISMISSED. New POST /comments/dismiss-suggestion; apply stays canEdit. - Both return `{ outcome: 'deleted' | 'resolved' }` so the client picks the optimistic action. Data-integrity (review F1): the shared `deleteEphemeralSuggestion` removes the anchor mark FIRST and FATALLY, then deletes the DB row only on success. The row delete is irreversible, so a mark-removal failure — including the COLLAB_DISABLE_REDIS "no live instance" hard-error — must abort the whole operation (→ 5xx, repeatable) rather than swallow the error and leave a permanent orphan anchor pointing at a deleted comment. `deleteCommentMark` is no longer best-effort (unlike resolve, where the row is kept and a failed mark is recoverable). Client: - `canShowDismiss` (canComment) alongside `canShowApply` (canEdit); a "Dismiss" button next to Apply in the suggestion block. - `useApplySuggestionMutation`/`useDismissSuggestionMutation` reconcile the cache on `outcome` ('deleted' → remove; 'resolved' → relocate to the resolved tab). - Idempotent races (review F2): BOTH apply and dismiss onError reduce 404/400 to success (comment already gone/resolved), dropping it from the cache instead of a red error — restores the #315 apply idempotency the ephemeral delete would otherwise break. - i18n Dismiss / "Не применять" (ru/en). Not done (flagged): deleteCommentMark on the normal /comments/delete path — left out (would change every non-suggestion delete + needs gateway injection; the interactive client already strips the mark via unsetComment). Out of scope per the issue. Tests: server — apply/dismiss delete-vs-resolve fork, all four dismiss state guards, the deleteCommentMark handler, controller authz (dismiss=canComment, apply=canEdit), AND a mark-removal-failure test proving the row is NOT deleted + the error propagates (F1). client — Dismiss show-conditions, outcome cache reconciliation, and 404 idempotent race for BOTH dismiss and apply (F2). Verified: server tsc clean; comment+collaboration jest 144 passed. client tsc clean; vitest 905 passed | 1 expected-fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
claude code agent 227
|
f13105333a |
feat(client): intraline diff highlighting in the suggestion before→after block (#331)
The suggestion block (#315) struck the whole `selection` red and showed the whole `suggestedText` green, so a one-letter edit (заведем→заведём) highlighted the entire line. Now only the CHANGED fragments are emphasized intraline, git-style. Pure, render-only — nothing changes in the DB/backend/MCP/IComment/mutations/ Apply/Badge. New pure `computeSuggestionDiff(old, new) => { old: Segment[], new: Segment[] }` (Segment = {text, changed}) in suggestion.ts: hybrid word+char — `diffWordsWithSpace` for the word skeleton, then `diffChars` inside an adjacent removed+added pair so only the differing letters (not the whole word) are flagged; a lone insertion/deletion is wholly changed; equal parts are common on both sides. Concatenating each side reproduces the input (lossless). Wrapped in `useMemo` on [selection, suggestedText]. comment-list-item.tsx renders per-segment spans instead of two whole <Text>; changed segments get `.suggestionChanged` (a stronger currentColor tint + bold, NO text-decoration so the old block's inherited line-through survives on the changed letters — the whole old line still reads removed, new as added). `diff@8.0.3` (jsdiff, already in the root package.json) added to apps/client/package.json (+ lockfile, additive) so the workspace resolves it; it bundles its own types. Tests: new suggestion.test.ts (one-letter ё/е; word replacement keeping the shared word common with no per-letter noise; word insertion/deletion; identical) — asserts segment text + changed flags, non-vacuous. Two pre-existing comment-list-item.test assertions switched from getByText (a single text node) to container.textContent (the new line is now multiple spans) — adapts to the intended DOM change, not a weakening. Verified: tsc --noEmit clean; client vitest 892 passed | 1 expected-fail. Visual/pixel check of the tint at the 390px comment panel needs a human (no screenshot tooling in-repo). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
claude code agent 227
|
b62db917de |
feat(comment): suggestion diff block + Apply button + mutation (#315 phase 5)
Client UI for agent comment suggestions. - IComment gains suggestedText / suggestionAppliedAt / suggestionAppliedById. - comment-list-item shows a "было → стало" block (old selection struck/red, new suggestedText green) for a top-level comment with a suggestion, plus an Apply button — gated by canShowApply(comment, canEdit): edit permission AND a suggestion AND not applied AND not resolved AND top-level. Once applied, an "Applied" badge replaces the button. - canEdit comes from page.permissions.canEdit (real edit permission, NOT the looser canComment) and is threaded through CommentListItem and nested ChildComments; fail-closed when undefined. - useApplySuggestionMutation posts to /comments/apply-suggestion; on success it writes the applied + server auto-resolve fields into the react-query cache (UI flips to Applied + resolved without a refetch); on 409 it shows a specific message with the server's currentText, else a generic error. - i18n keys added in en-US + ru-RU. Tests (comment-list-item.test.tsx + canShowApply unit suite): Apply visibility across canEdit/applied/resolved/reply, click dispatches the mutation, diff rendering. 34 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |