fix(quotes): handle cross-thread quotes (publish + hover preview) (#1153)

* fix(quotes): publish only same-thread quoted cids

Quoting a post from another thread failed to publish with "One or more
quoted CIDs are not under the same post". That message is the protocol
error ERR_QUOTED_CID_NOT_UNDER_POST from @pkcprotocol/pkc-js, not a custom
5chan error: a reply's quotedCids must all live under the reply's own
thread.

The reply publish path resolved every >>number quotelink to a CID through
the board-scoped number map and sent them all as quotedCids, so any
cross-thread quotelink made the protocol reject the whole post.

Track each comment's thread (postCid) in usePostNumberStore and filter the
merged quotedCids to same-thread entries before publishing. Cross-thread
quotelinks still render and navigate via their >>number text; only the
published metadata changes. Same-thread replies resolved on demand by the
external-quote resolver are preserved because the resolver registers what
it finds.

Regression introduced in 9f14a6326 (populate quotedCids when publishing
replies with quote references).

* fix(quotes): show hover preview for cross-thread quotelinks

Hovering a >>number quote whose thread was not loaded showed nothing: the
quotelink rendered as an inert span with no floating preview. NumberQuoteLink
only fell back to the lazy resolver when the number->cid mapping was unknown.
When the cid was known but the comment body was not cached, it rendered
ReplyQuotePreview with an undefined comment, which is treated as pending
resolution and rendered as inert text that never fetched the body (the lookup
used onlyIfCached).

Load the body lazily on hover instead: NumberQuoteLink keeps onlyIfCached until
the quotelink is hovered, and ReplyQuotePreview's pending quotelink is now
hoverable when its cid is known so it can request that fetch and prime the
floating-preview position. Once the body resolves the preview appears without
re-hovering. Same-thread quotes stay cached, so this never fetches for them.

* fix(quotes): filter the final quotedCids payload to same-thread

Apply the same-thread filter to the merged publish options rather than only the
derived subset, so any quotedCids carried on stored publishCommentOptions cannot
bypass the guard. Defense in depth for the protocol's same-thread requirement
(ERR_QUOTED_CID_NOT_UNDER_POST); addresses PR review feedback.
This commit is contained in:
Tommaso Casaburi
2026-06-05 17:45:42 +07:00
committed by GitHub
parent 12977fa24a
commit 69ee15786b
9 changed files with 223 additions and 18 deletions
@@ -191,17 +191,20 @@ vi.mock('../../reply-quote-preview', () => ({
default: ({
isOP,
isQuotelinkUnavailable,
quotelinkCid,
quotelinkNumber,
quotelinkReply,
}: {
isOP?: boolean;
isQuotelinkUnavailable?: boolean;
quotelinkCid?: string;
quotelinkNumber?: number;
quotelinkReply?: TestComment;
}) =>
createElement(
'span',
{
'data-cid': quotelinkCid ?? '',
'data-number': String(quotelinkNumber ?? ''),
'data-op': String(Boolean(isOP)),
'data-testid': 'reply-quote-preview',
@@ -605,6 +608,26 @@ describe('Markdown', () => {
expect(lazyLinks.map((node) => node.textContent)).toEqual(['>>42', '>>>/fit/77']);
});
it('passes the known cid to the quote preview when a cross-thread number resolves but its body is uncached', async () => {
// #2 lives in another thread: its number->cid mapping is registered, but the comment body is not
// cached. It must NOT render the lazy external link (cid is known); it renders the quote preview
// with the cid so the body can be fetched on hover instead of staying inert.
testState.numberToCid = { 'music-posting.eth': { 2: 'comment-2' } };
testState.cidToNumber = { 'comment-2': 2 };
await renderMarkdown({
content: '>>2',
postCid: 'thread-cid',
communityAddress: 'music-posting.eth',
});
expect(container.querySelector('[data-testid="external-number-quote-link"]')).toBeNull();
const preview = container.querySelector('[data-testid="reply-quote-preview"]');
expect(preview?.getAttribute('data-number')).toBe('2');
expect(preview?.getAttribute('data-cid')).toBe('comment-2');
expect(preview?.textContent).toBe('missing');
});
it('shows youtube thumbnails in the floating hover preview instead of loading the iframe', async () => {
testState.embeddableHosts = new Set(['www.youtube.com']);
testState.mediaInfoByHref = {
+16 -2
View File
@@ -422,10 +422,14 @@ interface MarkdownProps {
}
const NumberQuoteLink = ({ number, threadPostCid, communityAddress }: { number: number; threadPostCid?: string; communityAddress?: string }) => {
// A cross-thread quote's number->cid mapping can be known locally while its comment body is not
// yet cached. Fetch the body lazily on hover so the floating preview can show, instead of leaving
// the quotelink inert. Same-thread quotes stay cached, so this never fetches for them.
const [resolveRequested, setResolveRequested] = useState(false);
const cid = usePostNumberStore((state) => getCidForPostNumber(state.numberToCid, communityAddress, number));
const threadPostNumber = usePostNumberStore((state) => (threadPostCid ? state.cidToNumber[threadPostCid] : undefined));
const commentFromStore = useCommunitiesPagesStore((state) => (cid ? state.comments[cid] : undefined));
const commentFromHook = useComment({ commentCid: cid, onlyIfCached: true });
const commentFromHook = useComment({ commentCid: cid, onlyIfCached: !resolveRequested });
const comment = commentFromHook?.number !== undefined ? commentFromHook : commentFromStore;
const isOP = Boolean((threadPostCid && cid === threadPostCid) || (threadPostNumber !== undefined && number === threadPostNumber));
@@ -449,7 +453,17 @@ const NumberQuoteLink = ({ number, threadPostCid, communityAddress }: { number:
);
}
return <ReplyQuotePreview isQuotelinkReply={true} quotelinkReply={comment} quotelinkNumber={number} isOP={isOP} showTrailingBreak={false} />;
return (
<ReplyQuotePreview
isQuotelinkReply={true}
quotelinkReply={comment}
quotelinkNumber={number}
quotelinkCid={cid}
onResolveQuotelink={resolveRequested ? undefined : () => setResolveRequested(true)}
isOP={isOP}
showTrailingBreak={false}
/>
);
};
const AnchorLink = ({ href, text }: { href: string; text: string }) => {
@@ -596,4 +596,40 @@ describe('ReplyQuotePreview', () => {
expect(container.textContent).toContain('>>42');
expect(Array.from(container.querySelectorAll('a')).some((anchor) => anchor.textContent?.includes('#'))).toBe(false);
});
it('requests a lazy fetch on hover for a pending quotelink with a known cid, then shows the preview once it resolves', async () => {
testState.quoteAvailability = 'unresolved';
const onResolveQuotelink = vi.fn();
// Pending: the cross-thread body is not loaded yet, but the cid is known.
await renderPreview({
isQuotelinkReply: true,
quotelinkNumber: 2,
quotelinkReply: undefined,
quotelinkCid: 'cross-thread-cid',
onResolveQuotelink,
});
const pendingQuote = Array.from(container.querySelectorAll('span')).find((node) => node.textContent === '>>2');
expect(pendingQuote).toBeTruthy();
expect(document.querySelector('[data-testid="post-preview"]')).toBeNull();
// Hovering requests the lazy fetch (and primes the hover state for the quoted cid).
await act(async () => {
pendingQuote?.dispatchEvent(new MouseEvent('mouseover', { bubbles: true }));
});
expect(onResolveQuotelink).toHaveBeenCalledTimes(1);
// Once the body resolves, the floating preview appears without needing to re-hover.
testState.quoteAvailability = 'available';
await renderPreview({
isQuotelinkReply: true,
quotelinkNumber: 2,
quotelinkReply: { cid: 'cross-thread-cid', number: 2, communityAddress: 'music-posting.eth' },
quotelinkCid: 'cross-thread-cid',
onResolveQuotelink,
});
expect(document.querySelector('[data-testid="post-preview"]')?.textContent).toBe('cross-thread-cid');
});
});
@@ -19,6 +19,10 @@ interface ReplyQuotePreviewProps {
isQuotelinkReply?: boolean;
quotelinkReply?: Comment;
quotelinkNumber?: number;
// Known cid of the quoted comment even when its body has not been loaded yet, so a pending
// quotelink can still be hovered (to position the preview) and request a lazy fetch.
quotelinkCid?: string;
onResolveQuotelink?: () => void;
isQuotelinkUnavailable?: boolean;
isOP?: boolean;
showTrailingBreak?: boolean;
@@ -99,6 +103,8 @@ const DesktopQuotePreview = ({
backlinkReply,
quotelinkReply,
quotelinkNumber,
quotelinkCid,
onResolveQuotelink,
isBacklinkReply,
isQuotelinkReply,
isQuotelinkUnavailable,
@@ -252,7 +258,21 @@ const DesktopQuotePreview = ({
{quotelinkUnavailable ? (
<span className={quotelinkClassName}>{quotelinkLabel}</span>
) : quotelinkPendingResolution ? (
<span className={styles.quoteLink}>{quotelinkLabel}</span>
quotelinkCid ? (
<span
ref={refs.setReference}
className={styles.quoteLink}
onMouseOver={() => {
onResolveQuotelink?.();
handleMouseOver(quotelinkCid);
}}
onMouseLeave={() => handleMouseLeave(quotelinkCid)}
>
{quotelinkLabel}
</span>
) : (
<span className={styles.quoteLink}>{quotelinkLabel}</span>
)
) : (
<Link
to={quotelinkRoute}
@@ -283,6 +303,8 @@ const MobileQuotePreview = ({
backlinkReply,
quotelinkReply,
quotelinkNumber,
quotelinkCid,
onResolveQuotelink,
isBacklinkReply,
isQuotelinkReply,
isQuotelinkUnavailable,
@@ -399,16 +421,28 @@ const MobileQuotePreview = ({
isUnavailable: quotelinkUnavailable,
});
const isOwnQuotelink = useIsOwnQuotelink(normalizedQuotelinkReply);
// When the body is not loaded yet but the cid is known, still allow hover/focus so the lazy fetch
// can be requested and the preview can position itself once the comment resolves.
const lazyResolvableQuotelinkCid = quotelinkPendingResolution && quotelinkCid ? quotelinkCid : undefined;
const quotelinkHoverCid = resolvedQuotelinkCid ?? lazyResolvableQuotelinkCid;
const isQuotelinkHoverable = !quotelinkUnavailable && Boolean(quotelinkHoverCid);
const handleQuotelinkPointerEnter = () => {
if (lazyResolvableQuotelinkCid) {
onResolveQuotelink?.();
}
handleMouseOver(quotelinkHoverCid);
};
const handleQuotelinkPointerLeave = () => handleMouseLeave(quotelinkHoverCid ?? null);
const replyQuotelink = (
<>
<span
ref={quotelinkUnavailable || quotelinkPendingResolution ? undefined : refs.setReference}
ref={isQuotelinkHoverable ? refs.setReference : undefined}
className={quotelinkPendingResolution ? styles.quoteLink : quotelinkClassName}
onMouseOver={quotelinkUnavailable || quotelinkPendingResolution ? undefined : () => handleMouseOver(resolvedQuotelinkCid)}
onFocus={quotelinkUnavailable || quotelinkPendingResolution ? undefined : () => handleMouseOver(resolvedQuotelinkCid)}
onMouseLeave={quotelinkUnavailable || quotelinkPendingResolution ? undefined : () => handleMouseLeave(resolvedQuotelinkCid)}
onBlur={quotelinkUnavailable || quotelinkPendingResolution ? undefined : () => handleMouseLeave(resolvedQuotelinkCid)}
onMouseOver={isQuotelinkHoverable ? handleQuotelinkPointerEnter : undefined}
onFocus={isQuotelinkHoverable ? handleQuotelinkPointerEnter : undefined}
onMouseLeave={isQuotelinkHoverable ? handleQuotelinkPointerLeave : undefined}
onBlur={isQuotelinkHoverable ? handleQuotelinkPointerLeave : undefined}
>
{formatQuoteNumber(resolvedQuotelinkNumber)}
{isOP && ' (OP)'}
@@ -449,6 +483,8 @@ const ReplyQuotePreview = ({
backlinkReply,
quotelinkReply,
quotelinkNumber,
quotelinkCid,
onResolveQuotelink,
isBacklinkReply,
isQuotelinkReply,
isQuotelinkUnavailable,
@@ -462,6 +498,8 @@ const ReplyQuotePreview = ({
backlinkReply={backlinkReply}
quotelinkReply={quotelinkReply}
quotelinkNumber={quotelinkNumber}
quotelinkCid={quotelinkCid}
onResolveQuotelink={onResolveQuotelink}
isBacklinkReply={isBacklinkReply}
isQuotelinkReply={isQuotelinkReply}
isQuotelinkUnavailable={isQuotelinkUnavailable}
@@ -473,6 +511,8 @@ const ReplyQuotePreview = ({
backlinkReply={backlinkReply}
quotelinkReply={quotelinkReply}
quotelinkNumber={quotelinkNumber}
quotelinkCid={quotelinkCid}
onResolveQuotelink={onResolveQuotelink}
isBacklinkReply={isBacklinkReply}
isQuotelinkReply={isQuotelinkReply}
isQuotelinkUnavailable={isQuotelinkUnavailable}
+39 -1
View File
@@ -80,7 +80,8 @@ describe('usePublishReply', () => {
testState.lastPublishOptions = undefined;
testState.publishAuthorBlockedReason = undefined;
useChallengesStore.setState({ challenges: [] });
usePostNumberStore.setState({ cidToNumber: {}, numberToCid: { 'music.eth': { 12: 'quoted-cid' } } });
// 'quoted-cid' (post #12) lives under the reply's own thread (postCid falls back to 'parent-cid').
usePostNumberStore.setState({ cidToNumber: {}, numberToCid: { 'music.eth': { 12: 'quoted-cid' } }, cidToPostCid: { 'quoted-cid': 'parent-cid' } });
usePublishReplyStore.setState({
author: {},
challengeRequest: {},
@@ -127,12 +128,49 @@ describe('usePublishReply', () => {
});
});
it('drops cross-thread quoted cids so the protocol does not reject the publish', async () => {
// Post #12 resolves to a comment that lives under a different thread; quoting it must not be
// published as a quotedCid (ERR_QUOTED_CID_NOT_UNDER_POST), even though the >>12 text stays.
usePostNumberStore.setState({ cidToPostCid: { 'quoted-cid': 'another-thread-cid' } });
await act(async () => {
latestValue.setPublishReplyOptions({
content: 'Replying to >>12',
} as never);
});
expect(testState.lastPublishOptions).toMatchObject({
content: 'Replying to >>12',
parentCid: 'parent-cid',
postCid: 'parent-cid',
});
expect(testState.lastPublishOptions?.quotedCids).toBeUndefined();
});
it('drops cross-thread cids that arrive via stored publish options, keeping same-thread ones', async () => {
// Defense in depth: even quotedCids carried on the stored publish options must be filtered to
// the reply's own thread before publishing, never bypassing the same-thread guard.
await act(async () => {
usePostNumberStore.setState({ cidToPostCid: { 'quoted-cid': 'parent-cid', 'foreign-cid': 'other-thread-cid' } });
usePublishReplyStore.setState({
publishCommentOptions: {
'parent-cid': { content: 'body', parentCid: 'parent-cid', postCid: 'parent-cid', quotedCids: ['quoted-cid', 'foreign-cid'] },
},
});
});
expect(testState.lastPublishOptions?.quotedCids).toEqual(['quoted-cid']);
});
it('resolves same-board external quote references before triggering publish', async () => {
testState.resolveExternalQuoteTargetMock.mockResolvedValue({
cid: 'external-cid',
route: '/music/thread/external-cid',
communityAddress: 'music.eth',
});
// The resolver registers whatever it finds; here #44 turns out to be an unloaded reply under
// this same thread, so its cid is allowed in the published quotedCids.
usePostNumberStore.setState({ cidToPostCid: { 'quoted-cid': 'parent-cid', 'external-cid': 'parent-cid' } });
await act(async () => {
latestValue.setPublishReplyOptions({
+16 -2
View File
@@ -5,7 +5,7 @@ import { useShallow } from 'zustand/react/shallow';
import { useDirectories } from './use-directories';
import usePublishReplyStore from '../stores/use-publish-reply-store';
import usePostNumberStore, { getScopedNumberToCidMap } from '../stores/use-post-number-store';
import { getQuotedCidsFromContent, mergeQuotedCids } from '../lib/utils/reply-quote-utils';
import { filterSameThreadQuotedCids, getQuotedCidsFromContent, mergeQuotedCids } from '../lib/utils/reply-quote-utils';
import { extractUnresolvedExternalQuoteReferences, getExternalQuoteStatusMessage } from '../lib/utils/external-quote-utils';
import { resolveExternalQuoteTarget } from '../lib/utils/external-quote-resolver';
import useChallengesStore from '../stores/use-challenges-store';
@@ -97,6 +97,8 @@ const usePublishReply = ({ cid, communityAddress, postCid }: UsePublishReplyOpti
const resetPublishReplyOptions = useCallback(() => resetPublishReplyStore(parentCid), [parentCid, resetPublishReplyStore]);
const scopedNumberToCid = usePostNumberStore((state) => getScopedNumberToCidMap(state.numberToCid, communityAddress));
const cidToPostCid = usePostNumberStore((state) => state.cidToPostCid);
const threadPostCid = postCid ?? parentCid;
const quotedCids = useMemo(() => getQuotedCidsFromContent(content, scopedNumberToCid), [content, scopedNumberToCid]);
const unresolvedExternalQuoteReferences = useMemo(
() =>
@@ -126,7 +128,19 @@ const usePublishReply = ({ cid, communityAddress, postCid }: UsePublishReplyOpti
return merged.size > 0 ? [...merged] : undefined;
}, [quotedCids, resolvedExternalQuotedCids]);
const mergedPublishOptions = useMemo(() => mergeQuotedCids(publishCommentOptions, mergedQuotedCids), [publishCommentOptions, mergedQuotedCids]);
const mergedPublishOptions = useMemo(() => {
// Filter the FINAL payload so a reply only ever publishes same-thread quotedCids, even for any
// that arrived via stored publishCommentOptions. The protocol rejects cross-thread quotes
// (ERR_QUOTED_CID_NOT_UNDER_POST); cross-thread quotelinks still render/navigate via their text.
const options = mergeQuotedCids(publishCommentOptions, mergedQuotedCids);
if (!options?.quotedCids) {
return options;
}
const sameThreadQuotedCids = filterSameThreadQuotedCids(options.quotedCids, cidToPostCid, threadPostCid);
const { quotedCids: _crossThreadQuotedCids, ...optionsWithoutQuotedCids } = options;
return sameThreadQuotedCids ? { ...optionsWithoutQuotedCids, quotedCids: sameThreadQuotedCids } : optionsWithoutQuotedCids;
}, [publishCommentOptions, mergedQuotedCids, cidToPostCid, threadPostCid]);
const publishOptionsWithAbandon = useMemo(
() => ({
...mergedPublishOptions,
+13 -1
View File
@@ -10,7 +10,7 @@ import {
hasEnoughPreviewReplies,
sortRepliesForDisplay,
} from '../replies-preview-utils';
import { getQuotedCidsFromContent, mergeQuotedCids } from '../reply-quote-utils';
import { filterSameThreadQuotedCids, getQuotedCidsFromContent, mergeQuotedCids } from '../reply-quote-utils';
import { formatUserIDForDisplay, truncateWithEllipsisInMiddle } from '../string-utils';
import { getActiveSpecialTheme, getFormattedDate, getFormattedTimeAgo, getSpecialThemeClass, isChristmas, isHalloween } from '../time-utils';
@@ -249,6 +249,18 @@ describe('misc utils', () => {
expect(mergeQuotedCids(undefined, ['cid-12'])).toBeUndefined();
});
it('keeps only same-thread quoted cids so cross-thread quotes are not published', () => {
const cidToPostCid = { 'cid-12': 'thread-a', 'cid-45': 'thread-b', 'cid-op': 'thread-a' };
// cid-12 and the OP (cid-op) are under thread-a; cid-45 lives under another thread and is dropped.
expect(filterSameThreadQuotedCids(['cid-12', 'cid-45', 'cid-op'], cidToPostCid, 'thread-a')).toEqual(['cid-12', 'cid-op']);
// Every quote points at another thread, so nothing is published.
expect(filterSameThreadQuotedCids(['cid-45'], cidToPostCid, 'thread-a')).toBeUndefined();
// Unknown thread mapping or missing inputs resolve to no quoted cids.
expect(filterSameThreadQuotedCids(['cid-unknown'], cidToPostCid, 'thread-a')).toBeUndefined();
expect(filterSameThreadQuotedCids(['cid-12'], cidToPostCid, undefined)).toBeUndefined();
expect(filterSameThreadQuotedCids(undefined, cidToPostCid, 'thread-a')).toBeUndefined();
});
it('formats ids, truncates long strings, and localizes time labels', () => {
expect(formatUserIDForDisplay('board.eth')).toBe('board.eth');
expect(formatUserIDForDisplay('averyverylongdomainname.eth', 12)).toBe('averyvery...');
+13
View File
@@ -11,6 +11,19 @@ export const getQuotedCidsFromContent = (content: string | undefined, numberToCi
return cids.size > 0 ? [...cids] : undefined;
};
// The protocol only accepts a reply whose quotedCids all live under the same post/thread
// (ERR_QUOTED_CID_NOT_UNDER_POST), so drop cross-thread quotes here. The >>number text stays
// in the content and still renders/navigates as a quotelink; only the published metadata changes.
export const filterSameThreadQuotedCids = (
quotedCids: string[] | undefined,
cidToPostCid: Record<string, string> | undefined,
threadPostCid: string | undefined,
): string[] | undefined => {
if (!quotedCids?.length || !cidToPostCid || !threadPostCid) return undefined;
const sameThread = quotedCids.filter((cid) => cidToPostCid[cid] === threadPostCid);
return sameThread.length > 0 ? sameThread : undefined;
};
export const mergeQuotedCids = (publishCommentOptions: Record<string, any> | undefined, quotedCids: string[] | undefined) => {
if (!publishCommentOptions || !quotedCids?.length) return publishCommentOptions;
const currentQuotedCids = publishCommentOptions.quotedCids ?? [];
+21 -6
View File
@@ -7,6 +7,8 @@ interface PostNumberState {
// Post numbers are only unique within a board, so scope by canonical community address.
numberToCid: Record<string, Record<number, string>>;
cidToNumber: Record<string, number>;
// Thread (post) cid each comment lives under, so replies only publish same-thread quotedCids.
cidToPostCid: Record<string, string>;
registerComments: (comments: Comment[]) => void;
}
@@ -45,14 +47,24 @@ export const getCidForPostNumber = (numberToCid: Record<string, Record<number, s
const usePostNumberStore = create<PostNumberState>((set) => ({
numberToCid: {},
cidToNumber: {},
cidToPostCid: {},
registerComments: (comments: Comment[]) => {
if (!comments?.length) return;
set((state) => {
let nextNumberToCid = state.numberToCid;
let nextCidToNumber = state.cidToNumber;
let nextCidToPostCid = state.cidToPostCid;
let hasUpdates = false;
const ensureCloned = () => {
if (hasUpdates) return;
nextNumberToCid = { ...state.numberToCid };
nextCidToNumber = { ...state.cidToNumber };
nextCidToPostCid = { ...state.cidToPostCid };
hasUpdates = true;
};
for (const c of comments) {
const num = c?.number;
const cid = c?.cid;
@@ -61,24 +73,27 @@ const usePostNumberStore = create<PostNumberState>((set) => ({
const existingCid = nextNumberToCid[addr]?.[num];
if (existingCid !== cid || nextCidToNumber[cid] !== num) {
if (!hasUpdates) {
nextNumberToCid = { ...state.numberToCid };
nextCidToNumber = { ...state.cidToNumber };
hasUpdates = true;
}
ensureCloned();
if (!nextNumberToCid[addr] || nextNumberToCid[addr] === state.numberToCid[addr]) {
nextNumberToCid[addr] = { ...nextNumberToCid[addr] };
}
nextNumberToCid[addr][num] = cid;
nextCidToNumber[cid] = num;
}
// OPs are their own thread (no postCid); replies always carry their thread's postCid.
const postCid = c?.postCid || cid;
if (nextCidToPostCid[cid] !== postCid) {
ensureCloned();
nextCidToPostCid[cid] = postCid;
}
}
if (!hasUpdates) {
return state;
}
return { numberToCid: nextNumberToCid, cidToNumber: nextCidToNumber };
return { numberToCid: nextNumberToCid, cidToNumber: nextCidToNumber, cidToPostCid: nextCidToPostCid };
});
},
}));