diff --git a/__tests__/unit/hooks/useContentEditableEditor.test.tsx b/__tests__/unit/hooks/useContentEditableEditor.test.tsx new file mode 100644 index 000000000..8d139dfbc --- /dev/null +++ b/__tests__/unit/hooks/useContentEditableEditor.test.tsx @@ -0,0 +1,83 @@ +/** + * The composer must empty itself after a post — including when it still has + * focus. + * + * This is the bug that made Ctrl+Enter look broken in production. The shortcut + * worked and the post was created; the composer simply kept showing the text, + * because the effect that syncs `content` into the contentEditable bailed out + * whenever the editor was the active element — which it always is right after + * you press a key in it. So the natural response was to press again, and the + * server rejected that as a duplicate ("You just posted this"), which looked + * like a second failure. + * + * Clicking the button never showed it: a click moves focus to the button first, + * so the guard did not apply. A shortcut and a button that disagree about + * whether the composer clears is the shape worth pinning here. + */ + +import { renderHook, act } from '@testing-library/react'; +import { useContentEditableEditor } from '@/hooks/useContentEditableEditor'; + +jest.mock('@/utils/markdownEditor', () => ({ + markdownToHtml: (md: string) => md, + htmlToMarkdown: (html: string) => html, + getSelectionRange: () => ({ start: 0, end: 0 }), + setSelectionRange: jest.fn(), +})); + +/** + * Mount the hook and give it a real element to own. `tabIndex` is what makes a + * div focusable in jsdom — without it `focus()` is a no-op and the guard under + * test would never engage, so the test would pass for the wrong reason. + */ +function mount(initial: string) { + const editor = document.createElement('div'); + editor.contentEditable = 'true'; + editor.tabIndex = 0; + document.body.appendChild(editor); + + const view = renderHook( + ({ content }: { content: string }) => + useContentEditableEditor({ content, onContentChange: jest.fn() }), + { initialProps: { content: initial } } + ); + + act(() => { + view.result.current.editorRef.current = editor; + }); + + return { editor, rerender: view.rerender }; +} + +describe('useContentEditableEditor content sync', () => { + afterEach(() => { + document.body.innerHTML = ''; + }); + + it('clears the editor when content is reset while it still has focus', () => { + const { editor, rerender } = mount('a draft'); + editor.innerHTML = 'a draft'; + editor.focus(); + // Guard the guard: if focus did not take, this test proves nothing. + expect(document.activeElement).toBe(editor); + + // What a successful post does: content state goes back to empty. + act(() => rerender({ content: '' })); + + expect(editor.textContent?.trim()).toBe(''); + }); + + it('still refuses to overwrite text being typed while focused', () => { + // The relaxed guard exists for a real reason, and only the empty case is + // exempt — an external change to non-empty text must not clobber a + // half-written post. + const { editor, rerender } = mount(''); + editor.innerHTML = 'what I am typing'; + editor.focus(); + expect(document.activeElement).toBe(editor); + + act(() => rerender({ content: 'something the app decided' })); + + expect(editor.textContent).toBe('what I am typing'); + }); +}); diff --git a/scripts/check-data-invariants.mjs b/scripts/check-data-invariants.mjs index 01bd67a14..a0a6a2e4e 100644 --- a/scripts/check-data-invariants.mjs +++ b/scripts/check-data-invariants.mjs @@ -383,6 +383,48 @@ async function checkCatHandle() { notes.push(`cat: @${CAT_HANDLE} resolves to the Cat`); } +/** + * Functions that reference a column, table or type that does not exist. + * + * Nineteen of them on 2026-08-28, silently, for months: likes, dislikes, + * replies, deleting a post and quote replies were all dead in production, along + * with four AI-withdrawal functions and both nearby searches. Every one looked + * healthy — defined, routable, called by the app — because plpgsql only plans a + * statement when it runs, so a write to a missing column raises 42703 at call + * time and never before. + * + * Nothing else in the stack sees this. Unit tests mock the database; + * check-rpc-exists proves a function is DEFINED, which all of these were; + * migration replay proves the SQL applies, and creating a function never + * validates its body. + * + * A ratchet rather than a demand for zero: eleven remain after the timeline + * ones were repaired, and each of those needs a decision rather than a + * mechanical edit (does `ai_creator_withdrawals` want a `completed_at` column, + * or should the write go?). Demanding zero tomorrow would make this red about + * work that is queued, which is how a gate teaches people to ignore it. + * `SELECT * FROM list_broken_plpgsql_functions()` names them. + */ +const BROKEN_FUNCTION_BASELINE = 11; + +async function checkBrokenFunctions() { + const count = Number(await rpc('count_broken_plpgsql_functions')); + + if (count > BROKEN_FUNCTION_BASELINE) { + violation( + 'functions.reference_missing_objects', + `${count} plpgsql function(s) reference something that does not exist, up from ` + + `${BROKEN_FUNCTION_BASELINE}. A new one will fail only when a user triggers it, with ` + + `42703 and no other symptom — run list_broken_plpgsql_functions() to see which`, + [] + ); + } else { + notes.push( + `functions: ${count} reference a missing object (baseline ${BROKEN_FUNCTION_BASELINE}, never rises)` + ); + } +} + async function checkOrphanedProfiles() { const count = Number(await rpc('count_orphaned_profiles')); @@ -516,6 +558,7 @@ async function main() { checkOrphanedProfiles, checkEmailDerivedUsernames, checkCatHandle, + checkBrokenFunctions, checkOrphanedCatConversations, checkOrphanedActors, ]; diff --git a/src/components/timeline/EditPostModal.tsx b/src/components/timeline/EditPostModal.tsx index 985fab1a3..f1ee93774 100644 --- a/src/components/timeline/EditPostModal.tsx +++ b/src/components/timeline/EditPostModal.tsx @@ -118,29 +118,15 @@ export function EditPostModal({ {/* Modal */}
- {/* Header */} -
+ {/* Header — dismissal only. The action that commits the edit lives at + the end of the form, next to the character count that says whether + it is allowed, rather than diagonally opposite it. */} +

Edit post

- -
{/* Content */} @@ -194,14 +180,33 @@ export function EditPostModal({
- {/* Character count */} -
- {charCount.toLocaleString()} / {maxChars.toLocaleString()} + {/* Character count and the commit action, in reading order: what + you have written, then whether you may save it, then Save. */} +
+
+ {charCount.toLocaleString()} / {maxChars.toLocaleString()} +
+ +
diff --git a/src/components/timeline/PostActions.tsx b/src/components/timeline/PostActions.tsx index 4158d2275..53c0abc72 100644 --- a/src/components/timeline/PostActions.tsx +++ b/src/components/timeline/PostActions.tsx @@ -1,18 +1,31 @@ 'use client'; import React from 'react'; -import { Heart, MessageCircle, Share2, ThumbsDown, Repeat2 } from 'lucide-react'; +import { Heart, MessageCircle, Share2, ThumbsDown, Repeat2, type LucideIcon } from 'lucide-react'; import { ShareModal } from '@/components/timeline/ShareModal'; import { TimelineDisplayEvent } from '@/types/timeline'; import { usePostInteractions } from '@/hooks/usePostInteractions'; import { cn } from '@/lib/utils'; -import { TIMELINE_SURFACE } from '@/config/timeline'; /** - * PostActions Component + * The row of things you can do to a post. * - * Renders interaction buttons (like, dislike, comment, repost, share) for a post. - * All business logic is delegated to the usePostInteractions hook for DRY compliance. + * Written as data rather than as five near-identical JSX blocks, because that + * is what it is: the same button with a different icon, count, colour and + * handler. The duplicated version had already drifted — the reply button + * carried a `-ml-2` none of the others did, only some had a `title`, and the + * reply count was computed inline by a three-way fallback IIFE while the rest + * read a single field. Every one of those is invisible until you look at two + * of them side by side. + * + * Layout follows X, for the reason X does it: the count sits in a fixed-width + * slot that is reserved whether or not there is a number in it. Rendering `''` + * for zero — which is what this did — means the row re-flows the moment you + * like something, and every icon after it slides sideways under the cursor + * that just clicked. Reserving the space costs nothing and the row never moves. + * + * All business logic stays in usePostInteractions. This file decides how the + * row looks and nothing else. */ interface PostActionsProps { @@ -24,8 +37,59 @@ interface PostActionsProps { isReposting?: boolean; } -function actionClassName(active: boolean, activeClassName: string) { - return cn(TIMELINE_SURFACE.iconButton, 'gap-1 px-2 text-sm', active && activeClassName); +interface ActionSpec { + key: string; + label: string; + icon: LucideIcon; + count?: number; + active: boolean; + /** Colour once active, and on hover — one accent per action, as X does. */ + accent: string; + onClick?: () => void; + disabled?: boolean; + /** Only where the meaning is not obvious from the icon. */ + title?: string; + /** The heart and repeat fill when active; share never does. */ + fillWhenActive?: boolean; +} + +function ActionButton({ spec }: { spec: ActionSpec }) { + const { icon: Icon, count, active, accent, fillWhenActive = true } = spec; + + return ( + + ); } export function PostActions({ @@ -36,7 +100,6 @@ export function PostActions({ onRepostClick, isReposting = false, }: PostActionsProps) { - // Delegate all interaction logic to the hook const { isLiking, handleLike, @@ -49,76 +112,79 @@ export function PostActions({ handleShareConfirm, } = usePostInteractions({ event, onUpdate, onAddEvent }); - return ( - <> -
- {/* Reply */} - - - {/* Repost */} - + // One place decides what a reply count is. It used to be an inline IIFE with + // three fallbacks in this file only, so any other reader of the same number + // could disagree with the row rendering it. + const replyCount = + event.replyCount ?? + (Array.isArray(event.replies) ? event.replies.length : undefined) ?? + event.commentsCount ?? + 0; - {/* Like */} - + const actions: ActionSpec[] = [ + { + key: 'reply', + label: 'Reply', + icon: MessageCircle, + count: replyCount, + active: false, + accent: 'text-fg-primary', + onClick: onToggleComments, + }, + { + key: 'repost', + label: 'Repost', + icon: Repeat2, + count: event.repostsCount || 0, + active: !!event.userReposted, + accent: 'text-status-positive', + onClick: onRepostClick, + disabled: isReposting, + }, + { + key: 'like', + label: 'Like', + icon: Heart, + count: event.likesCount || 0, + active: !!event.userLiked, + accent: 'text-status-negative', + onClick: handleLike, + disabled: isLiking, + }, + { + key: 'dislike', + label: 'Dislike', + icon: ThumbsDown, + count: event.dislikesCount || 0, + active: !!event.userDisliked, + accent: 'text-status-warning', + onClick: handleDislike, + disabled: isDisliking, + title: 'Dislike this post — helps the crowd flag scams', + }, + { + key: 'share', + label: 'Share', + icon: Share2, + active: !!event.userShared, + accent: 'text-fg-primary', + onClick: handleShareOpen, + disabled: isSharing, + fillWhenActive: false, + }, + ]; - {/* Dislike */} - - - {/* Share */} - + return ( + <> + {/* `-ml-1.5` pulls the first icon's round target back so the ICON aligns + with the text above it, not the padding around it. Applied once, to + the row, instead of to one button and not the others. */} +
+ {actions.map(spec => ( + + ))}
- {/* Share Modal */}
-
-
- - {remainingCharacters} - - characters left -
-
- - -
-
- - {/* Original Post Preview (X-style) */} + {/* The post being quoted sits directly under what you are writing, + as it does on X and as it will in the timeline once posted. It + used to come AFTER the buttons, which put the action row between + the composer and the thing it refers to and broke the reading + order — write, see what you are quoting, then act. */}
+ +
+ {/* Only counts down once it matters. A number sitting there from + the first keystroke reads as a limit you are approaching. */} + + {remainingCharacters <= 20 ? `${remainingCharacters} left` : ''} + +
+ + +
+
diff --git a/src/hooks/useContentEditableEditor.ts b/src/hooks/useContentEditableEditor.ts index 0a68c3797..d9955171d 100644 --- a/src/hooks/useContentEditableEditor.ts +++ b/src/hooks/useContentEditableEditor.ts @@ -44,16 +44,41 @@ export function useContentEditableEditor({ useEffect(() => { const editor = editorRef.current; - if (!editor || isComposing || document.activeElement === editor) { + if (!editor || isComposing) { + return; + } + + // Normally an external content change must not overwrite what someone is + // actively typing, so a focused editor is left alone. Being RESET to empty + // is the exception, and it has to be: that is what a successful post does, + // and after Ctrl+Enter the editor still has focus. + // + // Without this exception the composer kept the text of a post that had just + // been created. It looked like the shortcut had done nothing, so the + // natural response was to press it again — which the server rejected as a + // duplicate ("You just posted this"), making it look broken twice over. + // Clicking the button never showed this because clicking moves focus to the + // button first. + // + // Safe as an exception because there is nothing to clobber: the text being + // removed is text the app has already decided to remove. + const isReset = content.trim().length === 0; + if (!isReset && document.activeElement === editor) { return; } const currentHtml = editor.innerHTML.replace(/\s+/g, ' ').trim(); const expectedHtml = markdownToHtml(content).replace(/\s+/g, ' ').trim(); - if (currentHtml !== expectedHtml && expectedHtml !== '
') { + // `expectedHtml !== '
'` stops an empty render from wiping the editor + // mid-keystroke, but it would also block a deliberate reset, so a reset is + // allowed past it for the same reason as above. + if (currentHtml !== expectedHtml && (isReset || expectedHtml !== '
')) { const selection = getSelectionRange(editor); - const wasFocused = document.activeElement === editor; + // On a reset there is no position left to restore — the old offsets point + // into text that no longer exists — so the caret simply goes to the start + // of the now-empty composer, which is where someone would begin typing. + const wasFocused = !isReset && document.activeElement === editor; editor.innerHTML = sanitize(expectedHtml || '
'); diff --git a/supabase/migrations/20260828110000_reactions_write_a_column_that_does_not_exist.sql b/supabase/migrations/20260828110000_reactions_write_a_column_that_does_not_exist.sql new file mode 100644 index 000000000..d66bcc273 --- /dev/null +++ b/supabase/migrations/20260828110000_reactions_write_a_column_that_does_not_exist.sql @@ -0,0 +1,436 @@ +-- Likes, dislikes and replies have never worked. This is why. +-- +-- Six functions end with a line writing a column that does not exist: +-- +-- -- Update timeline_events.like_count if column exists +-- UPDATE timeline_events SET like_count = v_count WHERE id = p_event_id; +-- +-- `timeline_events` has no count columns at all — never has. The comment says +-- "if column exists", but plpgsql has no such conditional: the statement is +-- planned when the function runs and raises 42703, `column "like_count" of +-- relation "timeline_events" does not exist`. +-- +-- Because a function body is one transaction, the exception rolls back the +-- INSERT that ran three lines earlier. So the like is written, then unwritten, +-- and the caller gets an error. The UI does the rest of the damage invisibly: +-- it applies an optimistic update, sees `success: false`, and rolls back — so +-- the heart fills for ~150ms and empties again. Nothing is logged where anyone +-- would look, and the button never appears broken, only unresponsive. +-- +-- Verified in production 2026-08-28 by clicking Like on orangecat.ch/timeline +-- and reading the response: HTTP 400, code 42703. +-- +-- Affected, all reachable from the app: +-- like_timeline_event / unlike_timeline_event → likes +-- dislike_timeline_event / undislike_timeline_event → dislikes +-- add_timeline_comment → replies +-- delete_timeline_comment → deleting a reply +-- +-- THE FIX is to delete that line, and nothing else. Every one of these already +-- upserts `timeline_event_stats`, which is the real home for counts +-- (like_count, dislike_count, comment_count, share_count, view_count) and what +-- the read path already reads. The bad line was a second, non-existent copy of +-- a number that was being stored correctly one line above — so removing it +-- loses nothing and restores the single source of truth. +-- +-- Bodies below are the LIVE definitions, taken from pg_get_functiondef, with +-- only that line (and its comment) removed. Reproducing them from the baseline +-- migration instead would risk reverting whatever else has been replaced since. + +CREATE OR REPLACE FUNCTION public.add_timeline_comment(p_event_id uuid, p_user_id uuid, p_content text, p_parent_comment_id uuid DEFAULT NULL::uuid) + RETURNS TABLE(comment_id uuid, comment_count integer) + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_comment_id UUID; + v_count INTEGER; +BEGIN + -- Verify user is authenticated + IF auth.uid() != p_user_id THEN + RAISE EXCEPTION 'Unauthorized'; + END IF; + + -- Insert the comment + INSERT INTO timeline_comments (event_id, user_id, content, parent_comment_id) + VALUES (p_event_id, p_user_id, p_content, p_parent_comment_id) + RETURNING id INTO v_comment_id; + + -- Get the updated count (only top-level comments) + SELECT COUNT(*)::INTEGER INTO v_count + FROM timeline_comments + WHERE event_id = p_event_id AND is_deleted = false AND parent_comment_id IS NULL; + + -- Update cached stats + INSERT INTO timeline_event_stats (event_id, comment_count, updated_at) + VALUES (p_event_id, v_count, NOW()) + ON CONFLICT (event_id) DO UPDATE SET comment_count = v_count, updated_at = NOW(); + + + RETURN QUERY SELECT v_comment_id, v_count; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.delete_timeline_comment(p_comment_id uuid, p_user_id uuid) + RETURNS boolean + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_event_id UUID; + v_count INTEGER; +BEGIN + -- Verify user is authenticated + IF auth.uid() != p_user_id THEN + RAISE EXCEPTION 'Unauthorized'; + END IF; + + -- Get event_id before deleting + SELECT event_id INTO v_event_id + FROM timeline_comments WHERE id = p_comment_id AND user_id = p_user_id; + + IF v_event_id IS NULL THEN + RETURN FALSE; + END IF; + + -- Soft delete the comment + UPDATE timeline_comments + SET is_deleted = true, deleted_at = NOW() + WHERE id = p_comment_id AND user_id = p_user_id; + + -- Update count + SELECT COUNT(*)::INTEGER INTO v_count + FROM timeline_comments + WHERE event_id = v_event_id AND is_deleted = false AND parent_comment_id IS NULL; + + -- Update cached stats + UPDATE timeline_event_stats SET comment_count = v_count, updated_at = NOW() + WHERE event_id = v_event_id; + + + RETURN TRUE; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.dislike_timeline_event(p_event_id uuid, p_user_id uuid) + RETURNS TABLE(dislike_count integer) + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_count INTEGER; +BEGIN + -- Verify user is authenticated + IF auth.uid() != p_user_id THEN + RAISE EXCEPTION 'Unauthorized'; + END IF; + + -- Insert the dislike (ignore if already exists) + INSERT INTO timeline_dislikes (event_id, user_id) + VALUES (p_event_id, p_user_id) + ON CONFLICT (event_id, user_id) DO NOTHING; + + -- Remove any existing like + DELETE FROM timeline_likes + WHERE event_id = p_event_id AND user_id = p_user_id; + + -- Get the updated count + SELECT COUNT(*)::INTEGER INTO v_count + FROM timeline_dislikes WHERE event_id = p_event_id; + + -- Update cached stats + INSERT INTO timeline_event_stats (event_id, dislike_count, updated_at) + VALUES (p_event_id, v_count, NOW()) + ON CONFLICT (event_id) DO UPDATE SET dislike_count = v_count, updated_at = NOW(); + + + RETURN QUERY SELECT v_count; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.like_timeline_event(p_event_id uuid, p_user_id uuid) + RETURNS TABLE(like_count integer) + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_count INTEGER; +BEGIN + -- Verify user is authenticated + IF auth.uid() != p_user_id THEN + RAISE EXCEPTION 'Unauthorized'; + END IF; + + -- Insert the like (ignore if already exists) + INSERT INTO timeline_likes (event_id, user_id) + VALUES (p_event_id, p_user_id) + ON CONFLICT (event_id, user_id) DO NOTHING; + + -- Remove any existing dislike + DELETE FROM timeline_dislikes + WHERE event_id = p_event_id AND user_id = p_user_id; + + -- Get the updated count + SELECT COUNT(*)::INTEGER INTO v_count + FROM timeline_likes WHERE event_id = p_event_id; + + -- Update cached stats + INSERT INTO timeline_event_stats (event_id, like_count, updated_at) + VALUES (p_event_id, v_count, NOW()) + ON CONFLICT (event_id) DO UPDATE SET like_count = v_count, updated_at = NOW(); + + + RETURN QUERY SELECT v_count; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.undislike_timeline_event(p_event_id uuid, p_user_id uuid) + RETURNS TABLE(dislike_count integer) + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_count INTEGER; +BEGIN + -- Verify user is authenticated + IF auth.uid() != p_user_id THEN + RAISE EXCEPTION 'Unauthorized'; + END IF; + + -- Remove the dislike + DELETE FROM timeline_dislikes + WHERE event_id = p_event_id AND user_id = p_user_id; + + -- Get the updated count + SELECT COUNT(*)::INTEGER INTO v_count + FROM timeline_dislikes WHERE event_id = p_event_id; + + -- Update cached stats + INSERT INTO timeline_event_stats (event_id, dislike_count, updated_at) + VALUES (p_event_id, v_count, NOW()) + ON CONFLICT (event_id) DO UPDATE SET dislike_count = v_count, updated_at = NOW(); + + + RETURN QUERY SELECT v_count; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.unlike_timeline_event(p_event_id uuid, p_user_id uuid) + RETURNS TABLE(like_count integer) + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_count INTEGER; +BEGIN + -- Verify user is authenticated + IF auth.uid() != p_user_id THEN + RAISE EXCEPTION 'Unauthorized'; + END IF; + + -- Remove the like + DELETE FROM timeline_likes + WHERE event_id = p_event_id AND user_id = p_user_id; + + -- Get the updated count + SELECT COUNT(*)::INTEGER INTO v_count + FROM timeline_likes WHERE event_id = p_event_id; + + -- Update cached stats + INSERT INTO timeline_event_stats (event_id, like_count, updated_at) + VALUES (p_event_id, v_count, NOW()) + ON CONFLICT (event_id) DO UPDATE SET like_count = v_count, updated_at = NOW(); + + + RETURN QUERY SELECT v_count; +END; +$function$; + + +-- --------------------------------------------------------------------------- +-- Deleting a post has never worked either, for two independent reasons. +-- +-- One: the ownership check reads `SELECT user_id FROM profiles WHERE role = +-- 'admin'`. `profiles` has neither column — it keys on `id` and has no `role` +-- at all — so the IF raised 42703 on every call and no post was ever deleted. +-- The admin escape hatch it was reaching for has therefore never existed; +-- removing it takes away nothing that ever worked, and leaves the honest rule: +-- you can delete your own posts. A real moderator capability should be built +-- deliberately, against a table that exists, not inherited from a line that +-- always threw. +-- +-- Two, and this one would have bitten even after fixing the first: the function +-- set only `deleted_at`, while every read path filters on `is_deleted = false` +-- (services/timeline/queries/*). A post "deleted" this way would have stayed +-- fully visible. Confirmed against production: 102 events have both flags set, +-- and ZERO have `deleted_at` without `is_deleted` — nothing has ever taken this +-- path. Both flags are now set together, and the reason goes in the +-- `deletion_reason` COLUMN that already exists rather than being stuffed into +-- metadata beside it. +-- +-- The trailing "decrement the parent's reply_count" block is dropped: that +-- column does not exist either, so it never ran. +CREATE OR REPLACE FUNCTION public.soft_delete_timeline_event(event_id uuid, reason text DEFAULT NULL::text) + RETURNS boolean + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_actor_id UUID; + v_rows INTEGER := 0; +BEGIN + v_actor_id := auth.uid(); + + IF v_actor_id IS NULL THEN + RAISE EXCEPTION 'Authentication required'; + END IF; + + UPDATE timeline_events + SET is_deleted = TRUE, + deleted_at = NOW(), + deletion_reason = reason, + updated_at = NOW() + WHERE id = event_id + AND actor_id = v_actor_id + AND is_deleted IS NOT TRUE; + + GET DIAGNOSTICS v_rows = ROW_COUNT; + + IF v_rows = 0 THEN + -- Deleting something already deleted is the user's intent already + -- satisfied, not a failure to report. Only a post that is not theirs (or + -- does not exist) is an error. + IF EXISTS ( + SELECT 1 FROM timeline_events + WHERE id = event_id AND actor_id = v_actor_id + ) THEN + RETURN TRUE; + END IF; + RAISE EXCEPTION 'Event not found or access denied'; + END IF; + + RETURN TRUE; +END; +$function$; + +-- --------------------------------------------------------------------------- +-- A quote reply could not be created, and would have rendered blank if it had. +-- +-- `timeline_events.content` is jsonb and the function assigned the text +-- parameter straight into it, which plpgsql_check reports as "column content is +-- of type jsonb but expression is of type text". It also incremented +-- `quote_count`, another column that does not exist. +-- +-- The rendering half matters just as much: PostContent reads +-- `event.description`, and this function never wrote one. So a quote reply that +-- somehow succeeded would have appeared as an empty post — which is the shape +-- of "reposts look broken" rather than "reposts error". Both are written now, +-- `description` for the reader and `content` in the {text: …} shape the rest of +-- the timeline uses, so the two cannot disagree about what the post says. +CREATE OR REPLACE FUNCTION public.create_quote_reply(p_parent_event_id uuid, p_actor_id uuid, p_content text, p_quoted_content text, p_visibility text DEFAULT 'public'::text) + RETURNS uuid + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_new_event_id UUID; + v_parent_actor_id UUID; +BEGIN + IF auth.uid() != p_actor_id THEN + RAISE EXCEPTION 'Actor mismatch'; + END IF; + + SELECT actor_id INTO v_parent_actor_id + FROM timeline_events + WHERE id = p_parent_event_id + AND is_deleted IS NOT TRUE; + + IF v_parent_actor_id IS NULL THEN + RAISE EXCEPTION 'Parent event not found'; + END IF; + + -- `subject_type` is NOT NULL and was never supplied, so this INSERT could not + -- have succeeded on any input — a third failure, hiding behind the first two + -- and invisible to plpgsql_check, which does not evaluate constraints. Every + -- one of the 1,406 existing rows sets subject_type and subject_id; a post + -- authored by a person is ('profile', that person), which is what the + -- formatter already assumes when it defaults subjectType to 'profile'. + INSERT INTO timeline_events ( + event_type, + actor_id, + subject_type, + subject_id, + title, + description, + content, + parent_event_id, + visibility, + metadata, + created_at, + updated_at + ) VALUES ( + 'quote_reply', + p_actor_id, + 'profile', + p_actor_id, + 'Quoted a post', + p_content, + jsonb_build_object('text', p_content), + p_parent_event_id, + p_visibility, + jsonb_build_object( + 'quoted_content', p_quoted_content, + 'quoted_actor_id', v_parent_actor_id + ), + NOW(), + NOW() + ) + RETURNING id INTO v_new_event_id; + + -- The notification insert named four columns `notifications` does not have + -- (actor_id, title, data) and used a `type` its CHECK constraint forbids + -- ('quote_reply'). Only the first of those is visible to plpgsql_check — a + -- constraint violation is a runtime failure — so this had two more ways to + -- fail after the obvious one was fixed. + -- + -- The real shape is (user_id, type, message, metadata, is_read, created_at, + -- read_at, action_url). `comment` is the allowed type that means what this + -- means: somebody responded to your post. The quote-ness is kept in metadata + -- rather than invented as a new type, because adding a type means widening + -- the constraint AND teaching the notification UI to render it — a deliberate + -- change, not a side effect of repairing a broken function. + IF v_parent_actor_id != p_actor_id THEN + INSERT INTO notifications ( + user_id, + type, + message, + metadata, + action_url, + created_at + ) VALUES ( + v_parent_actor_id, + 'comment', + LEFT(p_content, 100), + jsonb_build_object( + 'kind', 'quote_reply', + 'event_id', v_new_event_id, + 'parent_event_id', p_parent_event_id, + 'actor_id', p_actor_id + ), + '/post/' || v_new_event_id::text, + NOW() + ) + ON CONFLICT DO NOTHING; + END IF; + + RETURN v_new_event_id; +END; +$function$; diff --git a/supabase/migrations/20260828120000_detect_functions_that_reference_nothing.sql b/supabase/migrations/20260828120000_detect_functions_that_reference_nothing.sql new file mode 100644 index 000000000..7691d2f16 --- /dev/null +++ b/supabase/migrations/20260828120000_detect_functions_that_reference_nothing.sql @@ -0,0 +1,81 @@ +-- Make "this function writes a column that does not exist" a thing we can see. +-- +-- Nineteen functions were in that state on 2026-08-28, and had been for as long +-- as anyone can tell. Likes, dislikes, replies, deleting a post and quote +-- replies were all dead in production; four AI-withdrawal functions and both +-- nearby-search functions are dead too. Every one of them looked fine: the +-- function exists, `\df` lists it, PostgREST routes to it, and the app calls it +-- happily. plpgsql only plans a statement when it RUNS, so a reference to a +-- column that was never added raises 42703 at call time and at no other moment. +-- +-- Nothing in the stack could have caught it. Unit tests mock the database. The +-- RPC-existence gate (scripts/check-rpc-exists.mjs) proves a function is +-- DEFINED, which every one of these was. Migration replay proves the SQL +-- applies, which it does — creating a function never validates its body. +-- Existence and correctness came apart, and only correctness matters. +-- +-- plpgsql_check reads every function body against the live schema and reports +-- exactly this. Exposed here as a COUNT over PostgREST, matching +-- count_email_derived_usernames, so the nightly data-invariant gate can ratchet +-- it down and never up. +-- +-- LIMITS, stated because a gate that is trusted beyond its reach is worse than +-- none. This is a STATIC check: it sees undefined columns, tables and type +-- mismatches. It does NOT evaluate constraints, so a NOT NULL column left +-- unset or a value outside a CHECK still fails only at runtime — both of which +-- create_quote_reply also did, hiding behind the static error until it was +-- fixed. Trigger functions are skipped because plpgsql_check needs a relation +-- to check them against. + +CREATE EXTENSION IF NOT EXISTS plpgsql_check; + +CREATE OR REPLACE FUNCTION public.count_broken_plpgsql_functions() +RETURNS bigint +LANGUAGE sql +SECURITY DEFINER +SET search_path TO 'public' +AS $$ + SELECT count(DISTINCT p.oid)::bigint + FROM pg_proc p + JOIN pg_namespace n ON n.oid = p.pronamespace + JOIN pg_language l ON l.oid = p.prolang + CROSS JOIN LATERAL plpgsql_check_function_tb(p.oid) cf + WHERE n.nspname = 'public' + AND l.lanname = 'plpgsql' + AND p.prorettype <> 'trigger'::regtype + AND cf.level = 'error'; +$$; + +-- Same lock-down as the other invariant helpers: the list of broken functions +-- is a map of what is currently exploitable-by-accident, so it returns a number +-- and only service_role may ask. +REVOKE ALL ON FUNCTION public.count_broken_plpgsql_functions() FROM PUBLIC; +REVOKE ALL ON FUNCTION public.count_broken_plpgsql_functions() FROM anon, authenticated; +GRANT EXECUTE ON FUNCTION public.count_broken_plpgsql_functions() TO service_role; + +COMMENT ON FUNCTION public.count_broken_plpgsql_functions() IS + 'How many plpgsql functions reference a column, table or type that does not exist. Static only — constraints still fail at runtime. A ratchet for check-data-invariants.mjs: it may fall or hold, never rise.'; + +-- The names, for a human fixing them. Kept separate from the count so the gate +-- can stay a number while a person can still ask "which ones?". +CREATE OR REPLACE FUNCTION public.list_broken_plpgsql_functions() +RETURNS TABLE(function_name text, problem text) +LANGUAGE sql +SECURITY DEFINER +SET search_path TO 'public' +AS $$ + SELECT p.proname::text, cf.message::text + FROM pg_proc p + JOIN pg_namespace n ON n.oid = p.pronamespace + JOIN pg_language l ON l.oid = p.prolang + CROSS JOIN LATERAL plpgsql_check_function_tb(p.oid) cf + WHERE n.nspname = 'public' + AND l.lanname = 'plpgsql' + AND p.prorettype <> 'trigger'::regtype + AND cf.level = 'error' + ORDER BY 1; +$$; + +REVOKE ALL ON FUNCTION public.list_broken_plpgsql_functions() FROM PUBLIC; +REVOKE ALL ON FUNCTION public.list_broken_plpgsql_functions() FROM anon, authenticated; +GRANT EXECUTE ON FUNCTION public.list_broken_plpgsql_functions() TO service_role;