Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/app/ui/components/Channel/PinnedPostBanner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,12 @@ export function PinnedPostBanner({
}
}, [pinnedPostId]);

if (!pinnedPostId || !postQuery.data || isDismissed) {
if (
!pinnedPostId ||
!postQuery.data ||
postQuery.data.isDeleted ||
isDismissed
) {
return null;
}

Expand Down
128 changes: 128 additions & 0 deletions packages/shared/src/db/queries.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1292,6 +1292,134 @@ describe('getPendingPosts', () => {
});
});

// TLON-5911: `clearGhostPosts` sweeps rows whose send was acknowledged but
// never sequenced and whose delete the server confirmed — no sequenced
// `addPost` will ever replace them, so they'd otherwise tombstone at the
// bottom of chat scrollers forever.
describe('clearGhostPosts', () => {
const channelId = 'ghost-sweep-channel';
const authorId = '~zod';
let seedCounter = 0;

async function seedPost(overrides: Partial<Post>): Promise<Post> {
const t = Date.now() + seedCounter++;
const base: Post = {
id: `ghost-${overrides.sentAt ?? t}-${Math.random()}`,
type: 'chat',
channelId,
authorId,
sentAt: t,
receivedAt: t,
sequenceNum: 0,
content: JSON.stringify([{ inline: ['seed'] }]),
syncedAt: t,
...overrides,
} as Post;
await queries.insertChannelPosts({ posts: [base] });
return base;
}

test('removes confirmed-delete ghosts and repoints the channel head; leaves every other shape alone', async () => {
await queries.insertChannels([{ id: channelId, type: 'chat' }]);

const confirmed = await seedPost({
id: 'confirmed-post',
sequenceNum: 5,
deliveryStatus: null,
});
// Confirmed tombstone: renders in place, must survive.
const confirmedTombstone = await seedPost({
id: 'confirmed-tombstone',
sequenceNum: 6,
deliveryStatus: null,
isDeleted: true,
});
// Delete still in flight: outcome unknown, must survive.
const deleteInFlight = await seedPost({
id: 'delete-in-flight',
deliveryStatus: 'sent',
isDeleted: true,
deleteStatus: 'pending',
});
// Send still in flight (delete already confirmed): delivery polling via
// `getDeliveryPendingPosts` must still be able to reconcile the original
// send, so the sweep must not touch it.
const sendInFlight = await seedPost({
id: 'send-in-flight',
deliveryStatus: 'enqueued',
isDeleted: true,
deleteStatus: 'sent',
});
// Deleted but not yet delete-confirmed sent row: the normal tombstone
// window, must survive.
const sentCatchUp = await seedPost({
id: 'sent-catchup',
deliveryStatus: 'sent',
isDeleted: true,
});
// Reply ghosts are out of scope for the sweep (they don't flow through
// the pending merge layer).
const replyGhost = await seedPost({
id: 'reply-ghost',
type: 'reply',
parentId: confirmed.id,
deliveryStatus: 'sent',
isDeleted: true,
deleteStatus: 'sent',
});
// The actual ghost: acknowledged send, never sequenced, delete confirmed.
const ghost = await seedPost({
id: 'ghost',
deliveryStatus: 'sent',
isDeleted: true,
deleteStatus: 'sent',
});

// Simulate the stale head state a lingering ghost can leave behind.
await queries.updateChannel({ id: channelId, lastPostId: null });

await queries.clearGhostPosts();

const remaining = (
await getClient()!
.select({ id: schema.posts.id })
.from(schema.posts)
.where($.eq(schema.posts.channelId, channelId))
).map((r) => r.id);
expect(remaining).not.toContain(ghost.id);
expect(remaining).toEqual(
expect.arrayContaining([
confirmed.id,
confirmedTombstone.id,
deleteInFlight.id,
sendInFlight.id,
sentCatchUp.id,
replyGhost.id,
])
);

// Channel head repointed to the newest remaining previewable post.
const chan = await queries.getChannel({ id: channelId });
expect(chan!.lastPostId).toBe(confirmed.id);
});

test('is a no-op when there are no ghosts', async () => {
await queries.insertChannels([{ id: channelId, type: 'chat' }]);
const post = await seedPost({
id: 'plain-post',
sequenceNum: 3,
deliveryStatus: null,
});

await queries.clearGhostPosts();

const row = await getClient()!.query.posts.findFirst({
where: (posts, { eq }) => eq(posts.id, post.id),
});
expect(row).toBeTruthy();
});
});

// TLON-5606: `undoOptimisticReplyBump` must undo a single optimistic reply
// add without clobbering server-sourced reply summary state. Two regimes:
// complete local cache → full recompute; partial local cache → decrement
Expand Down
38 changes: 38 additions & 0 deletions packages/shared/src/db/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4621,6 +4621,44 @@ export const getPendingPosts = createReadQuery(
['posts']
);

/**
* Hard-delete "ghost" tombstones: top-level rows whose send was acknowledged
* but never sequenced (`deliveryStatus: 'sent'` / `'needs_verification'`,
* `sequenceNum === 0`) and whose delete was confirmed by the server
* (`deleteStatus: 'sent'`). No sequenced `addPost` will ever replace such a
* row, so it would otherwise sit in the pending merge layer as an `isDeleted`
* tombstone pinned to the bottom of chat-style scrollers indefinitely
* (TLON-5911). `deletePost` now clears these at delete time; this sweep
* repairs rows poisoned before that fix. In-flight `enqueued` / `pending`
* rows are excluded — `getDeliveryPendingPosts` keeps polling those so the
* original send can still reconcile against the server.
*/
export const clearGhostPosts = createWriteQuery(
'clearGhostPosts',
async (ctx: QueryCtx) => {
return withTransactionCtx(ctx, async (txCtx) => {
const ghosts = await txCtx.db
.delete($posts)
.where(
and(
eq($posts.isDeleted, true),
eq($posts.sequenceNum, 0),
isNull($posts.parentId),
inArray($posts.deliveryStatus, ['sent', 'needs_verification']),
eq($posts.deleteStatus, 'sent')
Comment thread
jamesacklin marked this conversation as resolved.
Outdated
)
)
.returning({ channelId: $posts.channelId });
const channelIds = [...new Set(ghosts.map((g) => g.channelId))];
for (const channelId of channelIds) {
await recomputeChannelLastPost({ channelId }, txCtx);
}
return ghosts.length;
});
},
['posts', 'channels']
);

/**
* Rows that are still in-flight from the sender's perspective, used by
* `syncChannelWithBackoff` to decide whether delivery polling should
Expand Down
107 changes: 107 additions & 0 deletions packages/shared/src/store/postActions/postActions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1442,3 +1442,110 @@ describe('clearing a failed optimistic post', () => {
expect(parentAfter!.replyContactIds).toEqual(['~alfa', '~bravo']);
});
});

// TLON-5911: deleting a server-acknowledged-but-unsequenced send
// (`deliveryStatus: 'sent'`, `sequenceNum: 0`). Once the server confirms the
// delete, the sequenced `addPost` that would normally replace the cached row
// is never coming, so the row must be hard-deleted — otherwise it survives as
// an `isDeleted` tombstone that `mergePendingPosts` sorts unconfirmed-first,
// i.e. pinned to the bottom of the chat scroller indefinitely.
describe('deleting a sent-but-unsequenced post', () => {
beforeEach(async () => {
await db.insertChannels([
db.buildChannel({ id: TEST_CHANNEL, type: 'chat' }),
Comment thread
jamesacklin marked this conversation as resolved.
]);
vi.mocked(poke).mockResolvedValue(0);
updateSession({ startTime: Date.now(), channelStatus: 'active' });
});

afterEach(() => {
vi.mocked(poke).mockClear();
updateSession(null);
});

async function seedSentUnsequencedPost(): Promise<db.Post> {
const channel = (await db.getChannel({ id: TEST_CHANNEL }))!;
const post = db.buildPost({
authorId: '~zod',
author: null,
channel,
sequenceNum: 0,
content: [{ inline: [friendlyUniqueString()] }],
deliveryStatus: 'sent',
});
await db.insertChannelPosts({ posts: [post] });
return post;
}

test('deletePost() sends the server delete, then hard-deletes the unsequenced row', async () => {
const channel = (await db.getChannel({ id: TEST_CHANNEL }))!;
const previous = db.buildPost({
authorId: '~zod',
author: null,
channel,
sequenceNum: 5,
content: [{ inline: ['previous previewable post'] }],
deliveryStatus: 'sent',
});
await db.insertChannelPosts({ posts: [previous] });

const post = await seedSentUnsequencedPost();
expect((await db.getPendingPosts(TEST_CHANNEL)).map((p) => p.id)).toContain(
post.id
);

await deletePost({ post });

// Unlike the failed-optimistic short-circuit, the row may exist on the
// server, so the delete round trip must happen.
expect(poke).toHaveBeenCalled();

// Once the server confirms, the row is gone — no ghost tombstone left in
// the DB or the pending merge layer.
expect(await fetchPost(post.id)).toBeUndefined();
const pending = await db.getPendingPosts(TEST_CHANNEL);
expect(pending.map((p) => p.id)).not.toContain(post.id);
const merged = mergePendingPosts({
newPosts: [],
pendingPosts: pending,
existingPosts: [],
deletedPosts: {},
hasNewest: true,
});
expect(merged.map((p) => p.id)).not.toContain(post.id);

// Channel preview repoints to the newest remaining previewable post.
const chan = await db.getChannel({ id: TEST_CHANNEL });
expect(chan!.lastPostId).toBe(previous.id);
});

test('deletePost() keeps the normal tombstone when the sequenced echo lands during the delete round trip', async () => {
const post = await seedSentUnsequencedPost();
vi.mocked(poke).mockImplementationOnce(async () => {
// The sequenced addPost catches up mid-flight: the row is no longer
// unsequenced by the time the delete is acknowledged.
await db.updatePost({ id: post.id, sequenceNum: 42 });
return 0;
});

await deletePost({ post });

const rowAfter = await fetchPost(post.id);
expect(rowAfter).toBeTruthy();
expect(rowAfter!.isDeleted).toBe(true);
expect(rowAfter!.sequenceNum).toBe(42);
});

test('deletePost() failure rolls back and does NOT hard-delete the row', async () => {
const post = await seedSentUnsequencedPost();
vi.mocked(poke).mockRejectedValueOnce(new Error('nack'));

await deletePost({ post });

const rowAfter = await fetchPost(post.id);
expect(rowAfter).toBeTruthy();
expect(rowAfter!.isDeleted).toBeFalsy();
expect(rowAfter!.deliveryStatus).toBe('sent');
expect(rowAfter!.deleteStatus).toBe('failed');
});
});
22 changes: 22 additions & 0 deletions packages/shared/src/store/postActions/postActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,28 @@ export async function deletePost({ post }: { post: db.Post }) {
: api.deletePost(post.channelId, post.id, post.authorId)
);
await db.updatePost({ id: post.id, deleteStatus: 'sent' });

// If the row is still a server-acknowledged-but-unsequenced send
// (`deliveryStatus: 'sent'`, `sequenceNum === 0`), the server just
// confirmed the delete, so the sequenced `addPost` that would normally
// replace this cached row is never coming. Without this, the row survives
// as an `isDeleted` tombstone that `mergePendingPosts` sorts
// unconfirmed-first — i.e. pinned to the bottom of chat-style scrollers
// forever (TLON-5911). Re-read first: the sequenced echo may have landed
// during the delete round trip, in which case the normal tombstone path
// applies. Scoped to top-level rows (replies don't flow through the
// pending merge layer) and to shapes with no live send flow —
// `enqueued` / `pending` rows still converge via the delivery machinery.
const settledPost = await db.getPost({ postId: post.id });
Comment thread
jamesacklin marked this conversation as resolved.
Outdated
if (
settledPost &&
!settledPost.parentId &&
settledPost.sequenceNum === 0 &&
(settledPost.deliveryStatus === 'sent' ||
settledPost.deliveryStatus === 'needs_verification')
) {
await clearUnsentPost(settledPost);
Comment thread
jamesacklin marked this conversation as resolved.
Outdated
}
} catch (e) {
console.error('Failed to delete post', e);

Expand Down
7 changes: 7 additions & 0 deletions packages/shared/src/store/sync/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2152,6 +2152,13 @@ export const syncStart = async (alreadySubscribed?: boolean) => {
logger.trackError('sync start: changes sync failed', { error })
);

// Repair ghost tombstones (deleted, never-sequenced sends whose delete
// the server already confirmed) left behind before `deletePost` learned
// to clear them — see db.clearGhostPosts / TLON-5911. Local-DB only.
db.clearGhostPosts().catch((error) =>
logger.trackError('sync start: ghost post cleanup failed', { error })
);

// brief delay to let syncSince queue first (it requires a storage item read before
// it hits the sync queue)
const isE2eRun = (globalThis as any).TLON_IS_E2E === true;
Expand Down
Loading